apache / apache/iceberg

rewrite_data_files silently drops all compaction progress (and can NPE) when the starting snapshot is expired concurrently

Open
#17,202 1 comment 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.