[GitHub] [iceberg] aokolnychyi commented on a change in pull request #2328: (#2317) Stop removal of files when catalog state is uncertain

[GitBox <gi...@apache.org>]

aokolnychyi commented on a change in pull request #2328:
URL: https://github.com/apache/iceberg/pull/2328#discussion_r592944422



##########
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 am afraid this will be dangerous. This means we have to update all places that throw a specific exception like `AlreadyExistsException` to now throw `CommitFailedException`. This is error-prone and we lose details that will be helpful. We could throw `CommitFailedException` and add the specific exception as a cause but the first point still holds.
   
   Also, an error log message will most likely be ignored by the user. This is a case where we really want to propagate as much info as possible.
   
   Would it make sense to introduce a new exception type? E.g., `UnknownCommitStateException`? That way, we can keep the existing logic for handling exceptions except cases where we really don't know what happens.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org