rewrite_data_files silently drops all compaction progress (and can NPE) when the starting snapshot is expired concurrently
- Dominant language
- Java
- Stars
- 9.2k
- Forks
- 3.5k
- Avg merge
- 2d 16h
- Merged PRs (30d)
- 129
Description
### Apache Iceberg version
main (development)
### Query engine
Spark
### Please describe the bug 🐞
When `rewrite_data_files` runs with partial progress enabled (`partial-progress.enabled=true`) and a concurrent `expire_snapshots` removes the rewrite's *starting* snapshot before the file-group commits complete, the action can either throw a `NullPointerException` or, more insidiously, **silently complete "successfully" with zero committed rewrites** - losing all compaction progress with no error surfaced to the caller.
**Root cause (two layers, both present on `main`):**
1. **NPE path (default configuration).** `RewriteDataFilesCommitManager#commitFileGroups` resolves the starting snapshot's sequence number when `use-starting-sequence-number` is enabled (the default):
```java
// core/src/main/java/org/apache/iceberg/actions/RewriteDataFilesCommitManager.java
RewriteFiles rewrite = table.newRewrite().validateFromSnapshot(startingSnapshotId);
if (useStartingSequenceNumber) {
long sequenceNumber = table.snapshot(startingSnapshotId).sequenceNumber(); // NPE
rewrite.dataSequenceNumber(sequenceNumber);
}
```
`Table#snapshot(long)` returns `null` for a snapshot that has been expired, so this line throws `NullPointerException`.
2. **Silent-swallow path.** The resulting exception (the NPE above, or a `ValidationException` from `validationHistory` when `use-starting-sequence-number=false`) is caught by the catch-all in `BaseCommitService#commitReadyCommitGroups`:
```java
// core/src/main/java/org/apache/iceberg/actions/BaseCommitService.java
} catch (Exception e) {
LOG.error("Failure during rewrite commit process, partial progress enabled. Ignoring", e);
}
```
Because partial progress is enabled, this is counted as a "tolerable" failed commit. If `failedCommits <= partial-progress.max-commits` (default 5), `RewriteDataFilesSparkAction` emits only a `LOG.warn` and returns a result with **0 committed rewrites**. The caller observes a successful action.
**Steps to reproduce:**
1. Create a table and write enough data files to form at least one rewrite file group.
2. Start a `rewrite_data_files` with `partial-progress.enabled=true`.
3. Concurrently, before the file-group commits complete, run `expire_snapshots` that expires the snapshot which was current when the rewrite started.
4. Observe: the action completes without error, the committed/rewritten file count is 0, and only a `LOG.warn` (or an NPE stacktrace) appears in the logs.
**Expected behavior:** the action should fail loudly with a clear, actionable error indicating that the starting snapshot was expired (e.g., by a concurrent `expire_snapshots`), rather than silently discarding all compaction progress.
**Proposed fix (I can contribute):** null-guard the starting snapshot in `commitFileGroups` (`Preconditions.checkState` with a clear message) and have `BaseCommitService` propagate non-retriable state errors instead of swallowing them, so the failure surfaces to the caller. I would include a unit-level reproducer for the commit manager plus an integration reproducer for the action.
### Willingness to contribute
- [X] I can contribute a fix for this bug independently
- [ ] I would be willing to contribute a fix for this bug with guidance from the Iceberg community
- [ ] I cannot contribute a fix for this bug at this time
Contributor guide
Research direction
Start with core/src/main/java/org/apache/iceberg/actions/RewriteDataFilesCommitManager.java and BaseCommitService.java, then trace the partial-progress path in RewriteDataFilesSparkAction. Reproduce concurrent expiration of the starting snapshot with partial progress enabled, and add the proposed unit-level commit-manager and integration action coverage. Done means the action surfaces a clear failure instead of silently returning zero committed rewrites.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 55/100