mirror of
https://github.com/lrnselfreliance/wrolpi.git
synced 2026-10-26 02:48:38 -04:00
search: don't COUNT(*) OVER() the page query (#702)
COUNT(*) OVER() forced SQLite to visit every matching file_group before LIMIT, which is a full table scan of the (large) file_group table — ~22s cold on a Pi for videos ordered by download date. Count separately and cache for 60s by filters so indexed orders can stop after one page. Past-the-end pages now report the true total. Videos/archives/docs pagers keep totalPages across page changes.
This commit is contained in:
parent
0b61d3ab08
commit
8008cd1fd8
9 changed files with 436 additions and 74 deletions
|
|
@ -30,7 +30,8 @@ from wrolpi.dates import now, Seconds
|
|||
from wrolpi.db import get_db_session, get_db_curs
|
||||
from wrolpi.errors import UnknownArchive, InvalidOrderBy, InvalidDatetime
|
||||
from wrolpi.events import Events
|
||||
from wrolpi.files.lib import handle_file_group_search_results
|
||||
from wrolpi.files.lib import handle_file_group_search_results, cached_search_total, \
|
||||
search_filter_cache_key, count_file_groups
|
||||
from wrolpi.files.models import FileGroup
|
||||
from wrolpi.switches import register_switch_handler, ActivateSwitchMethod
|
||||
from wrolpi.tags import tag_append_sub_select_where
|
||||
|
|
@ -1407,9 +1408,8 @@ def search_archives(search_str: str, domain: str, limit: int, offset: int, order
|
|||
# which case we behave as if no search was requested.
|
||||
file_group_search = fts.file_group_search(search_str, deep=deep) if search_str else None
|
||||
if file_group_search:
|
||||
# A search_str was provided by the user, modify the query to filter by it. The rank and
|
||||
# snippets come from a subquery because FTS5 auxiliary functions (snippet) cannot appear
|
||||
# in a query that also uses window functions (COUNT(*) OVER()).
|
||||
# A search_str was provided by the user, modify the query to filter by it. Rank and
|
||||
# snippets come from a subquery (FTS5 auxiliary functions cannot mix with some SELECT shapes).
|
||||
headline_selects = fts.file_group_headline_selects() if headline else ''
|
||||
fts_join = f'''JOIN (
|
||||
SELECT fts.rowid AS id, {file_group_search.rank_select}{headline_selects}
|
||||
|
|
@ -1425,14 +1425,22 @@ def search_archives(search_str: str, domain: str, limit: int, offset: int, order
|
|||
fts.b_headline, fts.c_headline, fts.d_headline,
|
||||
fg.title AS title_headline'''
|
||||
|
||||
null_filter = None
|
||||
if order:
|
||||
try:
|
||||
order_by = ARCHIVE_ORDERS[order]
|
||||
except KeyError:
|
||||
raise InvalidOrderBy(f'Invalid order byy {order}')
|
||||
if order in NO_NULL_ORDERS:
|
||||
wheres.append(NO_NULL_ORDERS[order])
|
||||
null_filter = NO_NULL_ORDERS[order]
|
||||
wheres.append(null_filter)
|
||||
|
||||
if not file_group_search and order in ('rank', '-rank'):
|
||||
# Rank without a search used to ORDER BY the windowed total (a constant).
|
||||
order_by = ARCHIVE_ORDERS['-published_datetime'] if order == 'rank' \
|
||||
else ARCHIVE_ORDERS['published_datetime']
|
||||
|
||||
tag_names = tag_names or []
|
||||
wheres, params = tag_append_sub_select_where(wheres, params, tag_names)
|
||||
|
||||
if domain:
|
||||
|
|
@ -1442,8 +1450,7 @@ def search_archives(search_str: str, domain: str, limit: int, offset: int, order
|
|||
"a.collection_id = (select id from collection where collection.name = :domain and collection.kind = 'domain' LIMIT 1)")
|
||||
|
||||
select_columns = f"{select_columns}," if select_columns else ""
|
||||
wheres = '\n AND '.join(wheres)
|
||||
where = f'WHERE\n{wheres}' if wheres else ''
|
||||
where = ('WHERE\n' + '\n AND '.join(wheres)) if wheres else ''
|
||||
if where or fts_join:
|
||||
from_clause = f'''FROM archive a
|
||||
LEFT JOIN file_group fg ON fg.id = a.file_group_id
|
||||
|
|
@ -1459,7 +1466,7 @@ def search_archives(search_str: str, domain: str, limit: int, offset: int, order
|
|||
SELECT
|
||||
a.file_group_id AS id, -- always get `file_group.id` for `handle_file_group_search_results`
|
||||
{select_columns}
|
||||
COUNT(*) OVER() AS total
|
||||
fg.id AS _fg_id
|
||||
{headline_columns}
|
||||
{from_clause}
|
||||
{where}
|
||||
|
|
@ -1468,7 +1475,33 @@ def search_archives(search_str: str, domain: str, limit: int, offset: int, order
|
|||
'''.strip()
|
||||
logger.debug(f'{stmt} {params}')
|
||||
|
||||
results, total = handle_file_group_search_results(stmt, params)
|
||||
unfiltered = not search_str and not tag_names and not domain and not null_filter
|
||||
if unfiltered:
|
||||
total = cached_search_total(
|
||||
search_filter_cache_key('archives'),
|
||||
lambda: count_file_groups('SELECT COUNT(*) AS total FROM archive', dict()),
|
||||
)
|
||||
else:
|
||||
if file_group_search:
|
||||
count_from = f'''FROM archive a
|
||||
LEFT JOIN file_group fg ON fg.id = a.file_group_id
|
||||
{file_group_search.join}'''
|
||||
count_wheres = list(wheres)
|
||||
count_wheres.append(file_group_search.where)
|
||||
count_where = ('WHERE\n' + '\n AND '.join(count_wheres)) if count_wheres else ''
|
||||
count_stmt = f'SELECT COUNT(*) AS total {count_from} {count_where}'
|
||||
else:
|
||||
# Same filters as the page, but do not walk the date index just to count.
|
||||
count_from = '''FROM archive a
|
||||
LEFT JOIN file_group fg ON fg.id = a.file_group_id'''
|
||||
count_stmt = f'SELECT COUNT(*) AS total {count_from} {where}'
|
||||
cache_key = search_filter_cache_key(
|
||||
'archives', search_str=search_str, domain=domain, tag_names=tag_names,
|
||||
deep=deep, null_filter=null_filter,
|
||||
)
|
||||
total = cached_search_total(cache_key, lambda: count_file_groups(count_stmt, params))
|
||||
|
||||
results, total = handle_file_group_search_results(stmt, params, total=total)
|
||||
|
||||
if file_group_search and headline and results:
|
||||
# Highlight the plain titles like the FTS snippets above (Postgres used ts_headline).
|
||||
|
|
|
|||
|
|
@ -158,6 +158,10 @@ async def test_search_offset(test_session, archive_factory, async_client):
|
|||
await check_results(async_client, data, list(range(300, 280, -1)))
|
||||
data = {'search_str': None, 'offset': 500}
|
||||
await check_results(async_client, data, [])
|
||||
# Past the last page still reports the true total (a separate COUNT, not COUNT(*) OVER()).
|
||||
request, response = await async_client.post('/api/archive/search', content=json.dumps(data))
|
||||
assert response.status_code == HTTPStatus.OK
|
||||
assert response.json['totals']['file_groups'] == 500
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
|
|||
|
|
@ -12,7 +12,8 @@ from wrolpi.collections.models import Collection
|
|||
from wrolpi.collections.types import collection_type_registry
|
||||
from wrolpi.db import get_db_session
|
||||
from wrolpi.errors import ValidationError
|
||||
from wrolpi.files.lib import split_path_stem_and_suffix, get_mimetype
|
||||
from wrolpi.files.lib import split_path_stem_and_suffix, get_mimetype, cached_search_total, \
|
||||
search_filter_cache_key
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
|
@ -388,7 +389,19 @@ def _search_docs(search_str=None, author=None, subject=None, language=None, mime
|
|||
.subquery()
|
||||
query = query.filter(FileGroup.id.in_(tagged_fg_ids))
|
||||
|
||||
total = query.count()
|
||||
unfiltered = not (search_str or author or subject or language or mimetype or tag_names)
|
||||
if unfiltered:
|
||||
total = cached_search_total(
|
||||
search_filter_cache_key('docs'),
|
||||
lambda: session.query(func.count(Doc.id)).scalar() or 0,
|
||||
)
|
||||
else:
|
||||
cache_key = search_filter_cache_key(
|
||||
'docs', search_str=search_str, author=author, subject=subject,
|
||||
language=language, mimetype=mimetype, tag_names=tag_names, deep=deep,
|
||||
)
|
||||
# Count before ORDER BY / LIMIT; cache by filters not page.
|
||||
total = cached_search_total(cache_key, lambda q=query: q.count())
|
||||
|
||||
# Ordering.
|
||||
if order_by == 'rank' and fts_sq is not None:
|
||||
|
|
|
|||
|
|
@ -669,8 +669,7 @@ async def test_search_videos_default_page_pagination(test_session, video_factory
|
|||
await assert_video_search(order_by='-published_datetime', limit=2, offset=0, assert_total=6, assert_ids=newest[0:2])
|
||||
await assert_video_search(order_by='-published_datetime', limit=2, offset=2, assert_total=6, assert_ids=newest[2:4])
|
||||
await assert_video_search(order_by='-published_datetime', limit=2, offset=4, assert_total=6, assert_ids=newest[4:6])
|
||||
# Past the end the fast path still reports the true total (the generic query has no rows to
|
||||
# carry `COUNT(*) OVER()` in, so it reports 0 there; the fast path is the better behavior).
|
||||
# Past the end both the date fast path and the generic query report the true total.
|
||||
await assert_video_search(order_by='-published_datetime', limit=2, offset=6, assert_total=6, assert_ids=[])
|
||||
|
||||
# Oldest first: NULL dates first (SQLite sorts NULL lowest), ties broken by id ascending.
|
||||
|
|
@ -691,32 +690,38 @@ async def test_search_videos_default_page_pagination(test_session, video_factory
|
|||
generic = ids(order, limit, offset)
|
||||
assert fast == generic, f'{order} limit={limit} offset={offset}: fast {fast} != generic {generic}'
|
||||
|
||||
# Generic orders used to report total=0 past the last page because COUNT(*) OVER() had no
|
||||
# rows to ride on. A separate COUNT reports the true total, like the date fast path.
|
||||
await assert_video_search(order_by='-download_datetime', limit=2, offset=6, assert_total=6, assert_ids=[])
|
||||
await assert_video_search(order_by='-size', limit=2, offset=6, assert_total=6, assert_ids=[])
|
||||
|
||||
|
||||
def test_cached_total(test_session):
|
||||
"""The unfiltered Videos page total is cached briefly: counting every video is a full index scan
|
||||
that costs seconds cold on a large library, and pagination can tolerate a slightly stale total."""
|
||||
from modules.videos.video import lib
|
||||
"""Search totals are cached briefly: counting a large library is a full index scan
|
||||
that costs seconds cold, and pagination can tolerate a slightly stale total."""
|
||||
from wrolpi.files import lib as files_lib
|
||||
calls = []
|
||||
|
||||
def compute():
|
||||
calls.append(1)
|
||||
return 42
|
||||
|
||||
lib._TOTALS_CACHE.clear()
|
||||
with mock.patch('modules.videos.video.lib.PYTEST', False):
|
||||
assert lib._cached_total('k', compute) == 42
|
||||
assert lib._cached_total('k', compute) == 42
|
||||
files_lib._SEARCH_TOTALS_CACHE.clear()
|
||||
with mock.patch('wrolpi.files.lib.PYTEST', False):
|
||||
assert files_lib.cached_search_total('k', compute) == 42
|
||||
assert files_lib.cached_search_total('k', compute) == 42
|
||||
assert len(calls) == 1, 'second call within the TTL should hit the cache'
|
||||
# Expire the entry.
|
||||
cached_at, total = lib._TOTALS_CACHE['k']
|
||||
lib._TOTALS_CACHE['k'] = (cached_at - lib.TOTALS_CACHE_SECONDS - 1, total)
|
||||
assert lib._cached_total('k', compute) == 42
|
||||
cached_at, total = files_lib._SEARCH_TOTALS_CACHE['k']
|
||||
files_lib._SEARCH_TOTALS_CACHE['k'] = (
|
||||
cached_at - files_lib.SEARCH_TOTALS_CACHE_SECONDS - 1, total)
|
||||
assert files_lib.cached_search_total('k', compute) == 42
|
||||
assert len(calls) == 2, 'expired entry should be recomputed'
|
||||
|
||||
# Tests need exact totals, so the cache is bypassed under pytest.
|
||||
assert lib._cached_total('k', compute) == 42
|
||||
assert len(calls) == 3
|
||||
lib._TOTALS_CACHE.clear()
|
||||
assert files_lib.cached_search_total('k', compute) == 42
|
||||
assert len(calls) == 3, 'pytest should bypass the cache'
|
||||
|
||||
|
||||
async def test_video_description_endpoint(async_client, test_session, video_factory):
|
||||
"""The description is served by its own endpoint, like comments and captions. It comes from the
|
||||
|
|
|
|||
|
|
@ -1,8 +1,7 @@
|
|||
import asyncio
|
||||
import random
|
||||
import time
|
||||
from datetime import timedelta
|
||||
from typing import Tuple, Optional, List, Dict, Callable
|
||||
from typing import Tuple, Optional, List
|
||||
|
||||
import yt_dlp
|
||||
from sqlalchemy import or_
|
||||
|
|
@ -14,10 +13,11 @@ from wrolpi.common import logger, limit_concurrent, wrol_mode_check
|
|||
from wrolpi.dates import now
|
||||
from wrolpi.db import get_db_session, get_db_curs
|
||||
from wrolpi.downloader import download_manager
|
||||
from wrolpi.files.lib import handle_file_group_search_results
|
||||
from wrolpi.files.lib import handle_file_group_search_results, cached_search_total, \
|
||||
search_filter_cache_key, count_file_groups
|
||||
from wrolpi.files.models import FileGroup
|
||||
from wrolpi.tags import tag_append_sub_select_where
|
||||
from wrolpi.vars import VIDEO_COMMENTS_FETCH_COUNT, YTDLP_CACHE_DIR, PYTEST
|
||||
from wrolpi.vars import VIDEO_COMMENTS_FETCH_COUNT, YTDLP_CACHE_DIR
|
||||
from ..cookies import cookies_unlocked, cookies_for_download
|
||||
from ..lib import get_yt_dlp_http_headers, get_yt_dlp_sleep_opts
|
||||
|
||||
|
|
@ -91,22 +91,6 @@ VIDEO_QUERY_LIMIT = 24
|
|||
INDEXED_DATE_ORDERS = ('published_datetime', '-published_datetime')
|
||||
VIDEO_MIMETYPE_PREFIXES = ('video/', 'audio/')
|
||||
|
||||
# The unfiltered Videos page total, cached for a short time. Counting every video walks the whole
|
||||
# mimetype index, which costs seconds cold on a large library on a spinning disk; pagination can
|
||||
# tolerate a total that is a minute stale. Bypassed under pytest so tests see exact totals.
|
||||
TOTALS_CACHE_SECONDS = 60
|
||||
_TOTALS_CACHE: Dict[str, Tuple[float, int]] = dict()
|
||||
|
||||
|
||||
def _cached_total(key: str, compute: Callable[[], int]) -> int:
|
||||
if not PYTEST:
|
||||
hit = _TOTALS_CACHE.get(key)
|
||||
if hit and time.monotonic() - hit[0] < TOTALS_CACHE_SECONDS:
|
||||
return hit[1]
|
||||
total = compute()
|
||||
_TOTALS_CACHE[key] = (time.monotonic(), total)
|
||||
return total
|
||||
|
||||
|
||||
def _video_mimetypes() -> List[str]:
|
||||
"""Every distinct video/* and audio/* mimetype in the library (a handful; served by the mimetype index)."""
|
||||
|
|
@ -133,12 +117,11 @@ def _count_videos() -> int:
|
|||
def _search_videos_by_date(order: str, limit: int, offset: int) -> Tuple[List[dict], int]:
|
||||
"""The unfiltered Videos page, ordered by date, without visiting every video.
|
||||
|
||||
The generic query (`mimetype LIKE 'video/%' OR ...` + `COUNT(*) OVER()`) must visit every
|
||||
video's index entry before it can apply LIMIT, which scales with the library rather than the
|
||||
page. Here each exact mimetype is a pure covering-index seek on (mimetype, effective_datetime)
|
||||
that reads only `limit + offset` entries; those few rows are then merged. Results must be
|
||||
identical to the generic query's (same ORDER BY, including the id tiebreaker and NULL placement).
|
||||
The total is a separate, cached count."""
|
||||
The generic query (`mimetype LIKE 'video/%' OR ...`) plus a windowed COUNT used to visit every
|
||||
video before LIMIT. Here each exact mimetype is a covering-index seek on
|
||||
(mimetype, effective_datetime) that reads only `limit + offset` entries; those few rows are
|
||||
then merged. Results must be identical to the generic query's (same ORDER BY, including the
|
||||
id tiebreaker and NULL placement). The total is a separate, cached count."""
|
||||
order_by = VIDEO_ORDERS[order]
|
||||
mimetypes = _video_mimetypes()
|
||||
if not mimetypes:
|
||||
|
|
@ -160,7 +143,7 @@ def _search_videos_by_date(order: str, limit: int, offset: int) -> Tuple[List[di
|
|||
'''.strip()
|
||||
logger.debug(f'{stmt} {params}')
|
||||
|
||||
total = _cached_total('videos', _count_videos)
|
||||
total = cached_search_total(search_filter_cache_key('videos'), _count_videos)
|
||||
return handle_file_group_search_results(stmt, params, total=total)
|
||||
|
||||
|
||||
|
|
@ -199,17 +182,15 @@ def search_videos(
|
|||
params['channel_id'] = channel_id
|
||||
|
||||
# `deep` searches d_text (captions) as well; the default abc search is much smaller and faster.
|
||||
# The subquery join is required: snippet() cannot coexist with COUNT(*) OVER() in one SELECT.
|
||||
# Rank/snippets come from a subquery so they stay valid if a caller adds window functions.
|
||||
fts_search = fts.file_group_search_join(search_str, deep=deep, headlines=bool(headline)) \
|
||||
if search_str else None
|
||||
if fts_search:
|
||||
# A search_str was provided by the user, modify the query to filter by it.
|
||||
select_columns = f'fg.id, {fts_search.rank_select}, COUNT(*) OVER() AS total'
|
||||
select_columns = f'fg.id, {fts_search.rank_select}'
|
||||
joins.append(fts_search.join)
|
||||
params.update(fts_search.params)
|
||||
else:
|
||||
# No (usable) search_str provided. Get id and total only.
|
||||
select_columns = 'fg.id, COUNT(*) OVER() AS total'
|
||||
select_columns = 'fg.id'
|
||||
|
||||
wheres, params = tag_append_sub_select_where(wheres, params, tag_names)
|
||||
|
||||
|
|
@ -222,21 +203,27 @@ def search_videos(
|
|||
# Convert the user-friendly order by into a real order by, restrict what can be interpolated by using the
|
||||
# whitelist.
|
||||
order_by = VIDEO_ORDERS[DEFAULT_VIDEO_ORDER]
|
||||
null_filter = None
|
||||
if order:
|
||||
try:
|
||||
order_by = VIDEO_ORDERS[order]
|
||||
except KeyError:
|
||||
raise
|
||||
if order in NO_NULL_ORDERS:
|
||||
wheres.append(NO_NULL_ORDERS[order])
|
||||
null_filter = NO_NULL_ORDERS[order]
|
||||
wheres.append(null_filter)
|
||||
if order in JOIN_ORDERS:
|
||||
join_video = True
|
||||
|
||||
effective_order = order or DEFAULT_VIDEO_ORDER
|
||||
if not fts_search and effective_order in ('rank', '-rank'):
|
||||
# Rank without a search used to ORDER BY the windowed total (a constant). Equivalent: id.
|
||||
order_by = 'fg.id ASC' if effective_order == '-rank' else 'fg.id DESC'
|
||||
|
||||
if join_video:
|
||||
joins.insert(0, 'LEFT JOIN video v on v.file_group_id = fg.id')
|
||||
|
||||
wheres = '\n AND '.join(wheres)
|
||||
where = f'WHERE\n{wheres}' if wheres else ''
|
||||
where = ('WHERE\n' + '\n AND '.join(wheres)) if wheres else ''
|
||||
join = '\n'.join(joins)
|
||||
stmt = f'''
|
||||
SELECT
|
||||
|
|
@ -250,7 +237,35 @@ def search_videos(
|
|||
'''.strip()
|
||||
logger.debug(f'{stmt} {params}')
|
||||
|
||||
results, total = handle_file_group_search_results(stmt, params)
|
||||
unfiltered = (not search_str and not tag_names and not channel_id and not censored
|
||||
and not null_filter)
|
||||
if unfiltered:
|
||||
total = cached_search_total(search_filter_cache_key('videos'), _count_videos)
|
||||
else:
|
||||
if fts_search:
|
||||
fts_count = fts.file_group_search(search_str, deep=deep)
|
||||
count_joins = [j for j in joins if 'file_group_fts' not in j]
|
||||
if fts_count:
|
||||
count_joins.append(fts_count.join)
|
||||
count_wheres = [w for w in wheres]
|
||||
if fts_count:
|
||||
count_wheres.append(fts_count.where)
|
||||
count_join = '\n'.join(count_joins)
|
||||
count_where = ('WHERE\n' + '\n AND '.join(count_wheres)) if count_wheres else ''
|
||||
count_stmt = f'SELECT COUNT(*) AS total FROM file_group fg {count_join} {count_where}'
|
||||
count_params = dict(params)
|
||||
if fts_count:
|
||||
count_params.update(fts_count.params)
|
||||
else:
|
||||
count_stmt = f'SELECT COUNT(*) AS total FROM file_group fg {join} {where}'
|
||||
count_params = params
|
||||
cache_key = search_filter_cache_key(
|
||||
'videos', search_str=search_str, channel_id=channel_id, tag_names=tag_names,
|
||||
censored=censored, deep=deep, null_filter=null_filter,
|
||||
)
|
||||
total = cached_search_total(cache_key, lambda: count_file_groups(count_stmt, count_params))
|
||||
|
||||
results, total = handle_file_group_search_results(stmt, params, total=total)
|
||||
return results, total
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue