fix(compaction): preserve row order across parallel tasks

[lance-gatefixer]

Summary

• preserve compaction row order for out-of-order, partial, and gapped result sets while keeping manifests sorted by fragment ID
• relabel untouched trailing fragments with newly reserved IDs while reusing their data files and preserving deletion/index metadata
• rebase stale distributed compaction results across metadata-only relabels and retry row-adding concurrent transactions
• retain the released table format without a new protobuf field, feature flag, or format-spec change

Root cause

Compaction replacement fragments receive fresh IDs above the manifest high-water mark. The commit path globally sorts fragments by ID, so a replacement for an early range moved behind any untouched later fragments. Concurrently completed tasks could also arrive in a different order, and bounded compaction exposed the same defect repeatedly.

Fix

Compaction results are first ordered by their current source positions. Starting at the earliest rewritten range, the commit completes one ordered replacement suffix: real compaction outputs replace planned ranges, while untouched trailing fragments are represented by metadata-only replacements that keep their data files and receive fresh consecutive IDs. Deletion files are copied to the paths implied by the new fragment IDs, physical row-address indices are remapped, and stable-row-ID index coverage follows the relabeled fragments.

Stale distributed tasks recognize prior metadata-only relabels and rebase captured row addresses. Genuine source changes remain retryable conflicts, as do concurrent appends and row-adding updates that would invalidate the reserved suffix ordering. The commit boundary continues to require strictly increasing fragment IDs, so released readers and writers retain their existing representation and compatibility.

Validation

• cargo fmt --all -- --check
• cargo clippy --all --tests --benches -- -D warnings
• cargo test -p lance dataset::optimize::tests -- --nocapture (117 passed)
• cargo test -p lance dataset::transaction::tests -- --nocapture (63 passed)
• cargo test -p lance io::commit::conflict_resolver::tests -- --nocapture (44 passed)
• cargo test -p lance test_check_fragment_ids_requires_sorted_order -- --nocapture (1 passed)
• cargo test -p lance test_compact_distributed -- --nocapture (4 passed)
• cargo test -p lance test_bounded_compaction_preserves_order_across_candidate_gap -- --nocapture (2 passed)
• rebuilt the local Python extension with make build from python/
• focused JsonIndex/FtsIndex upgrade-downgrade compatibility tests (8 passed across Lance 0.36.0, 8.0.1, 9.0.1, and 10.0.0)

Fixes #3465

[github-actions]

Important

This PR touches the Lance format specification.

Substantive changes to the format specification — the .proto definitions
and the spec docs under docs/src/format/ — require a PMC vote before merge.
Minor edits such as typo fixes, wording, or formatting are excluded; use your
judgment.

If this is a meaningful format change:

• Start a vote following the Lance community voting process.
  Format specification modifications need 3 binding +1 votes (excluding the
  proposer), held on GitHub Discussions, with a minimum voting period of 1 week.
• Once the vote passes, link the completed vote in this PR. It should not be
  merged until the vote is linked.

[lance-gatefixer]

Addressed in 93b75ef. The repair now retains the ID-sorted manifest contract by reserving one ordered replacement suffix and metadata-only relabeling untouched trailing fragments. The logical-order feature bit, protobuf/spec changes, and format-vote dependency are removed; focused JsonIndex/FtsIndex upgrade-downgrade tests pass against the locally available historical releases.

[lance-gatekeeper]

❌ Gate recommendation: request changes.

The suffix cap bounds relabel work, but it replaces the established oldest-first incremental behavior with tail-only planning that can permanently leave eligible fragments untouched.

A viable revision must preserve a progress contract as well as row order and bounded work. Decouple logical order from fresh fragment IDs, or make tail-only/no-progress semantics an explicit maintainer decision instead of describing this option as bounded incremental compaction.

[lance-gatekeeper]

This hard suffix cut can permanently strand eligible compaction work. With fragment row counts [100, 100, 1000, 1000], target_rows_per_fragment=250, and max_source_fragments=2, fragments 0 and 1 are an eligible pair, but the healthy final pair makes suffix_start=2, so this branch removes the only candidate bin. Every repeated bounded run therefore returns an empty plan rather than advancing the oldest eligible work.

