make opening index size inspection O(1)#26
Closed
zehiko wants to merge 1 commit into
Closed
Conversation
|
ACTION NEEDED The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification. For details on the error please inspect the "PR Title Check" action. |
LanceIndexStore::deep_size_of_children counted object_store + metadata_cache, both Arc-shared process-wide infrastructure, not data the index owns. A cached BTreeIndex reaches this store via Arc, so every index-cache entry was charged the entire shared metadata cache: - mis-weighting: inflated/equalized index-cache entry weights so size-based eviction evicted index objects immediately and re-opened them every query (the production open_scalar_index churn); - cost: each index-cache insert walked the metadata cache (approx_size_bytes iterates all entries) -> O(metadata-cache size) per open. Count only owned data (index_dir + file_sizes). Mirrors BTreeIndex, which already skips its index_cache field.
531c10d to
416dd77
Compare
Member
Author
|
This path is no longer relevant after Lance lance-format#6222, see details in https://linear.app/rerun/issue/RR-4918/maven-slow-query-investigation |
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.
Problem
LanceIndexStore::deep_size_of_childrencountedobject_store+index_dir+metadata_cache.object_storeandmetadata_cacheareArcs to process-wide shared infrastructure, not data the index owns.A cached
BTreeIndexreaches itsLanceIndexStore(and thusmetadata_cache) viaArc, so moka's byte-weigher — which callsdeep_size_of()on every index-cache insert — walked the entire shared metadata cache to weigh a single index. Two consequences:LanceCache::deep_size_of_children→approx_size_bytes()iterates every entry). On a large shared cache this dominates CPU.In production this showed up as
open_scalar_indexon essentially every query, each paying the O(metadata-cache) walk.Fix
Count only data the store owns:
index_dir + file_sizes. This mirrorsBTreeIndex::deep_size_of_children, which already skips itsindex_cachefield to avoid circular references.It deliberately does not touch
approx_size_bytes/LanceCache::deep_size_of_children(noweighted_size), so it carries no telemetry-accuracy tradeoff — the per-open path simply never reaches the cache.Testing done
Microbenchmark timing the real
LanceIndexStore::deep_size_of()vs metadata-cache fill N (release-7.0.0vs this branch, same harness):O(N) → O(1), independent of cache fill.