yadavay-amzn opened a new issue, #17202:
URL: https://github.com/apache/iceberg/issues/17202
### 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
