[GitHub] [iceberg] stevenzwu commented on a diff in pull request #4673: Core: Fix table corruption from OOM during commit cleanup in Spark

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

stevenzwu commented on code in PR #4673:
URL: https://github.com/apache/iceberg/pull/4673#discussion_r862492783


##########
core/src/main/java/org/apache/iceberg/SnapshotProducer.java:
##########
@@ -343,11 +343,15 @@ public void commit() {
         LOG.warn("Failed to load committed snapshot, skipping manifest clean-up");
       }
 
-    } catch (RuntimeException e) {
-      LOG.warn("Failed to load committed table metadata, skipping manifest clean-up", e);
+    } catch (Throwable e) {

Review Comment:
   Catching and swallowing the `CommitStateUnknownException` is also not ideal. If the commit actually failed, that would lead Spark to assume the commit is successful.
   
   Ideally, Spark needs to know these commit results and then iceberg-spark can translate Iceberg commit result to Spark commit result.
   * commit success. treat it as success
   * commit failure. treat it as failure and perform the abort
   * commit state unknown. treat it as failure but don't perform the abort
   
   This discussion is probably outside the scope of this PR. This PR is fine. Basically, it swallows any exceptions from post-commit-success cleanup steps.



-- 
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: issues-unsubscribe@iceberg.apache.org

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