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 falsely 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
