feat: balance scalar index fragment batches - #758
Merged
majin1102 merged 2 commits intoAug 24, 2026
Conversation
ddupg
marked this pull request as ready for review
August 11, 2026 07:38
ddupg
force-pushed
the
feat/balance-index-fragment-batches
branch
from
August 11, 2026 07:42
80be25d to
7c79a7a
Compare
![lance-gatekeeper[bot]](https://avatars.githubusercontent.com/in/4433141?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The change addresses count-only task skew at the planner boundary with deterministic largest-first assignment by live row count, while preserving disjoint exact-once fragment coverage and the existing segment commit path. This is preferable to contiguous count slicing because its worst-case behavior no longer depends on fragment ordering.
lance-gatekeeper
Bot
added
the
K-approved
ddupg
force-pushed
the
feat/balance-index-fragment-batches
branch
from
August 20, 2026 08:13
7c79a7a to
13f9216
Compare
lance-gatekeeper
Bot
removed
the
K-approved
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The change addresses count-only task skew with deterministic largest-first assignment by live row count while preserving disjoint exact-once fragment coverage and the existing segment commit path. The current boxing simplification preserves those semantics across the supported Scala variants.
lance-gatekeeper
Bot
added
the
K-approved
ivscheianu
added a commit
to ivscheianu/lance-spark
that referenced
this pull request
Aug 26, 2026
REFRESH INDEX could commit an FTS index that no full-text query could read. Segments carry their build configuration, and an inverted index is read with one configuration for all of them, so a refresh that did not repeat the original options built segments Lance accepted at commit and then refused at read: "inconsistent inverted index details across segments". SHOW INDEXES reported full coverage throughout. The refresh now compares the details of the segments it built against the ones they would join and fails before committing, for index types whose read path requires them to agree. The comparison is on what was built rather than what was requested, and the segments are still uncommitted, so a rejection leaves the index untouched. Lance exposes no Java contract for reading a built index's parameters back -- only optimizeIndices, which derives them internally and runs on a single node -- so options still come from the WITH clause, and the docs now say which methods that matters for. Refreshing an index named <column>_idx failed on every executor task. The segment build let Lance derive its own name, which is exactly that, so the name-collision pre-check fired against the index being refreshed. That is the name Lance assigns an unnamed index, and the name used throughout the docs. Naming the segment after the index it will join, with replace set, skips a check that means nothing for an uncommitted build; the driver's single commitExistingIndexSegments transaction still decides what to keep. Re-running CREATE INDEX with such a name failed the same way, and the explicit name also moves the genuine "already exists with different fields" check ahead of the distributed build instead of after it. Uppercase WITH-clause options were silently ignored. ANTLR reports identifier text as written and every consumer matches lower-case literals, so WITH (NUM_SEGMENTS = 2) built with default parallelism and WITH (TRAIN = false) refreshed instead of being rejected. Option names are now normalized at the parser boundary, which fixes CREATE INDEX, OPTIMIZE and VACUUM the same way. Fragment batches are contiguous runs again. Lance can only compact fragments covered by an identical set of index segments, so the row-balanced assignment added in lance-format#758 left every adjacent pair under a different segment and made OPTIMIZE a no-op for the whole table. Balancing by row count and covering contiguous runs are compatible: boundaries are placed where an even split would fall, moved to the nearest fragment, which stays deterministic and still yields exactly num_segments batches. The commit-time liveness check is replaced by coverage accounting. A fragment leaves the manifest either because its rows moved or because they were all deleted; only the first leaves data unindexed, and in both cases the segments for what remains are correct. Discarding a finished distributed build was the wrong response, and it made a routine concurrent DELETE fatal. The commit now restricts declared coverage to the live fragments, warns about the rest, reports the count it actually achieved, and fails only when nothing would be covered. One rule covers both build paths, so range-mode BTree is no longer judged against a fragment list its coverage does not come from. Range-mode BTree also goes back to unpinned read options. It reads the table back through the catalog, which resolves its own version, so pinning made the segment record a dataset version older than its own contents. A DROP INDEX during a refresh no longer resurrects the index. The commit re-resolves the index on a fresh handle, because committing segments under a name Lance no longer knows creates that index rather than extending it. Fragment enumeration goes through getFragmentStatistics(), which returns primitive arrays, instead of getFragments(), which materializes a Java object per fragment and per data file on every driver-side pass. Docs: scope the compaction guidance to the methods that actually lose coverage, record that a partially covered zonemap index can return incomplete results rather than merely scanning uncovered fragments, describe how accumulated segments limit OPTIMIZE, and drop the claim that Lance does not expose a built index's parameters.
This was referenced Aug 26, 2026
ivscheianu
added a commit
to ivscheianu/lance-spark
that referenced
this pull request
Aug 26, 2026
IndexUtils.batchFragments assigned fragments to segments with a least-loaded heap. For equal-sized fragments that deals them out round-robin, so segment coverage interleaved fragment ids: four equal fragments with num_segments=2 produced segments covering [0,2] and [1,3]. Lance's compaction planner only groups fragments covered by an identical set of index metadata entries, and every segment is a separate entry. Interleaved coverage therefore puts every adjacent pair of fragments in a different group, so OPTIMIZE coalesces nothing at all on an indexed table. Four single-fragment appends survive `optimize ... with (target_rows_per_fragment = 1000)` unchanged when a two-segment index covers them, while the same fragments compact with one contiguous segment, and compact with no index. Batching was contiguous before lance-format#758, which traded that away for row balance. The two are compatible, so this restores contiguity without giving the balance back: the partition minimising the heaviest batch is found exactly, by binary searching the smallest row budget a contiguous packing can respect. For a fixed budget, extending each run as far as it will go uses the fewest runs, so the smallest feasible budget is the optimal maximum. Packing there can use fewer runs than were asked for, which would cost parallelism, so the remainder are split at their balance points; a split only lowers the heaviest run, so optimality survives it. A heuristic that placed each boundary where an even split would fall was tried first and rejected: it cuts after indivisible leading fragments have already overshot their shares, which on [95, 93, 89, 8, 1, 4, 74, 88, 38] across six segments leaves a batch of 162 where fragment 0 alone forces 95. Both that case and a comparison against every contiguous partition are now regressions.
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
2 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.
Summary
Testing
Closes #757