The planner needs to keep earlier eligible work reachable within the accepted contract. If that requires logical order independent of fresh IDs, the format decision must be explicit rather than silently turning this incremental option into a permanent no-op.

Reproducer

I added this focused test on the current head:

#[tokio::test]
async fn test_max_source_fragments_progresses_before_healthy_suffix() {
    let test_dir = TempStrDir::default();
    let data = sample_data();
    let schema = data.schema();
    let fragment_rows = [100, 100, 1_000, 1_000];
    let write_params = WriteParams {
        max_rows_per_file: 1_000,
        ..Default::default()
    };

    Dataset::write(
        RecordBatchIterator::new(vec![Ok(data.slice(0, fragment_rows[0]))], schema.clone()),
        &test_dir,
        Some(write_params.clone()),
    )
    .await
    .unwrap();
    let mut offset = fragment_rows[0];
    for row_count in fragment_rows.iter().copied().skip(1) {
        let mut append_params = write_params.clone();
        append_params.mode = WriteMode::Append;
        Dataset::write(
            RecordBatchIterator::new(vec![Ok(data.slice(offset, row_count))], schema.clone()),
            &test_dir,
            Some(append_params),
        )
        .await
        .unwrap();
        offset += row_count;
    }

    let dataset = Dataset::open(&test_dir).await.unwrap();
    let options = CompactionOptions {
        target_rows_per_fragment: 250,
        max_source_fragments: Some(2),
        ..Default::default()
    };
    let plan = plan_compaction(&dataset, &options).await.unwrap();
    let planned_fragment_ids = plan
        .tasks()
        .iter()
        .flat_map(|task| task.fragments.iter().map(|fragment| fragment.id))
        .collect::<Vec<_>>();
    assert_eq!(planned_fragment_ids, vec![0, 1]);
}

cargo test -p lance test_max_source_fragments_progresses_before_healthy_suffix -- --nocapture

Expected [0, 1]; observed [].

[lance-gatefixer]

No code change was made at 56ee824 because the requested combination is incompatible with the current stable table contract. Rewrite transactions must use freshly reserved fragment IDs, and released readers require manifests to remain ID-sorted. An early rewrite must therefore either relabel every trailing fragment (exceeding max_source_fragments) or change row order. Restoring oldest-first bounded progress requires the separately reviewed logical-order format feature and its maintainer/PMC decision; source-ID reuse would violate the fragment-ID high-water invariant.

[lance-gatefixer]

No code change was made at d23b189. The requested combination remains incompatible with the released stable contract: fresh fragment IDs plus ID-sorted manifests require either relabeling the full trailing suffix (exceeding the strict bound) or changing row order. Oldest-first bounded progress therefore requires a maintainer choice to relax the bound or approve a logical-order format feature.

[lance-gatefixer]

No code change was made at 56ee824. The requested breaking-change label has been added and verified on this PR. The remaining contract choice—strictly bounding all identity changes versus guaranteeing incremental progress, or adopting logical fragment order as a new format feature—requires the maintainer/PMC decision described in the review.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[lance-gatefixer]

No code change was made at d23b189. The current implementation takes the strict affected-fragment-bound option while preserving ID-sorted stable manifests and row order; guaranteeing oldest-first bounded progress as well would require either relaxing that bound or approving the separately reviewed logical-order format feature. Selecting that contract is a maintainer/PMC decision, not an implementation choice this repair can assume.

[lance-gatefixer]

Blocked: PR #8400 requires a maintainer decision on the max_source_fragments contract before this repair can advance.

At remote head c5d750134390f23c4d920ceaa8e4e759ce1b6fb8, Gatekeeper Review PRR_kwDOHn_KdM8AAAABJYgnfA reports “maintainer decision required”: a strict identity-change bound can permanently skip older eligible work, while oldest-first progress requires either allowing suffix relabel/remap beyond the bound or adopting logical fragment order with compatibility and PMC approval.

The repair already implements and validates the strict-bound option while preserving row order and ID-sorted released manifests; all current actionable inline findings have verified App dispositions, and the focused compaction suite passes 128 tests. No code-only change can select among these policy contracts without maintainer authority.

