fix(fts): merge inverted index segments across format versions - #8681
Closed
LuQQiu wants to merge 4 commits into
Closed
fix(fts): merge inverted index segments across format versions#8681LuQQiu wants to merge 4 commits into
LuQQiu wants to merge 4 commits into
Conversation
Merging FTS (inverted) index segments required every source segment to share the same `format_version` and `posting_tail_codec`, and the same for the params comparison (which includes `format_version`). After an upgrade that changes the default posting encoding, a pre-existing index (e.g. V1 / Fixed32) is maintained alongside newly written deltas (V2/V3 / VarintDelta). The two encodings can never match, so `merge_segments` failed deterministically with "cannot merge inverted index segments with different posting tail codecs" (or "different format versions"), and the maintenance retried forever. These guards were overly strict: a partition is decoded with its own codec when read into an in-memory builder, and `merge_from` folds already-decoded logical entries (doc id / frequency / positions) into the target builder, re-encoding them with the target codec. The source encoding is therefore irrelevant to whether two segments can be merged. Changes: - `InvertedIndex::merge_segments`: keep the `token_set_format` guard and a new `InvertedIndexParams::is_compatible_for_merge` check (which ignores the encoding-only `format_version` and the run-only `memory_limit_mb` / `num_workers`). Re-encode to the highest format version present and stamp the written details/index_version with that target instead of `first`'s, so readers pick the correct decoder. - `InvertedIndexBuilder::merge_existing_segments`: fold every source partition into a fresh builder created with the target format before merging, so its already-encoded blocks are decoded and re-encoded with the target codec regardless of segment order. - `InnerBuilder::merge_from`: drop the `format_version` / `posting_tail_codec` equality guards (still requires matching positions, token set format, and block size). Adds a test that merges a V1 and a V3 segment into a single V3 index and verifies documents from both remain searchable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
github-actions
Bot
added
A-index
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![lance-gatekeeper[bot]](https://avatars.githubusercontent.com/in/4433141?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The per-segment decode/re-encode mechanism is appropriate, but the output must retain the authoritative existing format. FTS maintenance is documented to preserve that format for staged rollouts; selecting the newest source instead silently changes the compatibility boundary.
Use the reference segment’s format as the normalization target, then stamp the merged physical metadata, details, and index version from that target.
| // format version also carries the resulting merged index version. | ||
| let target_segment = segments | ||
| .iter() | ||
| .max_by_key(|segment| segment.format_version().index_version()) |
Contributor
There was a problem hiding this comment.
Selecting the maximum here silently upgrades a maintained index, violating docs/src/guide/migration.md’s contract that append, incremental indexing, optimize, and mem-WAL maintenance preserve the existing FTS format. A deployment pinned to V1 for older-reader compatibility can therefore emit a V3 merged segment. I ran CARGO_TARGET_DIR=/home/agent/tmp/root-pr8681-target cargo test --locked -p lance-index test_merge_segments_across_format_versions -- --nocapture on this head after changing the expected result for the existing [v1_index, v3_index] case to the preserved reference format; it fails with left: 3, right: 1. Use the authoritative first/reference segment as the target, decode every other segment with its own codec, and stamp all output version metadata from that target.
lance-gatekeeper
Bot
added
the
K-changes
Address review of the cross-format merge: - merge_existing_segments only folds a source partition through a fresh target-format builder when its format_version/posting_tail_codec differ from the target. The common single-format optimize/delta-merge path uses the partition directly again, restoring zero re-encoding for it. - is_compatible_for_merge now destructures InvertedIndexParams so any field added later is a compile error until its merge relevance is decided, and drops the two full-struct clones. - merge_from binds format_version/posting_tail_codec as `_` in the destructure (with the rationale in a comment) instead of a trailing `let _ = (...)`. - merge_segments uses `.expect()` on the max_by_key over the already non-empty slice; inline the single-use target-builder. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Per docs/src/guide/migration.md, operations that maintain an existing FTS index (append, incremental indexing, optimize, mem-wal flush) must preserve its format so a deployment pinned to an older format for reader compatibility is not silently upgraded. The previous revision picked the highest format version among the source segments, which could upgrade a maintained V1 index to V3. Normalize every segment to the reference (`first`) segment's format instead: other segments are still decoded with their own codec and re-encoded to the reference format, and the written details / index_version are stamped from the reference. The reference segment itself stays on the zero-re-encode fast path. Test now asserts the output follows the reference format (parametrized over reference = V1 and reference = V3) rather than the newest input. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LuQQiu
marked this pull request as draft
August 21, 2026 00:02
lance-gatekeeper
Bot
removed
the
K-changes
Xuanwo
pushed a commit
that referenced
this pull request
Aug 21, 2026
## What is the bug? A distributed FTS rebuild can legitimately produce an intermediate segment with zero partitions when its assigned fragments contain no indexable tokens. `InvertedIndex::posting_tail_codec` currently falls back to the default `VarintDelta` codec for such a segment, even when its declared format is V1 / `Fixed32`. Merging that empty V1 segment with a populated V1 segment then fails with: ``` cannot merge inverted index segments with different posting tail codecs ``` This is the same-format empty-segment failure observed in [ENT-2323](https://linear.app/lancedb/issue/ENT-2323/lanceerrorindex-cannot-merge-inverted-index-segments-with-different). It is narrower than the cross-format behavior proposed in #8681. ## What issues or incorrect behavior does the bug cause? V1 distributed FTS rebuilds fail during `merge_existing_index_segments` before the replacement index can be committed. Automated maintenance can retry the same deterministic failure. ## How does this PR fix the problem? When an inverted index has no partitions, derive its posting-tail codec from the segment's declared FTS format instead of using the global codec default. Populated segments continue to use their physical partition metadata, and cross-format merge behavior is unchanged. ## Tests Adds a regression test that merges an empty V1 segment with a populated V1 segment in both input orders. It verifies that: - the empty segment resolves to `Fixed32` - the merged index remains V1 - the populated document remains searchable Local compilation and tests were intentionally skipped; CI is the validation path for this PR.
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.
Problem
Merging FTS (inverted) index segments required every source segment to share
the same
format_versionandposting_tail_codec, and theparamscomparison also included
format_version. When the default posting encodingchanges between releases, a pre-existing index (e.g. V1 /
Fixed32) getsmaintained alongside newly written delta segments (V2/V3 /
VarintDelta).The two encodings can never match, so
InvertedIndex::merge_segmentsfailsdeterministically:
(or the equivalent "different format versions" / "different parameters"), and
index maintenance retries the same merge forever.
In cases where the guard happens to pass but the codecs still differ, the
older large segment is decoded under the wrong layout and panics deeper down
with an arrow
offset overflowin the posting-tail decode path.Why the guards were too strict
A partition is always decoded with its own codec when read into an
in-memory builder (
InvertedPartition::into_builder), andmerge_fromfoldsalready-decoded logical entries (doc id / frequency / positions) into the
target builder, re-encoding them with the target codec. The on-disk source
encoding is therefore irrelevant to whether two segments can be merged — they
only need to agree on things that actually affect tokenization and the
token/posting layout (
params,token_set_format,block_size).Changes
InvertedIndex::merge_segments: replace theparamsequality check witha new
InvertedIndexParams::is_compatible_for_merge(ignores theencoding-only
format_versionand the run-onlymemory_limit_mb/num_workers); drop theformat_version/posting_tail_codecguards.Re-encode to the highest format version present, and stamp the written
details /
index_versionwith that target (notfirst's) so readers pickthe correct decoder.
InvertedIndexBuilder::merge_existing_segments: fold every sourcepartition into a fresh builder created with the target format before
merging, so a partition's already-encoded blocks are decoded and re-encoded
with the target codec regardless of segment order (a source partition can't
simply become the merge accumulator, since its
encoded_blockscarry the oldcodec).
InnerBuilder::merge_from: drop theformat_version/posting_tail_codecequality guards (still requires matching positions,token set format, and block size).
Test
Adds
test_merge_segments_across_format_versions: builds a V1 and a V3 segmentwith identical params/token-set-format, merges them, and asserts the result is
a single V3 index in which documents from both source segments remain
searchable. Full
lance-indexinverted suite (406 tests) passes;clippyclean.