perf(backend): materialize virtual collection membership#3886
Open
gantoine wants to merge 1 commit into
Open
Conversation
virtual_collections was a view that unnested five JSON arrays per rom on every read, so both the collection list and opening a single collection scanned the whole library. Opening a collection with a handful of roms cost as much as one with thousands, since the name is computed inside the view and nothing can be narrowed down early. Membership now lives in virtual_collection_roms, one narrow row per (type, name, rom_id) plus that rom's covers, derived from the generated_* columns and maintained by triggers on roms (deletions ride the foreign key cascade). The view becomes a plain aggregate over that table, so the model, schemas and API shape are unchanged, and filtering roms by a virtual collection is an indexed subquery instead of an IN list of every member id. Covers are no longer aggregated in the view, which materialized every member's cover path on every request; the handler resolves at most five per collection, matching the smart collection cap and what both UIs render. On a synthetic 50k-rom library the list goes from 1.42s to 0.128s and a single collection from 0.111s to 0.0006s, with identical output. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Greptile SummaryThis PR materializes virtual collection membership to speed up collection reads. The main changes are:
Confidence Score: 5/5This looks mergeable with a small data-integrity hardening change.
backend/alembic/versions/0100_virtual_collection_roms.py Important Files Changed
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
backend/alembic/versions/0100_virtual_collection_roms.py:74
**Truncation Merges Distinct Collections**
When two provider values share their first 400 characters, `LEFT(j.value, 400)` stores both under the same primary-key name. Their ROM memberships are then combined into one virtual collection, so callers receive incorrect collection names and membership instead of two distinct collections.
Reviews (1): Last reviewed commit: "perf(backend): materialize virtual colle..." | Re-trigger Greptile |
| ) | ||
| source = f"roms r CROSS JOIN {json_table}" if from_roms else json_table | ||
| branches.append( | ||
| f"SELECT DISTINCT {row}.id, '{type_}', LEFT(j.value, {NAME_MAX_LENGTH}), " # nosec B608 |
Contributor
There was a problem hiding this comment.
Truncation Merges Distinct Collections
When two provider values share their first 400 characters, LEFT(j.value, 400) stores both under the same primary-key name. Their ROM memberships are then combined into one virtual collection, so callers receive incorrect collection names and membership instead of two distinct collections.
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/alembic/versions/0100_virtual_collection_roms.py
Line: 74
Comment:
**Truncation Merges Distinct Collections**
When two provider values share their first 400 characters, `LEFT(j.value, 400)` stores both under the same primary-key name. Their ROM memberships are then combined into one virtual collection, so callers receive incorrect collection names and membership instead of two distinct collections.
How can I resolve this? If you propose a fix, please make it concise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #3881.
virtual_collectionswas a view that unnested five JSON arrays per rom on every read, so both the collections list and opening a single collection scanned the whole library. The second case is the surprising one: the collection name is computed inside the view, so nothing can be narrowed down early and a collection with 5 roms costs the same as one with 5,000. #3826 sped uproms_metadatawith storedgenerated_*columns but left this view deriving on read; this gives it the same treatment.Membership now lives in a real table,
virtual_collection_roms, with one narrow row per (type,name,rom_id) plus that rom's cover paths. It is backfilled once from thegenerated_*columns and kept in sync by triggers onroms(virtual_collection_roms_ai/_auon MariaDB/MySQL,virtual_collection_roms_aiucallingromm_sync_virtual_collection_roms()on PostgreSQL); rom deletions ride the foreign key's cascade. No application hook and no background job, same as 0098. The update trigger short-circuits unless a tracked column actually changed, so ordinary rom writes cost nothing extra.virtual_collectionsis redefined as a plain aggregate over that table, so theVirtualCollectionmodel, the response schemas and the API shape are unchanged. Filtering roms by a virtual collection is now an indexed subquery instead of anINlist of every member id.Behavior changes worth calling out
SMART_COLLECTION_MAX_COVERS). Aggregating every member's cover was megabytes per request on a large library, and both v1 and v2 slice to 4.Autogenerated <name> collection, like MariaDB's. They previously interpolated the type on PostgreSQL and the name on MariaDB.Performance
Synthetic 50k-rom library on MariaDB 11.3, old view recreated side by side for comparison:
The single-collection lookup is an index lookup on
(type, name)on both engines (confirmed withEXPLAIN), so it no longer scales with library size. The synthetic metadata blobs here are far smaller than the ~1 GB roms table in the report, so real-world absolute numbers should improve much more than these.Equivalence was checked against the old view on the same dataset: identical set of collections (2848 on both) and identical
rom_idson sampled rows.Testing
test_saves.py/test_collection.pycomparing a non-UTC local zone against UTC; CI runs UTC).alembic upgrade/downgrade/ re-upgradeverified on both engines.Checklist
AI assistance disclosure
This change was written with AI assistance (Claude Code). The design (trigger-maintained membership table, capped covers), implementation, migration, tests and benchmarks were produced with the agent and reviewed by me; all timings and the old/new equivalence check were run against local MariaDB and PostgreSQL instances.