From 8008cd1fd87c7d36326a5ef05894833f118cd574 Mon Sep 17 00:00:00 2001 From: lrnselfreliance <33263708+lrnselfreliance@users.noreply.github.com> Date: Sat, 12 Sep 2026 14:44:10 -0600 Subject: [PATCH] search: don't COUNT(*) OVER() the page query (#702) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- app/src/hooks/customHooks.js | 3 - modules/archive/lib.py | 51 ++++++-- modules/archive/test/test_api.py | 4 + modules/docs/lib.py | 17 ++- modules/videos/test/test_api.py | 35 +++--- modules/videos/video/lib.py | 87 +++++++------ scripts/search_order_perf.py | 209 +++++++++++++++++++++++++++++++ wrolpi/files/lib.py | 90 +++++++++++-- wrolpi/files/test/test_lib.py | 14 +++ 9 files changed, 436 insertions(+), 74 deletions(-) create mode 100644 scripts/search_order_perf.py diff --git a/app/src/hooks/customHooks.js b/app/src/hooks/customHooks.js index e59f55bd..d8f9dcf1 100644 --- a/app/src/hooks/customHooks.js +++ b/app/src/hooks/customHooks.js @@ -523,7 +523,6 @@ export const useSearchArchives = (defaultLimit) => { const localSearchArchives = async () => { setArchives(null); - setTotalPages(0); try { let [archives_, total] = await searchArchives(offset, limit, domain, searchStr, order, activeTags, headline, deep); @@ -594,7 +593,6 @@ export const useSearchDocs = (defaultLimit) => { const localSearchDocs = async () => { setDocs(null); - setTotalPages(0); try { const mimetype = docFilterToMimetype(filter); let [docs_, total] = await searchDocs(offset, limit, searchStr, order, activeTags, author, subject, @@ -657,7 +655,6 @@ export const useSearchVideos = (defaultLimit, channelId, order_by) => { const localSearchVideos = async () => { setVideos(null); setLoading(true); - setTotalPages(0); try { let [videos_, total] = await searchVideos(offset, limit, channelId, searchStr, order, activeTags, headline, censored, deep); diff --git a/modules/archive/lib.py b/modules/archive/lib.py index 5ed60c50..1838db5b 100644 --- a/modules/archive/lib.py +++ b/modules/archive/lib.py @@ -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). diff --git a/modules/archive/test/test_api.py b/modules/archive/test/test_api.py index 5e90abc9..2a9509b6 100644 --- a/modules/archive/test/test_api.py +++ b/modules/archive/test/test_api.py @@ -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 diff --git a/modules/docs/lib.py b/modules/docs/lib.py index 657b0abd..3c2fd967 100644 --- a/modules/docs/lib.py +++ b/modules/docs/lib.py @@ -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: diff --git a/modules/videos/test/test_api.py b/modules/videos/test/test_api.py index 3ba0a19d..5f74702d 100644 --- a/modules/videos/test/test_api.py +++ b/modules/videos/test/test_api.py @@ -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 diff --git a/modules/videos/video/lib.py b/modules/videos/video/lib.py index 718c684f..b7424835 100644 --- a/modules/videos/video/lib.py +++ b/modules/videos/video/lib.py @@ -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 diff --git a/scripts/search_order_perf.py b/scripts/search_order_perf.py new file mode 100644 index 00000000..0f9322b9 --- /dev/null +++ b/scripts/search_order_perf.py @@ -0,0 +1,209 @@ +#!/usr/bin/env python3 +"""Benchmark file-search endpoints across every supported order. + +Usage: + python3 scripts/search_order_perf.py --host decay --label before + python3 scripts/search_order_perf.py --host decay --label after --passes 3 + +Writes CSV to ./perf_results/search_order__