ns-khsu opened a new issue, #16971:
URL: https://github.com/apache/iceberg/issues/16971
### Apache Iceberg version
1.11.0 (latest release)
### Query engine
Spark
### Please describe the bug 🐞
### Summary
When running the `rewrite_data_files` action with partial progress enabled
(`partial-progress.enabled=true`), the action can report commit failures —
emitting a `WARN` log, or even throwing a `RuntimeException` when
`partial-progress.max-failed-commits` is exceeded — **even though every commit
actually succeeded**.
### Root cause
`RewriteDataFilesSparkAction#doExecuteWithPartialProgress` does not count
failed commits directly. Instead it infers the count by subtraction:
```java
int totalCommits = Math.min(plan.totalGroupCount(), maxCommits);
int failedCommits = totalCommits - commitService.succeededCommits();
```
This assumes the total number of commits equals `min(totalGroupCount,
maxCommits)`. But the action groups multiple file groups into a single commit.
The number of file groups per commit is:
```
groupsPerCommit = ceil(totalGroupCount / maxCommits)
```
so the actual number of commits is:
```
ceil(totalGroupCount / groupsPerCommit)
```
which is usually **smaller** than `min(totalGroupCount, maxCommits)`. The
difference is then mis-reported as failed commits.
### Example
- `totalGroupCount = 70`, `maxCommits = 20`
- `groupsPerCommit = ceil(70 / 20) = 4`
- actual commits when all succeed = `ceil(70 / 4) = 18`
- reported `failedCommits = min(70, 20) - 18 = 2`
So 2 phantom failures are reported even though all 18 commits succeeded. If
`max-failed-commits < 2`, the whole operation fails with an exception despite a
fully successful rewrite.
### Expected behavior
`failedCommits` should reflect the number of commits that actually threw
during `commitOrClean`, and should be `0` when every commit succeeds.
### How to reproduce
1. Create a table and write enough data to produce many rewrite file groups
(e.g. ~70 groups).
2. Run `rewrite_data_files` with:
- `partial-progress.enabled = true`
- `partial-progress.max-commits` set so that `groupsPerCommit > 1`
(e.g. `max-commits = 20`)
- optionally a low `partial-progress.max-failed-commits` to trigger the
exception path.
3. Observe the spurious "N rewrite commits failed" WARN/exception even though
all commits succeeded.
### Proposed fix
Track failed commits directly in `BaseCommitService` rather than inferring
them:
- Add an `AtomicInteger failedCommits`, incremented in the `commitOrClean`
`catch` block.
- Make `succeededCommits` atomic as well, since commits run on the
multi-threaded rewrite pool (via `offer`).
- Expose a `failedCommits()` accessor and have the Spark action use it
directly
instead of computing `totalCommits - succeededCommits()`.
### 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]
