perf(scalar): cache store-free state for the zone map index#7652
Open
Ali2Arslan wants to merge 1 commit into
Open
perf(scalar): cache store-free state for the zone map index#7652Ali2Arslan wants to merge 1 commit into
Ali2Arslan wants to merge 1 commit into
Conversation
Opening a zone map index cached the live `Arc<dyn ScalarIndex>`, which holds an `IndexStore` and therefore the session-scoped object store, metadata cache, and scheduler. Every cached index pinned those shared resources and mis-reported them as its own bytes under `deep_size_of` (cf. the discussion on lance-format#7574). Follow the existing `BitmapIndexState` / `single_flight_open` pattern: cache only a `ZoneMapIndexState` (the zone-statistics batch + zone size, persisted through a `CacheCodec`) and inject the store, cache, and fragment-reuse index at reconstruct time. `try_from_serialized` already rebuilds the index with no IO, so the reconstruct is free. The remaining live-object-cached scalar indices (ngram, rtree, inverted, json) can follow this same template in separate PRs. Co-authored-by: Cursor <cursoragent@cursor.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
2 tasks
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.
Summary
Opening a zone map index cached the live
Arc<dyn ScalarIndex>. AZoneMapIndexholds anIndexStore, which transitively pins the session-scoped object store, metadata cache, and scan scheduler. So every cached zone map index:deep_size_of— each opened index reported the whole shared cache as its own bytes, N-times double-counting it.This is the general problem raised in the #7574 review discussion: a cache entry should never hold a session-scoped resource.
This PR applies the fix to the zone map index by following the pattern already used by
BitmapIndex/BTreeIndex/LabelListIndex:ZoneMapIndexState— a serializable, store-free snapshot (the zone-statisticsRecordBatch+rows_per_zone), persisted through aCacheCodec.IndexStore, cache handle, and fragment-reuse index at reconstruct time.ZoneMapIndex::try_from_serializedalready rebuilds the index with no IO, so reconstruct is free.get_from_cache/put_in_cache/get_or_insert_in_cacheto route through the state viasingle_flight_open(coalesces concurrent cold opens).zones -> RecordBatchencoding into a sharedzones_to_batchhelper (used by both the on-disk writer and the cache codec), replacing the builder's inline copy.Behavior is unchanged; only the cached representation changes.
Follow-ups
The remaining scalar indices that still cache a live object holding an
IndexStore—ngram,rtree,inverted,json— can migrate onto this same template in separate, focused PRs.Test plan
test_zonemap_state_codec_roundtrip—CacheCodecserialize/deserialize is lossless and matches a fresh encode of the live index.test_zonemap_state_reconstruct_is_io_free— reconstruct against a store rooted at an empty dir (missing index file) still searches identically, proving no store IO on reconstruct/search.test_zonemap_state_deep_size_excludes_shared_resources— the cached state'sdeep_size_ofcounts only its batch (guards against re-adding a shared field).test_zonemap_plugin_cache_is_single_flight_state— end-to-end through the plugin: aZoneMapIndexState(not a live index) is cached, cold/warm opens coalesce onto a single load, and both opens search identically.cargo test -p lance-index --lib scalar::zonemap(22 passed) and fullscalar::suite (395 passed).cargo fmt --all+cargo clippy -p lance-index --tests -- -D warningsclean.Made with Cursor