RussellSpitzer commented on a change in pull request #2328:
URL: https://github.com/apache/iceberg/pull/2328#discussion_r594604968
##########
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 actually am not sure that is the case here, we are in the situation
were one client, clientA has somehow lost the ability to confirm, but i'm not
sure it's more likely that client B would also lose the ability to confirm. I
guess it depends on if you believe a server-level failure is more likely than a
client-level failure.
In my experience it is much more likely that single client has connection
issues than all clients have connection issues, but ymmv
----------------------------------------------------------------
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]