Skip to content

fix(fts): split tail-partition merge by the worker memory budget#7601

Merged
BubbleCal merged 4 commits into
mainfrom
yang/fts-tail-partition-budget-split
Jul 6, 2026
Merged

fix(fts): split tail-partition merge by the worker memory budget#7601
BubbleCal merged 4 commits into
mainfrom
yang/fts-tail-partition-budget-split

Conversation

@BubbleCal

@BubbleCal BubbleCal commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Two fixes for FTS tail partitions:

  • merge_all_tail_partitions folded every leftover worker builder into a
    single partition unconditionally. On builds whose workers never hit the
    flush threshold (large LANCE_FTS_PARTITION_SIZE), the entire index
    collapsed into one partition and queries lost all intra-query
    parallelism. Fold with the same memory/doc-count budget as the
    partition-merge path, so the final partition count converges to
    total builder memory / budget regardless of worker layout.
  • Tail partitions hold most of the index when workers rarely flush;
    writing them sequentially serialized posting-list compression behind one
    producer at the end of the build. Write them through buffer_unordered.

Standalone verification vs its merge-base

5M-doc controlled build, LANCE_FTS_PARTITION_SIZE=128, 96 workers,
per-branch-tip wheels:

  • Leftover worker tails: the unconditional fold produced 1 collapsed
    tail partition (25 partitions total including flushed ones); with this fix
    the same tails split into 23 budget-sized partitions (47 total).
    Posting bytes essentially unchanged (3.59 vs 3.60 GB).
  • Build wall time 146.6s → 135.2s (−8%) from the concurrent tail writes even
    at this small scale; on the 200M-doc build, where tails hold most of the
    data, the build went 2.5h → 2.0h.

Independent of the block-size/impact PR stack.

🤖 Generated with Claude Code

Yang Cen added 2 commits July 3, 2026 12:57
merge_all_tail_partitions folded every leftover worker builder into a single
partition unconditionally; on a build whose workers never hit the flush
threshold (large LANCE_FTS_PARTITION_SIZE), the entire index collapsed into
one partition and queries lost all intra-partition parallelism. Fold with
the same memory/doc-count checks as the partition-merge path instead, so the
final partition count converges to total builder memory / budget.
Tail partitions hold most of the index when workers rarely hit the flush
threshold; writing them sequentially serialized the posting-list compression
of nearly the whole index behind one producer thread at the end of the
build. Write them through buffer_unordered instead.
@github-actions github-actions Bot added A-index Vector index, linalg, tokenizer bug Something isn't working labels Jul 3, 2026
@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.89340% with 14 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
rust/lance-index/src/scalar/inverted/builder.rs 92.89% 4 Missing and 10 partials ⚠️

📢 Thoughts on this report? Let us know!

@BubbleCal BubbleCal marked this pull request as draft July 5, 2026 17:51
@BubbleCal BubbleCal marked this pull request as ready for review July 6, 2026 06:21

@Xuanwo Xuanwo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before merging, please make the partition-shape change safe for fuzzy queries. This PR increases the number of partitions in tail-heavy builds, but fuzzy expansion currently applies max_expansions inside each partition and then unions the expanded terms across partitions. That means the same query can expand to more terms, and return different results, solely because the tail was split.

We should either enforce the cap globally after combining partition expansions or explicitly change the contract, and add a regression test that builds multiple tail partitions and verifies fuzzy query behavior.

Review follow-up: build the same corpus as one partition and as a
tail-split multi-partition layout through the real worker/budget path,
and assert fuzzy OR/AND queries return identical (row_id, score) sets
while max_expansions is not the binding constraint.
@BubbleCal

Copy link
Copy Markdown
Contributor Author

Thanks for the catch — the mechanism is exactly as you described: max_expansions was applied inside each partition and the picks were unioned, so the effective cap scaled with the partition count and this PR's tail split would have widened fuzzy queries.

Handled in two parts:

  • Regression test here (test_tail_partition_split_preserves_fuzzy_results): the same corpus built as one partition and as a tail-split multi-partition layout through the real worker/budget path must return identical fuzzy (row_id, score) sets while max_expansions is not binding.
  • Global cap enforcement in fix(fts): enforce fuzzy max_expansions globally across index partitions #7634 (separate PR, since the per-partition expansion predates this change): expansion now runs once per query at the index level under the query-wide budget, partitions receive the final token list, and a binding-cap regression test asserts results are independent of partition shape.

@Xuanwo Xuanwo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@BubbleCal BubbleCal merged commit 5afc915 into main Jul 6, 2026
30 checks passed
@BubbleCal BubbleCal deleted the yang/fts-tail-partition-budget-split branch July 6, 2026 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-index Vector index, linalg, tokenizer bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants