RussellSpitzer commented on issue #17202: URL: https://github.com/apache/iceberg/issues/17202#issuecomment-4982071595
This has come up a few times in a more general form (e.g. [#13972](https://github.com/apache/iceberg/issues/13972), [#9521](https://github.com/apache/iceberg/issues/9521)). My read is that the current behavior is working as designed. When partial-progress.enabled=true, commit failures are intentionally tolerated. Even a failure caused by concurrent operations that invalidate the rewrite's starting snapshot. The BaseCommitService catch-all and the max-failed-commits threshold are meant to let the action complete without failing the whole job when individual commit batches conflict with other table activity. In the expired-snapshot case, the compaction work isn't really being "dropped" in a meaningful sense: those file groups can't be committed anyway once the starting snapshot is gone, and commitOrClean cleans up the newly written files on validation failures. The caller gets back fewer (or zero) committed rewrites, same as any other partial-progress commit failure. I'm fine with a small improvement to the error path (null-guarding table.snapshot(startingSnapshotId) so we log a clear message instead of an NPE) but I wouldn't change the action to fail the overall operation. That would be inconsistent with how partial progress is documented and implemented today. -- 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]
