feat: add cache summary diagnostics#7818
Open
jackye1995 wants to merge 2 commits into
Open
Conversation
📝 WalkthroughWalkthroughThe cache APIs now expose optional entry records with weighted sizes. The Moka backend supplies these records, and ChangesCache diagnostics
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Session
participant LanceCache
participant MokaCacheBackend
Session->>LanceCache: request entry_records()
LanceCache->>MokaCacheBackend: request prefixed inventory
MokaCacheBackend-->>LanceCache: return CacheEntryRecord iterator
LanceCache-->>Session: return filtered records
Session->>Session: aggregate counts, sizes, and components
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rust/lance-core/src/cache/moka.rs (1)
175-182: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse the same bounded weight for approximate accounting.
approx_size_bytes()sums uncapped logical sizes, while eviction,size_bytes(), and inventory records cap each entry atu32::MAX. Large entries therefore produce conflicting cache-size diagnostics.Proposed fix
self.cache .iter() - .map(|(key, entry)| logical_entry_size(key.as_ref(), entry.size_bytes)) + .map(|(key, entry)| entry_weight(key.as_ref(), entry.size_bytes)) .sum()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/lance-core/src/cache/moka.rs` around lines 175 - 182, Update approx_size_bytes to apply the same u32::MAX cap used by eviction, size_bytes(), and inventory accounting when summing each logical entry size. Preserve the existing synchronous cache iteration and ensure the resulting approximate total is based on bounded per-entry weights.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rust/lance-core/src/cache/backend.rs`:
- Around line 28-30: Complete public API documentation across
rust/lance-core/src/cache/backend.rs:28-30, 73-83, and 145-152;
rust/lance-core/src/cache/mod.rs:289-301; and rust/lance/src/session.rs:26-69
and 308-349. Add compiling examples and relevant intra-doc links using the
actual CacheEntryRecord, inventory methods, backend implementation, summary
types, and current method signatures; document unsupported inventory behavior,
prefix-filtered record collection, relationships among the three summary levels,
and index/metadata summary usage.
In `@rust/lance/src/session.rs`:
- Around line 369-400: Update summarize_cache_records to return a fallible
result, such as Result<Option<CacheSummary>>, and replace every saturating_add
used for entry counts and byte totals with checked arithmetic. Propagate
overflow as a descriptive contextual error, including both aggregate and
per-component/group accumulation, and update callers to handle the new result
instead of silently producing a clamped summary.
- Around line 605-712: The new tests do not follow repository conventions for
test URIs and parameterized cases. In test_session_cache_summaries, replace the
dataset URI with plain "memory://"; convert
test_cache_key_component_names_are_compact into an rstest parameterized test
using readable named #[case::{name}(...)] cases for each input and expected
component.
---
Outside diff comments:
In `@rust/lance-core/src/cache/moka.rs`:
- Around line 175-182: Update approx_size_bytes to apply the same u32::MAX cap
used by eviction, size_bytes(), and inventory accounting when summing each
logical entry size. Preserve the existing synchronous cache iteration and ensure
the resulting approximate total is based on bounded per-entry weights.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c4ff2ba9-15c0-4f0e-b56f-8d5fb0c4624e
📒 Files selected for processing (4)
rust/lance-core/src/cache/backend.rsrust/lance-core/src/cache/mod.rsrust/lance-core/src/cache/moka.rsrust/lance/src/session.rs
Comment on lines
+28
to
+30
| /// Iterator over cache entries currently known to a backend. | ||
| pub type CacheEntryRecordIterator<'a> = Box<dyn Iterator<Item = CacheEntryRecord> + Send + 'a>; | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Complete documentation for the new public cache-diagnostics surface.
The new public APIs have descriptive prose but lack required examples and cross-links.
rust/lance-core/src/cache/backend.rs#L28-L30: add an iterator usage example and linkCacheEntryRecord.rust/lance-core/src/cache/backend.rs#L73-L83: demonstrate record inspection and link the inventory methods.rust/lance-core/src/cache/backend.rs#L145-L152: document backend implementation and unsupported-inventory behavior.rust/lance-core/src/cache/mod.rs#L289-L301: demonstrate collecting prefix-filtered records.rust/lance/src/session.rs#L26-L69: cross-link the three summary levels and show their relationships.rust/lance/src/session.rs#L308-L349: add compiling index and metadata summary examples.
As per coding guidelines, “Document all public APIs with examples and links to relevant structs and methods; keep examples synchronized with actual signatures.”
📍 Affects 3 files
rust/lance-core/src/cache/backend.rs#L28-L30(this comment)rust/lance-core/src/cache/backend.rs#L73-L83rust/lance-core/src/cache/backend.rs#L145-L152rust/lance-core/src/cache/mod.rs#L289-L301rust/lance/src/session.rs#L26-L69rust/lance/src/session.rs#L308-L349
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rust/lance-core/src/cache/backend.rs` around lines 28 - 30, Complete public
API documentation across rust/lance-core/src/cache/backend.rs:28-30, 73-83, and
145-152; rust/lance-core/src/cache/mod.rs:289-301; and
rust/lance/src/session.rs:26-69 and 308-349. Add compiling examples and relevant
intra-doc links using the actual CacheEntryRecord, inventory methods, backend
implementation, summary types, and current method signatures; document
unsupported inventory behavior, prefix-filtered record collection, relationships
among the three summary levels, and index/metadata summary usage.
Source: Coding guidelines
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rust/lance/src/session.rs`:
- Around line 473-483: Update checked_cache_summary_add to return the internal
error variant instead of Error::invalid_input for checked_add overflow,
preserving the existing diagnostic message. Add an overflow test for this helper
that asserts both the internal error variant and the message content.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6bbc390f-ae29-4c5c-9feb-5058fc270baa
📒 Files selected for processing (4)
rust/lance-core/src/cache/backend.rsrust/lance-core/src/cache/mod.rsrust/lance-core/src/cache/moka.rsrust/lance/src/session.rs
Comment on lines
+473
to
+483
| fn checked_cache_summary_add( | ||
| current: usize, | ||
| increment: usize, | ||
| context: impl AsRef<str>, | ||
| ) -> Result<usize> { | ||
| current.checked_add(increment).ok_or_else(|| { | ||
| Error::invalid_input(format!( | ||
| "Cache summary overflow while adding {increment} to {} (current value: {current})", | ||
| context.as_ref() | ||
| )) | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Classify summary overflow as an internal diagnostic failure.
These values come from backend inventory, so Error::invalid_input incorrectly blames the caller. Return an internal error and add an overflow test asserting both variant and message.
Proposed fix
- Error::invalid_input(format!(
+ Error::internal(format!(
"Cache summary overflow while adding {increment} to {} (current value: {current})",
context.as_ref()
))As per coding guidelines, “Match the Error variant to the root cause” and “Assert on both the error variant and the message content in tests.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fn checked_cache_summary_add( | |
| current: usize, | |
| increment: usize, | |
| context: impl AsRef<str>, | |
| ) -> Result<usize> { | |
| current.checked_add(increment).ok_or_else(|| { | |
| Error::invalid_input(format!( | |
| "Cache summary overflow while adding {increment} to {} (current value: {current})", | |
| context.as_ref() | |
| )) | |
| }) | |
| fn checked_cache_summary_add( | |
| current: usize, | |
| increment: usize, | |
| context: impl AsRef<str>, | |
| ) -> Result<usize> { | |
| current.checked_add(increment).ok_or_else(|| { | |
| Error::internal(format!( | |
| "Cache summary overflow while adding {increment} to {} (current value: {current})", | |
| context.as_ref() | |
| )) | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rust/lance/src/session.rs` around lines 473 - 483, Update
checked_cache_summary_add to return the internal error variant instead of
Error::invalid_input for checked_add overflow, preserving the existing
diagnostic message. Add an overflow test for this helper that asserts both the
internal error variant and the message content.
Source: Coding guidelines
wjones127
self-requested a review
July 16, 2026 06:05
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
1 participant
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.
Adds read-time cache entry summary APIs for index and metadata caches, backed by optional cache entry inventory in Lance core.
Summaries aggregate by cache group, component, type, entry count, and weighted size so callers can avoid dumping raw cache keys.
Index cache components
Index cache summaries group entries by index cache prefix. For a normal index this is the dataset URI plus index UUID; when fragment-reuse state is involved the prefix also includes the fragment-reuse UUID. Within each index group, the component field is a compact category instead of the raw cache key.
Tracked index-cache components include:
ivf: IVF partition and sub-index entries from keys likeivf-*.page: page-oriented scalar index entries from keys likepage-*.postings: inverted-index posting entries from keys likepostings-*.posting-list: ngram posting-list entries from keys likeposting-list-*.posting-metadata: inverted-index posting metadata from keys likeposting-metadata-*.positions: inverted-index position entries from keys likepositions-*.frag_reuse: fragment-reuse index entries from path-like keys such asfrag_reuse/<uuid>.type: scalar index detail entries from path-like keys such astype/<uuid>.version: index metadata entries keyed by version numbers.Example output
The Rust API returns
CacheSummarystructs. These examples show representative JSON-shaped renderings with illustrative counts and sizes.Vector index
{ "total_entries": 4, "total_size_bytes": 184320000, "groups": [ { "cache_prefix": "s3://bucket/dataset/11111111-1111-1111-1111-111111111111", "entry_count": 4, "size_bytes": 184320000, "components": [ { "component": "ivf", "type_name": "lance::index::vector::ivf::v2::PartitionEntry<...>", "entry_count": 4, "size_bytes": 184320000 } ] } ] }FTS / inverted index
{ "total_entries": 153, "total_size_bytes": 73400320, "groups": [ { "cache_prefix": "s3://bucket/dataset/22222222-2222-2222-2222-222222222222", "entry_count": 153, "size_bytes": 73400320, "components": [ { "component": "postings", "type_name": "PostingList", "entry_count": 96, "size_bytes": 50331648 }, { "component": "posting-metadata", "type_name": "PostingMetadata", "entry_count": 48, "size_bytes": 3145728 }, { "component": "positions", "type_name": "Position", "entry_count": 9, "size_bytes": 19922944 } ] } ] }Scalar index
{ "total_entries": 35, "total_size_bytes": 28311552, "groups": [ { "cache_prefix": "s3://bucket/dataset/33333333-3333-3333-3333-333333333333", "entry_count": 35, "size_bytes": 28311552, "components": [ { "component": "page", "type_name": "BTreePage", "entry_count": 32, "size_bytes": 25165824 }, { "component": "BTreeIndexState", "type_name": "BTreeIndexState", "entry_count": 1, "size_bytes": 2097152 }, { "component": "type", "type_name": "ScalarIndexDetails", "entry_count": 1, "size_bytes": 65536 }, { "component": "frag_reuse", "type_name": "FragReuseIndex", "entry_count": 1, "size_bytes": 983040 } ] } ] }Summary by CodeRabbit
New Features
Bug Fixes