perf(db): streamline gallery membership queries - #9385
Conversation
…mes query GET /api/v1/images/names fetches the full ordered name list for the virtualized gallery. The names query had no index matching its shape, so SQLite scanned idx_images_starred, probed board_images per row, and sorted every returned row in a temp B-tree on each request. Measured on the production method (SqliteImageRecordStorage .get_image_names, both statements, median of interleaved rounds, the client's default parameters): - 10k-image gallery: 8.7 ms -> 6.6 ms - 50k-image gallery: 55.6 ms -> 39.6 ms - 200k-image gallery: 239.7 ms -> 186.9 ms - 200k with starred_first=false: 798 ms -> 200 ms The two changes only work together: the covering index with the old LEFT JOIN query regresses 1.55x at 200k rows, and the anti-join without the index is neutral at 50k. Per-image insert overhead from the index is +76 us on a 3.2 ms insert transaction; the index adds ~3.1 MB per 50k images and builds once in 68 ms (50k) / 265 ms (200k).
lstein
left a comment
There was a problem hiding this comment.
Thanks for the unusually thorough writeup — the measurement discipline here is great, and the NOT EXISTS/EXISTS rewrite itself is correct. But I think the index is net negative as it stands, because it changes plans for two queries this PR doesn't touch, and the loss is bigger than the win.
All numbers below come from harnesses that build a real DB through init_db() (real schema, all existing indexes present), bulk-load N images, and toggle only idx_images_gallery_names between interleaved rounds.
Blocker — the index makes GET /boards/?all=true ~100× slower, more than cancelling the win
board_image_records_sqlite.get_image_count_for_board / get_asset_count_for_board still use the INNER JOIN images form. Once the covering index exists, SQLite stops driving from board_images (board_id=?) and instead scans the whole image_category/is_intermediate partition, probing board_images per row:
main: SEARCH board_images USING INDEX idx_board_images_board_id_created_at (board_id=?)
| SEARCH images USING INDEX idx_images_image_name (image_name=?)
PR: SEARCH images USING COVERING INDEX idx_images_gallery_names (image_category=? AND is_intermediate=?)
| SEARCH board_images USING INDEX sqlite_autoindex_board_images_1 (image_name=?)
The cost becomes proportional to total gallery size instead of board size. BoardService.get_all() calls both per board in a loop (boards_default.py:74-75), and BoardsList.tsx keeps useListAllBoardsQuery mounted — invalidated by the same events you list for the names query (app start, board switch, star, delete, move, upload).
200k images, 20 boards × 50 images each:
| request | main | this PR |
|---|---|---|
GET /boards/?all=true |
1.0 ms | 101.4 ms |
GET /images/names |
85.7 ms | 53.9 ms |
| net per app start | 86.7 ms | 155.3 ms |
Still net-negative at a more typical size — 20k images / 10 boards: 7.5 ms → 10.0 ms. Isolating one board at 200k images: a 20-image board's count query goes 0.01 ms → 15.2 ms.
This is exactly the failure mode your description documents ("the index alone regresses 1.55× because the planner keeps the join probe") — it just lands on a sibling query that wasn't rewritten. The stated invariant "don't ship the index without the query rewrite" is violated inside the PR itself.
Two fixes, both measured on the same DB:
- Add
ANALYZEto the migration. Board counts 99 ms → 0.2 ms for 20 boards, and the names query gets faster still (53.9 ms → 35.3 ms), because the planner stops guessing about the new index. One line, and it also softens the two findings below. Caveat: nothing in the codebase runsANALYZEtoday and stats go stale as the gallery grows, so it's worth pairing withPRAGMA optimizeon shutdown if you go this route. - Or rewrite the two count queries the same way
get_image_nameswas rewritten —EXISTSform, or aCROSS JOINto pin join order. Also 0.2 ms.
Major — the search_term path regresses ~1.8×
Gallery search flows through this same query. The index is chosen but is not covering (images.metadata is needed), so it trades a tiny temp B-tree — the LIKE had already eliminated nearly every row before the sort — for a much wider index scan plus a row lookup per row.
200k images, board=none, non-matching term: 15.2 ms → 26.8 ms (+76%). Reproduced on three separate fixtures (+83%, +76%, +27%, varying with metadata size). This is the interactive typing path.
Major — multiuser non-admin mode regresses on several toggles
get_image_names is always called with user_id=current_user.user_id. In single-user mode is_admin=True so the predicate is skipped — that's the configuration the PR measured. With multiuser enabled and a non-admin user, AND images.user_id = ? makes the index non-covering. 200k images:
| scenario | main | this PR |
|---|---|---|
images tab, board=none, DESC |
74.5 ms | 68.2 ms (−8%) |
starred_first=false |
63.4 ms | 73.6 ms (+16%) |
order_dir=ASC |
72.1 ms | 76.0 ms (+5%) |
search_term |
16.0 ms | 24.9 ms (+56%) |
Worth noting in the description that the QA step "plan shows SEARCH images USING COVERING INDEX" only holds for single-user/admin.
Minor
- The Assets tab keeps its temp B-tree.
ASSETS_CATEGORIESis four values, and theINon the leading index column blocks the ORDER-BY optimization —USE TEMP B-TREE FOR ORDER BYis still in the plan. It's still faster overall (covering scan), so not a regression, but "no temp B-tree" only holds for the single-category Images tab; the description and QA steps read as though it's universal. - No test pins the plan. The coupling between the two halves is documented only in prose. An
EXPLAIN QUERY PLANassertion would catch a future edit that reintroduces the join or the sort — and, extended to the sibling queries, would have caught the board-count hijack above. - The migration silently no-ops when
imagesis missing, then records itself as applied, so the index would never be created.depends_on="migration_1"guarantees the table exists, so the guard buys nothing and converts a real failure into silence. I'd drop it.
What holds up
- The rewrite is correct.
board_imagesisPRIMARY KEY (image_name)(migration_1.py:32), so at most one board row per image and the two forms are equivalent — including fortotal_count, which theLEFT JOINcould otherwise have inflated. I asserted result-set and order equality across 8 scenarios × both board modes × ASC/DESC ×starred_firston/off at 200k rows: zero mismatches,starred_countidentical throughout. - Migration ordering is sound.
Migration.sort_keyplaces graph-only migrations at(1, 0, id), after every legacy migration, so the index is built after any table rebuild, and2026_07_25_…correctly sorts after the two existing dated migrations. - Most collateral effects are positive:
get_many−67%,get_image_names_by_date−47%,get_intermediates_count−60%,get_all_board_image_names_for_board('none')−11%,get_image_datesneutral,get_most_recent_image_for_boardneutral. uv run pytest tests/app/services/image_records/ tests/app/services/shared/sqlite_migrator/→ 50 passed.
The shortest path to merge looks like adding ANALYZE to the migration: it turns the blocker into a non-issue and improves the target query further. Happy to share the benchmark scripts if useful.
Addresses query-plan feedback from invoke-ai#9385 (review). Co-Authored-By: Aiden <aiden@weco.ai>
|
Thanks for the detailed counterexamples. I rebased the repair onto current The updated branch keeps the covering index only for the common default image-name path. Explicit-board names and individual board counts are membership-first; search, non-admin, full-DTO, and other non-covering shapes use their category/user/table access paths instead. I used explicit plan shaping rather than On a real
The default full-name and app-start aggregates stayed within run spread. Complete ordered-result, count, pagination, ownership, image/video, board-summary, and migration-state digests matched before timing. The clean unpatched merge fails 16 of the new review controls; the repaired state passes all 20. Final focused and broader suites passed 52 and 79 tests, and a separate replay passed 62 tests with Ruff and formatting clean. |
lstein
left a comment
There was a problem hiding this comment.
Re-reviewed at 94b91a1242 against 01a8315ab6. The board-count blocker is fixed and the membership-first rewrites are a genuine win. But the plan-shaping layer that replaced ANALYZE now costs more than the index earns: on the endpoint the gallery actually calls, the index is worth ~0 and the hints regress five multiuser shapes by 19–27%.
Method. Two byte-identical DBs built through the real init_db() — 200k images (mixed categories, 5% intermediate, 2% starred, three users), 2k videos, boards of 20/200/5000/300 items — differing only in whether idx_images_gallery_names exists. 1157 (shape × method) combinations executed on both trees; every result digest compared; medians of 5 reps.
Confirmed fixed
get_image_count_for_board/get_asset_count_for_boardstay membership-first with the index present:SEARCH board_images USING INDEX idx_board_images_board_id_created_at (board_id=?) | SEARCH images USING INDEX idx_images_image_name (image_name=?)— identical to main. The 1.0 → 101 ms/boards/?all=trueregression is gone.- Explicit-board paths are dramatically better than main:
list_item_nameson a 20-image board 12.09 → 0.03 ms, on a 5000-image board 17.41 → 5.70 ms;list_items24.68 → 0.27 ms. - No behaviour change anywhere: 1157 runs, 0 result digest mismatches (ordered items, starred_count, total_count, board counts). 87 tests pass in
tests/app/services/{gallery,image_records,shared/sqlite_migrator}.
Blocker — the index earns nothing on the live endpoint; the rewrites earn everything
The gallery calls /gallery/items/names (useGetGalleryItemNamesQuery in use-gallery-image-names.ts). /images/names has no dispatch site left — appStarted.ts and GalleryImage.tsx both say so in comments. useListGalleryItemsQuery is @knipignore'd as "not used today".
Column 3 is your branch with the migration's index dropped and every INDEXED BY / NOT INDEXED stripped — i.e. the NOT EXISTS / EXISTS / CROSS JOIN rewrites alone:
/gallery/items/names shape |
main | this PR | rewrites only |
|---|---|---|---|
| images tab, no board (admin) | 722.8 | 712.3 | 721.3 |
| uncategorized (admin) | 681.9 | 691.4 | 684.1 |
| assets tab (admin) | 256.5 | 257.2 | 256.8 |
| tiny board (admin) | 12.09 | 0.03 | 0.03 |
| 5000-image board (admin) | 17.41 | 5.70 | 5.68 |
| tiny board (non-admin) | 12.58 | 0.03 | 0.03 |
| images tab ASC (non-admin) | 28.94 | 36.82 | 28.71 |
| search (non-admin) | 31.08 | 39.52 | 31.35 |
| uncategorized ASC (non-admin) | 29.51 | 37.03 | 29.65 |
| assets tab (non-admin) | 30.10 | 35.95 | 30.16 |
| assets uncategorized (non-admin) | 30.81 | 37.42 | 30.97 |
| no category filter (admin) | 797.7 | 1019.0 | 1017.5 |
Every board-scoped win is identical with the index dropped — they come from CROSS JOIN + INDEXED BY idx_images_image_name, not from idx_images_gallery_names. Every multiuser regression disappears in the same column. The single-user default gains 1.4% (10 ms of 712), which is inside run-to-run spread.
The 40+ ms wins you measured are real, but they land on image_records.get_image_names (−44% images tab, −56% tiny board) — the endpoint the frontend stopped calling.
Why the covering index can't move the live number: the union names query itself is 80 ms (execute + fetchall, 154k rows) out of a 712 ms call. The other ~88% is Python — one GalleryItemRef per row plus the starred_count pass in list_item_names. Even deleting the SQL entirely caps the win at ~11%.
So the index buys ≈0 on the live path, costs write amplification on every images insert/update/delete, and is the sole reason ~70 lines of hint logic exist in two files. I'd drop the migration and the whole base_index_hint / names_index_hint block, and ship the rewrites. That table's column 3 is the result: all the wins, none of the regressions, and gallery_default.py/image_records_sqlite.py stay readable.
Major — two of the hints are wrong on their own terms
If the hints do stay, two are defective independent of the above:
1. INDEXED BY idx_images_user_id is applied where no user_id predicate exists. In image_records_sqlite.get_image_names, has_non_admin_user_filter is computed as user_id is not None and not is_admin, but the WHERE builder only adds AND images.user_id = ? in the board_id == "none" and board_id is None branches — the explicit-board branch (image_records_sqlite.py:494) adds only the EXISTS. So non-admin + explicit board forces an index that constrains nothing:
main: SEARCH images USING INDEX idx_images_image_category (image_category=?) | SEARCH board_images ...
PR: SCAN images USING INDEX idx_images_user_id | SEARCH board_images EXISTS USING INDEX ...
A full 200k-row index scan with a row lookup each, to return 20 names. Measured 13.69 ms (main) → 16.45 ms (PR) → 6.19 ms with the hint stripped. The order_dir=ASC variant of the same shape becomes NOT INDEXED → SCAN images, a full table scan. gallery_default.py doesn't have this bug because its board branch is checked first; the two hint ladders have drifted apart. (Whether the explicit-board branch should filter by user is a separate, pre-existing question — but today it doesn't, so the hint is simply unbacked.)
2. The NOT INDEXED branch picks the worse of the two available plans. For non-admin + single category + starred_first + ASC, the ladder falls to NOT INDEXED while the adjacent branch uses INDEXED BY idx_images_user_id. On the 200k DB, same query, same params:
NOT INDEXED 16.9 ms SCAN images | USE TEMP B-TREE FOR ORDER BY
INDEXED BY idx_images_user_id 9.1 ms SEARCH images USING INDEX idx_images_user_id (user_id=?) | USE TEMP B-TREE
planner free (index present) 30.4 ms SEARCH images USING COVERING INDEX idx_images_gallery_names | USE TEMP B-TREE
The user_id hint fixes this shape too; the ASC carve-out costs 1.9×.
Similarly, elif categories is not None: INDEXED BY idx_images_image_category for non-admin multi-category overrides the planner's idx_images_user_id with something worse — list_items assets tab 13.67 → 24.35 ms (+78%), back to 13.24 ms with the hint stripped. That shape never touched the gallery index in the first place; the hint is a gratuitous override of a correct planner choice.
Major — INDEXED BY turns a missing index into an HTTP 500
INDEXED BY is a hard constraint, not a preference:
sqlite3.OperationalError: no such index: idx_images_starred
Both /gallery/items/names and /images/names wrap the call in except Exception: raise HTTPException(500), so a missing index means a blank gallery, not a slow one. The four named indexes aren't all unconditional: idx_images_starred is created in migration_1.py:145 only when the starred column was absent (it relies on the pre-migrator 3.x code having created it otherwise), and idx_images_user_id in migration_27.py:155 only when user_id was absent. Before this PR those were pure performance hints; now they're load-bearing. If the hints stay, the migration should CREATE INDEX IF NOT EXISTS every index it later names, so the invariant is enforced rather than assumed.
Minor
categories=None+board_id="none"regresses 28% (797.7 → 1019.0 ms), with or without the index or hints — it's the anti-join itself. main scansidx_images_starredand only needsUSE TEMP B-TREE FOR LAST 3 TERMS OF ORDER BY; the rewrite loses the starred-ordered scan and sorts everything. Low impact (the frontend always sendscategories), but it's reachable through the API and/virtual_boards/by_date/{date}/item_names, whosecategoriesdefaults toNone.- The plan tests can't fail for the right reason. They run on 1–3-row in-memory DBs and assert exactly the index each hint names, so they only break if the hint string changes — they can't detect that a forced index is slower than the planner's choice, which is what actually regressed here. Neither ladder's explicit-board × non-admin cell is covered, which is where defect 1 lives.
- Dropping the
images-table guard from the migration is the right call.
What holds up
- The
NOT EXISTS/EXISTS/CROSS JOINrewrites are correct: 1157 shape × method combinations across 4 board modes × 3 category sets × admin/non-admin × starred_first × ASC/DESC × search × is_intermediate, all digests identical to main,board_images/board_videosbothPRIMARY KEY (name)so the anti-join equivalence holds. CROSS JOINpinning join order for the two board counts is the right fix and is measurably stable with the index present.- Migration ordering and the
depends_on="migration_1"dependency are sound. - Choosing explicit plan shaping over
ANALYZEto avoid stats-freshness coupling is a defensible call — my objection is to the breadth of the shaping, not the principle.
Happy to share the harnesses (DB builder, the 1157-shape differ, and the three-way timing script).
Addresses reviewer feedback on invoke-ai#9385 by retaining the query-shape improvements while removing the forced planner hints and covering-index migration.
|
Thanks for the detailed counterexample. I removed the covering-index migration and the full INDEXED BY / NOT INDEXED hint layer, while retaining the EXISTS / NOT EXISTS and membership-first CROSS JOIN rewrites. I also rebased the branch by merging current main. The exact updated tree passes Ruff and 90 focused gallery, image-record, board-service, and SQLite migration tests. The query regression tests now verify that representative shapes emit no forced-index directives, so missing optional indexes cannot turn these endpoints into HTTP 500s. |
lstein
left a comment
There was a problem hiding this comment.
Re-reviewed at 42b1ec4d62 (fix commit d5316f43d8) against merge base 5e81b3ec09. You took the round-2 recommendation exactly: migration and both hint ladders deleted, rewrites kept. Every regression I measured last round is gone, and the board-path wins survive intact. This is good to merge — everything below is non-blocking, plus a correction to one of my own round-2 numbers.
Method. Same as round 2, but both trees now run against byte-identical copies of one DB (schemas are identical again, so no index variable): 200k images / 2k videos via the real init_db(), boards of 20/200/5k/300 plus purpose-built 10k/30k/60k/90k/150k boards. 1157 (shape × method) combinations on both trees with result digests compared; timings are medians of interleaved A/B reps in a single process against a single connection, which matters — see the correction.
Confirmed fixed
The hint layer is completely gone (grep -c "INDEXED BY" invokeai/ → 0), the migration and its test file are deleted, and _build_half lost the starred_first/order_dir params it only needed for hint selection. Every multiuser regression I reported disappears:
/gallery/items/names shape |
main | round 2 (hints) | now |
|---|---|---|---|
| images tab ASC (non-admin) | 29.54 | 36.82 | 28.41 |
| uncategorized ASC (non-admin) | 29.10 | 37.03 | 29.43 |
| assets tab (non-admin) | 29.46 | 35.95 | 29.74 |
| assets uncategorized (non-admin) | 29.91 | 37.42 | 31.04 |
| search (non-admin) | 30.76 | 39.52 | 30.57 |
And the board wins are unchanged, which was the point — they never came from the index:
| shape | main | now |
|---|---|---|
list_item_names, 20-image board |
12.01 | 0.03 |
list_item_names, 5000-image board |
16.88 | 5.76 |
list_items, 20-image board |
25.64 | 0.25 |
list_items, 5000-image board |
25.17 | 2.75 |
GET /boards/?all=true |
3.33 | 3.31 |
Correctness:
- 1157 shape × method combinations (4 board modes × 3 category sets × admin/non-admin ×
starred_first× ASC/DESC × search ×is_intermediate), 0 result-digest mismatches, 0 errors. - My round-2 digest only covered
(kind, name)forlist_items, so it would not have caught a wrongboard_id. Re-checked separately across 30 board × category × user combinations comparing everyGalleryItemfield —board_id,category,starred,is_intermediate,width,height,duration,fps,created_at— byte-identical to main. TheNULL AS board_idsubstitution forboard_id="none"is sound (theNOT EXISTSguarantees it). - 88 tests pass in
tests/app/services/{gallery,image_records,shared/sqlite_migrator}; ruff check + format clean; CI green;MERGEABLE.
Correction to my round-2 review
I reported categories=None + board_id="none" as a 28% regression (797.7 → 1019.0 ms) intrinsic to the anti-join. That doesn't reproduce. Timed at the SQL level with the two statements interleaved in one process against one connection, it is +3.0/+3.2/+3.3% across three runs. The round-2 numbers were separate-process measurements of a ~1-second call whose run-to-run spread is wider than the effect; this run they landed the other way round (main 1005.75, PR 792.12). Sorry for the noise — that item should not have been in the review.
Non-blocking
1. The board-count CROSS JOIN is now a no-op, and its comment points at a deleted index.
Those two hunks in board_image_records_sqlite.py existed only to stop idx_images_gallery_names from making images the outer loop. With the index gone, INNER JOIN and CROSS JOIN produce an identical plan and identical timings at every board size:
board size count main(INNER) PR(CROSS) delta
b_tiny 20 assets 0.01 0.01 1.7%
b_med 5000 assets 1.20 1.21 0.5%
b_10k 10000 assets 2.03 2.06 1.4%
b_30k 30000 assets 6.12 6.19 1.0%
b_60k 60000 assets 12.85 13.03 1.4%
b_90k 90000 assets 19.54 19.79 1.3%
The comment still reads "instead of allowing the gallery index to make images the outer loop for this query" — there is no gallery index any more. I'd revert both hunks (they're unrelated to what the PR now does), or at minimum rewrite the comment.
2. The explicit-board rewrite regresses on very large boards with a narrow category filter. Membership-first is a fixed choice, so its cost scales with board size while main's scales with how many rows the category filter matches globally. Same query, list_item_names, one board, varying size:
board size cat rows main ms PR ms delta
b_tiny 20 mask 0 7.51 0.02 -99.8%
b_med 5000 mask 237 6.26 1.04 -83.4%
b_10k 10000 mask 441 6.43 2.05 -68.1%
b_30k 30000 mask 1422 6.80 6.15 -9.5%
b_60k 60000 mask 2845 7.22 12.66 +75.4%
b_90k 90000 mask 4264 7.85 19.19 +144.3%
b_150k 150000 mask 7066 8.33 32.16 +286.3%
b_90k 90000 general 68469 58.41 43.09 -26.2% (wide filter: still a win)
Crossover is around a board holding ~30% of the library while the Assets tab (5% of rows) is selected. Worth knowing: this is not caused by the CROSS JOIN keyword — a plain INNER JOIN with the planner free picks membership-first too, at every size I tested (b_med 1.12 vs 1.13 ms; b_90k 19.22 vs 18.76 ms). It's intrinsic to converting the LEFT JOIN into an inner join, which is the whole point of the change. Given how lopsided the win is at realistic board sizes, accepting this is the right call — I'd just want it on the record rather than discovered later.
3. board_id="none" gets slightly slower, and it's the one rewrite with no upside. The join-drop that makes the names path fast applies to board_id is None, not to "none" — the Uncategorized board still has to consult board_images either way. Measured, three runs, interleaved:
case main PR delta
names: cats=None, board=none 118.1 122.0 +3.2%
items page: cats=general, board=none 31.6 34.1 +8.2%
items count(images): cats=general, board=none 21.9 24.0 +10.2%
items count(images): cats=None, board=none 22.0 25.0 +13.3%
Consistent across runs with min tracking median, so it's the correlated subquery rather than noise. Small in absolute terms, and it does make the SQL more uniform. Keeping LEFT JOIN … IS NULL for the "none" branch and using NOT EXISTS/EXISTS only where they buy something would be marginally faster; entirely your call.
4. Housekeeping. Anyone who ran an earlier revision of this branch has 2026_07_25_gallery_name_list_index in applied_migrations, and _validate_existing_applied_migrations now refuses to open that DB on both this branch and main:
MigrationError: Database contains unknown applied migration IDs: 2026_07_25_gallery_name_list_index
The migration never shipped, so no user is affected — but testers are. Worth a line in the PR description: DELETE FROM applied_migrations WHERE migration_id = '2026_07_25_gallery_name_list_index';
Also cosmetic: test_paginated_item_path_keeps_result_shape_and_avoids_gallery_index still names the deleted index, and test_name_shapes_do_not_force_indexes / test_query_shapes_do_not_force_indexes keep a 4-way parametrize whose parameters no longer affect the assertions. The assertions themselves are a reasonable guard against the hints creeping back.
What holds up
NOT EXISTS/EXISTS/inner-join rewrites are correct: 1157 shapes, 0 digest mismatches, full-field equality onlist_items, and the anti-join equivalence is sound becauseboard_images/board_videosare bothPRIMARY KEY (name).- Dropping the membership join entirely on the names path when no board is requested is the right shape — it's a wash at 200k rows on this hardware (85.91 → 86.30 ms at SQL level) but it removes work that scales with the library.
test_explicit_board_starts_from_mixed_membershipandtest_board_image_count_starts_from_membershipassert plan ordering rather than a specific index name, so they'll survive schema changes and still catch a regression to images-first. Those are the two tests carrying real weight here.- Thanks for taking the teardown on the chin — the diff is now three source files and ~30 net lines, and it's much easier to reason about than the version with the hint ladders.
Summary
This updates the gallery query shapes without adding a schema migration or forcing SQLite planner choices:
LEFT JOIN; the unboarded sentinel usesNOT EXISTS./images/namescompatibility path uses the sameEXISTS/NOT EXISTSmembership filtering and preserves non-admin user isolation.INDEXED BYorNOT INDEXEDdirectives.The implementation retains deterministic mixed image/video ordering and the existing result, count, and pagination behavior.
Supplementary autoresearch trajectory:
https://dashboard.weco.ai/share/DK5RIpsJirqAqrB8ty4dhZCMF5K6SN4H
Related Issues / Discussions
Validation
uvx ruff check --fix .Checklist