perf(index): pack prewarmed FTS posting groups#7720
Conversation
📝 WalkthroughWalkthroughThe inverted index removes persisted posting-group boundaries, adds runtime token-count grouping, and introduces packed posting-list group storage with versioned cache serialization and legacy decoding support. Reader, prewarm, writer, memory-accounting, and end-to-end tests are updated accordingly. ChangesSynthetic posting groups
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Query
participant PostingListReader
participant PostingGrouping
participant PostingListGroup
Query->>PostingListReader: request token postings
PostingListReader->>PostingGrouping: calculate synthetic group range
PostingGrouping-->>PostingListReader: return group range
PostingListReader->>PostingListGroup: load packed group
PostingListGroup-->>PostingListReader: return posting view
PostingListReader-->>Query: return postings
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Claude Code Review
Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.
Tip: disable this comment in your organization's Code Review settings.
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-index/src/scalar/inverted/index.rs`:
- Around line 1259-1263: Document the PostingGrouping enum variants with concise
behavioral semantics: describe None as ungrouped legacy or empty partitions, and
SyntheticFixed as using a fixed cache group size; clarify that group_size is
measured in token rows, not posting bytes.
🪄 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: bd5a9f7d-4465-49e3-ba68-272221a1e4aa
📒 Files selected for processing (4)
rust/lance-index/src/scalar/inverted/builder.rsrust/lance-index/src/scalar/inverted/cache_codec.rsrust/lance-index/src/scalar/inverted/encoding.rsrust/lance-index/src/scalar/inverted/index.rs
💤 Files with no reviewable changes (1)
- rust/lance-index/src/scalar/inverted/encoding.rs
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rust/lance-index/src/scalar/inverted/index.rs (1)
3703-3843: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: de-duplicate the
POSTING_COLList<LargeBinary>extraction.The exact
column_by_name(POSTING_COL) → as_list_opt::<i32>() → ok_or_else(...)block (with an identical error message) is repeated innew_packed(Lines 3707-3714) andposting_list(Lines 3803-3811). A small private helper onPackedPostingListGroupwould keep the error text and cast in sync if either changes later.♻️ Example helper extraction
impl PackedPostingListGroup { fn postings_column(&self) -> Result<&GenericListArray<i32>> { self.batch .column_by_name(POSTING_COL) .and_then(|column| column.as_list_opt::<i32>()) .ok_or_else(|| { Error::index(format!( "packed posting group column {POSTING_COL} must be List<LargeBinary>" )) }) } }🤖 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-index/src/scalar/inverted/index.rs` around lines 3703 - 3843, Deduplicate the repeated POSTING_COL List<LargeBinary> extraction by adding a private postings_column helper on PackedPostingListGroup that performs the existing lookup, cast, and error construction. Update new_packed and posting_list to use this helper while preserving their current validation and behavior.
🤖 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.
Outside diff comments:
In `@rust/lance-index/src/scalar/inverted/index.rs`:
- Around line 3703-3843: Deduplicate the repeated POSTING_COL List<LargeBinary>
extraction by adding a private postings_column helper on PackedPostingListGroup
that performs the existing lookup, cast, and error construction. Update
new_packed and posting_list to use this helper while preserving their current
validation and behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0537d44b-95e2-4380-9ae0-b595a9136c8e
📒 Files selected for processing (1)
rust/lance-index/src/scalar/inverted/index.rs
Performance issue
No-position FTS prewarm currently decodes posting rows into a
Vec<PostingList>object graph. This substantially amplifies the index cache for workloads dominated by singleton and small posting lists.Measured before this change:
How this improves performance
This PR:
The persistent FTS index format and public API do not change.
prewarm_indexstill populates the posting cache for subsequent queries.Benchmark
Fresh
c4-standard-16VM, no-position format-v2 indexes created once by baseline Lance5dbd1400d, then reused unchanged by the target. The target uses the default runtime group size of 128. Each prewarm result is the mean of five runs.A 64/128/256 sweep selected 128: compared with 256 it halves cache-group granularity for a 2.36% cache increase on 10M unique terms and 0.29% on Wikipedia-40M. Size 64 increased the 10M cache by 7.07% and cold prewarm time by 30.8%.
Three repeated, prewarmed query runs at group size 128 showed no materialization regression. Mean QPS changed by -0.7% to +0.6% across the 10M unique-term and Wikipedia k=10/k=100 workloads, within run-to-run variance.
Validation
cargo fmt --all -- --checkcargo test -p lance-index scalar::inverted --lib --profile release-with-debug(197 passed)cargo clippy --all --tests --benches -- -D warningsindex_action=reusedandprewarm_with_position=falseAddresses OSS-1409.
Summary by CodeRabbit