pvary commented on a change in pull request #2328:
URL: https://github.com/apache/iceberg/pull/2328#discussion_r595812395
##########
File path: core/src/main/java/org/apache/iceberg/SnapshotProducer.java
##########
@@ -293,8 +293,15 @@ public void commit() {
taskOps.commit(base, updated.withUUID());
});
+ } catch (CommitFailedException commitFailedException) {
+ // We have an acknowledged failure from the Catalog. We are confident
that the commit has not been applied
+ Exceptions.suppressAndThrow(commitFailedException, this::cleanAll);
} catch (RuntimeException e) {
- Exceptions.suppressAndThrow(e, this::cleanAll);
+ LOG.error("Cannot determine whether the commit was successful or not,
the underlying data files may or " +
Review comment:
I would go for API level flag that the change is idempotent since this
is more dependent on the client / action then the actual table.
Also I would go for a few connection retries and then throwing the
`UnknownCommitStateException` if we are not able to determine the status of the
commit. We should fail fast as soon as possible, so the user is able to
mitigate the issue. If they ignore the exception it is better to have a state
where we can recover so I would keep the files in case we are not able to
handle the error cleanly.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
