perf: assemble stable-row-id prefilter allow lists from cached per-fragment pieces - #8880
perf: assemble stable-row-id prefilter allow lists from cached per-fragment pieces#8880JaySon-Huang wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The content-keyed per-fragment cache is a sound reuse boundary, but piece construction must preserve the existing async/CPU separation. Load row IDs and deletion vectors asynchronously, then run the clone, deletion masking, and tree construction for each miss on spawn_cpu; the final merge can remain on the CPU pool.
…agment pieces do_create_deletion_mask_row_id builds a whole-dataset allow list by loading every fragment's row id sequence and deletion vector, so the cost is proportional to the dataset size rather than to what changed. The result is cached under (manifest version, restrict set), which amortizes less than it looks: every commit invalidates the entry, distinct restrict sets each pay their own full rebuild, and the resident mask scales with the dataset. Assemble the allow list from per-fragment pieces instead. Each piece is one fragment's live stable row ids (row id sequence minus deletion vector), cached under its content identity: the row id generation (the same identity RowIdSequenceKey uses for generation-safe sequence caching) plus the deletion file identity. Pieces are OR-ed into the requested mask: - commits that leave a fragment's row ids and deletions untouched keep its piece warm, so a post-commit rebuild only reloads touched fragments - overlapping restrict sets share pieces instead of reloading them - content is unchanged: a piece is exactly what the old fold OR-ed in for that fragment, and missing fragments contribute nothing The whole-mask cache under (version, restrict_hash) is kept, so repeated identical queries still hit it unchanged.
a32381b to
1647231
Compare
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The code tree is unchanged from the prior reviewed revision, so the async/CPU boundary issue remains. The content-keyed per-fragment cache is a sound reuse boundary, but load row IDs and deletion vectors asynchronously, then run the clone, deletion masking, and tree construction for each miss on spawn_cpu; the final merge can remain on the CPU pool.
Loading a fragment's row id sequence and deletion vector stays async, but the sequence clone, deletion masking and tree construction for each piece are CPU-bound and were running inline on the async runtime. Move them into spawn_cpu, matching the whole-mask fold's original async/CPU separation while keeping the per-piece pipelining: each piece loads on the IO path and builds on the CPU pool independently, so IO of one piece overlaps CPU of another. The final merge stays on the CPU pool.
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The prior async/CPU boundary issue is fixed: row-ID and deletion-vector loading remains asynchronous, while each piece’s clone, deletion masking, and tree construction now run on the CPU pool. The content-identity cache and restricted-mask semantics remain intact, and the focused cross-commit invalidation test passes.
Fixes #8849 (assembled-from-pieces variant; see the issue for the whole-dataset allow-list cost analysis)
Problem
DatasetPreFilter::do_create_deletion_mask_row_idbuilds a whole-dataset allow list by loading every fragment'sRowIdSequenceand deletion vector and folding them into oneRowAddrTreeMap. The cost is proportional to the dataset size, not to what changed. The whole mask is cached under(manifest version, restrict_hash), which amortizes less than it looks:restrict_tosets get distinct cache entries, so a workload with varying fragment restrictions rebuilds repeatedly at the same version;Change
Assemble the allow list from per-fragment pieces. Each piece is one fragment's live stable row ids (row id sequence minus deletion vector, exactly what the old fold OR-ed in for that fragment), cached in the dataset metadata cache under its content identity:
RowIdMeta), the same identityRowIdSequenceKeyhas used since RowIdSequenceKey cache key lacks a dataset version/generation — stale hit across drop + recreate at the same URI #7645 — deletes and added columns leave it untouched;read_version/id/file_type/base_id, mirroringDeletionFileKey).Pieces are fetched concurrently and OR-ed on a blocking thread. The whole-mask cache under
(version, restrict_hash)is kept, so repeated identical queries still hit it unchanged.restrict_tosets share pieces instead of reloading them.Correctness
A piece's content is fully determined by its key identity: any change to a fragment's row ids or deletions changes
row_id_metaor the deletion file identity, so a stale piece cannot be served.mask(F) = OR(pieces(F ∩ manifest))is identical to the old restricted fold (#6563/#6877 semantics preserved).Tests: the existing prefilter mask tests (including the #6877 regression test) pass unchanged, plus a new test
test_row_id_allow_list_pieces_invalidate_on_new_deletionscovering cross-commit invalidation and restrict semantics. Full--libsuite: 3223 passed, 0 failed.Performance
Local NVMe, 1M rows / 100 fragments / 0.5% scattered deletions / IVF_FLAT index, warm caches:
On object storage the post-commit rebuild drops from ~N fragment loads (one round trip each) to only the touched fragments, which is where this mask cost dominates prefiltered searches on large tables.
Notes / follow-ups
FilterLoadercurrently does not expose fragment coverage.