[smaheshwar-pltr]

Why

With partial-progress.enabled, each rewrite worker commits its own batch, so up to max-concurrent-file-group-rewrites commits to the same table run at once. One wins each round. The rest back off and retry, writing a new manifest list each time and re-filtering any input manifest that another commit replaced. A batch that runs out of commit.retry.num-retries is cleaned up and its rewrite is lost. With the default 4 retries this happens even without other writers: in benchmark runs with 4 to 16 workers, main lost groups in 54 of 75.

Change

The rewrite commit managers call apply() before taking a lock and commit() inside it. apply() runs concurrently and the producer caches the manifests it writes. Under the lock, commit() refreshes, validates, writes the manifest list and swaps metadata, and only re-filters or re-merges manifests that another commit changed in the meantime. A failed apply() is logged and commit() applies again, so failures and cleanup behave as before.

Applying again after a failed delete manifest write counted the added delete files twice in the snapshot summary, so MergingSnapshotProducer now rebuilds that summary with the manifests.

Locking the whole commit() would be simpler, but it serializes manifest writing too. The "whole-commit lock" column below shows the cost, measured with a variant that commits ready batches one at a time while the other workers keep rewriting.

Benchmark

Spark 4.1 local[16], HadoopCatalog, one group per commit, median of 5 runs in seconds. Groups are 4 small files that rewrite in about 70 ms, so runs are commit-bound. Delays simulate object-store latency on writes; reads are not delayed.

Scenario	main	whole-commit lock	this PR
No added latency, 16 workers, 4 retries	7.6	3.3	2.5
100 ms manifest writes, 8 workers, 20 retries	6.5	9.7	2.7
500 ms manifest writes, 16 workers finishing together, 20 retries	13.6	17.8	2.5
100 ms on all metadata writes, 16 workers, 20 retries	34.4	16.3	9.2
100 ms manifest writes, 1 worker	11.9	11.9	13.4
100 ms on all metadata writes, 1 worker	18.3	18.3	23.0

With this change no commit retried and no groups were lost. In the first row main also lost 2.8 of 32 groups per run. With one worker nothing overlaps the early apply(), so its extra refresh, validation and manifest list write show up: about 50 ms per commit, or 150 ms when manifest list writes are slow. Rewrites without partial progress and Flink's rewrite committer pay this once per commit.

manifests-replaced in the snapshot summary and commit report can over-count when a manifest that the early apply() filtered or merged is replaced before commit(). Retries on main already do this; I'll fix the count separately.

Testing

TestRewriteDataFilesCommitManager and TestRewritePositionDeletesCommitManager commit 8 groups concurrently with commit.retry.num-retries=0, which fails on main with CommitFailedException, and check that concurrent and failed commits leave no unreferenced metadata files. A new TestRowDelta case covers the summary fix. The Spark partial-progress test no longer tolerates a failed commit.

---

AI Disclosure

• Model: Claude Opus 5.5 (core change, tests, benchmark); [unknown - human to fill in] via Codex (first version, Spark test changes)
• Platform/Tool: Codex, Claude Code
• Human Oversight: [unknown - human to fill in]
• Prompt Summary: Stop partial-progress rewrite commits from conflicting with each other, keeping the change small and the manifest writing concurrent.

🤖 Generated with Claude Code

[github-actions]

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.

[smaheshwar-pltr]

Not stale