The smallest unblock is for a maintainer to confirm the strict-bound semantics. Alternatively, explicitly permit suffix work beyond the bound, or approve the logical-order format path and its required process.

[lance-gatekeeper]

🟡 Gate recommendation: maintainer decision required.

The current revision preserves row order, the released ID-sorted manifest contract, and excluded fragment identities by limiting rewrites to one relabeled suffix. The verified consequence is that max_source_fragments can permanently strand older eligible work; after the latest merge, a caller or unreadable-index exclusion likewise makes every earlier candidate indefinitely ineligible.

Maintainers need to choose whether those options may sacrifice compaction progress, whether the bound and exclusion-identity guarantees may be relaxed, or whether logical fragment order should become a format feature. If strict affected-fragment bounds, immutable exclusions, and oldest-first progress are all required, the logical-order format path is necessary, with its compatibility boundary and PMC approval.

[lance-gatefixer]

Blocked: PR #8400 requires a maintainer decision on the max_source_fragments contract before this repair can advance.

At remote head 58b67b44105ae2306db5670fb4c6fe1661796b16, current-head Gatekeeper Review PRR_kwDOHn_KdM8AAAABLfBhtw reports “maintainer decision required”: preserving the strict affected-fragment bound and excluded fragment identities can permanently strand older eligible compaction work, while guaranteeing oldest-first progress requires relaxing those constraints or adopting logical fragment order as a format feature.

The repair branch contains the current main tip, all prior actionable feedback has verified App dispositions with no newer external contribution, formatting and all 145 focused compaction tests pass, and all 38 current-head checks succeed. A normal push verified the published head; no code-only change can select this compatibility contract without maintainer authority.

The smallest unblock is for a maintainer to confirm that strict bound and exclusion semantics may sacrifice compaction progress. Alternatively, explicitly permit suffix relabel/remap work beyond the bound, or approve the logical-order format path and its required compatibility and PMC process.

[everySympathy]

Cross-reference from #8986: budget-aware TaskData prefix splitting reaches the same unresolved contract. Gatekeeper reproduced both row-order corruption (fresh replacement Fragment IDs move an early rewrite behind the untouched suffix) and non-convergence ([100, 100, 100, 100, 100] with target 250 and a two-Fragment hard budget stalls at [200, 300]). Reusing source Fragment IDs is not a compatible escape hatch because Lance explicitly treats Fragment IDs as never reused for row-address and Fragment Reuse Index correctness. This means strict bounded progress needs an ordered partial-commit representation/strategy from this PR, or an explicit decision to relax one of strict budget, oldest-first progress, or stable-manifest compatibility.

[lance-gatefixer]

No code change was made at 58b67b4. The #8986 evidence confirms the existing contract boundary rather than supplying a compatible implementation: this head preserves ID-sorted manifests and strictly bounds every changed Fragment identity, which can necessarily strand older eligible work, and the #8986 author likewise verified that prefix splitting is unsafe under that representation. Adopting ordered partial commits as a format feature or relaxing the strict budget/progress guarantees still requires explicit maintainer/PMC direction.

[lance-gatefixer]

Blocked: PR #8400 still requires an explicit maintainer/PMC decision on the bounded-compaction contract before this repair can advance.

At remote head 58b67b44105ae2306db5670fb4c6fe1661796b16, current-head Gatekeeper Review PRR_kwDOHn_KdM8AAAABLfBhtw requires maintainers to choose among strict affected-fragment bounds, oldest-first progress, and a logical-order format feature. The newer #8986 cross-reference IC_kwDOHn_KdM8AAAABSf5PFQ independently confirms that prefix splitting reorders rows or stalls at [200, 300], and that source Fragment ID reuse is not compatible.

The #8986 contribution now has a verified App disposition, all prior actionable threads retain valid dispositions, formatting and all 145 focused compaction tests pass, all 38 current-head checks succeed, and a normal push verified the published head. No code-only change can select among these incompatible compatibility guarantees without maintainer authority.

The smallest unblock is for a maintainer to confirm that strict bound and exclusion semantics may sacrifice compaction progress. Alternatively, explicitly permit suffix relabel/remap work beyond the bound, or approve the logical-order format path and its required compatibility and PMC process.