Parth-Brahmbhatt commented on a change in pull request #2328:
URL: https://github.com/apache/iceberg/pull/2328#discussion_r595495566
##########
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 don't think the property should be persisted at table level as a
single table could be written from both idempotent and non idempotent clients
(flink writer (idempotent) + background a compaction process (nonidempotent)).
We can either expose the API at Table or PendingUpdate level.
In absence of more external use cases it is hard to say what is worst ,
corrupting a table so nothing can read or write until manual intervention or
having duplicate data that can go undetected. I would pick duplicate data based
on general use cases that I have seen so if we are voting, I would vote to
default the behavior to assuming idempotent client.
----------------------------------------------------------------
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]
