cherry-pick: lance#6711: optimize_indices no-op on steady state#23
Merged
Conversation
…rrived (lance-format#6711) - Make `optimize_indices()` a no-op when no fragment is unindexed and the caller hasn't asked for `retrain` or `num_indices_to_merge >= 1`. Without this, a steady-state dataset (everything already indexed) had every call rewrite identical scalar-index files and trigger a vector single-segment auto-rebalance, producing a new manifest, transaction file, and per-index files on every call. - Adds a Python regression test that builds one of every index type (BTREE, BITMAP, LABEL_LIST, INVERTED, NGRAM, ZONEMAP, BLOOMFILTER, IVF_PQ), appends a fragment, runs `optimize_indices()` twice, and asserts the second call writes zero new files under the dataset directory. Two independent paths were both writing files when there was nothing to do: 1. **Scalar indices** — `merge_indices` in `rust/lance/src/index/append.rs` always called `index.update(...)` even when `unindexed.is_empty()`, producing a new UUID directory with identical contents. 2. **Vector indices** — the single-segment auto-rebalance from lance-format#6402 fires when `num_segments > 1 && num_indices_to_merge.is_none() && !retrain && unindexed.is_empty()`. After a prior `optimize_indices()` had folded a delta into its own segment, every subsequent call re-rebalanced one segment. The user-visible symptom was that calling `optimize_indices()` in a loop kept bumping the dataset version and creating index files indefinitely, even when the dataset hadn't changed. The fix lives in `Dataset::optimize_indices` in `rust/lance/src/index.rs`. Before calling `merge_indices` for each grouped delta set, we now skip the group when: - the caller did not pass `retrain = true`, **and** - `num_indices_to_merge` is `None` or `Some(0)` (no explicit delta consolidation requested), **and** - every fragment in the dataset is already covered by the union of the group's existing `fragment_bitmap`s. The bitmap union is computed from the metadata we already loaded (no extra IO). Pre-0.8 indices without a `fragment_bitmap` are treated conservatively — they still go through the merge path so they pick up the new format on first optimize. If any index group has work to do, the rest of the function is unchanged; the new check is purely a `continue`. When every group is skipped, the existing `if new_indices.is_empty() { return Ok(()); }` short-circuit prevents any commit, so no manifest or transaction file is written. - [x] New Python test `test_optimize_indices_second_call_is_noop` passes - [x] `python/tests/test_optimize.py`, `python/tests/test_scalar_index.py`, `python/tests/test_vector_index.py` — 225 passed, 10 skipped (pre-existing), 1 deselected (unrelated pre-existing failure in `test_create_index_progress_callback_error_before_completion_propagates`) - [x] `cargo test -p lance --lib index::` — 389 passed, 1 ignored (the auto-rebalance test noted above) - [x] `cargo clippy --lib --tests -- -D warnings` — clean - [ ] CI green - The fragment-bitmap-only check intentionally skips unioning bitmaps from system indices (`__lance_frag_reuse`, `__lance_mem_wal`); those are filtered out before this loop by the existing `is_system_index` check. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
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. |
andrea-reale
changed the title
zehiko
approved these changes
May 13, 2026
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
3 participants
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.
Cherry-pick of upstream lance-format/lance#6711 (commit
374d58f2) intorelease-3.0.0. See upstream PR and this discord thread for rationale.Related
RR-2364
Conflict
Upstream patch splits into two parts:
rust/lance/src/index.rs— short-circuits scalar index groups when every fragment is covered. Applied cleanly.rust/lance/src/index/append.rs— patches a vector arm that referenceslogical_index,ivf_view, andselect_segment_for_single_rebalance.Part 2 conflicted on cherry-picl because those symbols come from upstream PR #6402 (
feat: optimize one segmented vector segment per run) which is not inrelease-3.0.0. Git lined the hunk up textually against our scalar arm, producing nonsensical markers.Resolution
Kept our scalar arm unchanged.
Replaced the upstream vector hunk with a simplified guard at the top of the vector arm in
merge_indices_with_unindexed_frags:Dropped the
select_segment_for_single_rebalance/use_single_segment_rebalancereferences (require feat: optimize one segmented vector segment per run lance-format/lance#6402).Kept the new regression test, adjusted docstring to match.
Why safe
optimize_vector_indices) is fully covered by the simpler guard above.OptimizeOptions::append()semantics (num_indices_to_merge = Some(0)) handled identically.