RussellSpitzer commented on code in PR #4673:
URL: https://github.com/apache/iceberg/pull/4673#discussion_r862272257
##########
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:
The issue was that a throwable not caught by the catch block here would be
thrown to Spark, which would cause Spark to consider the commit failed and
cause Spark to perform it's own abort code removing the committed files.
So the old behavior was
1. Table Operations Commit (this can happen successfully)
2. While Cleaning up old files or theoretically while calling notify
listeners a non-runtime exception is thrown
3. The commit has been successful but the exception is re-thrown to the
Spark Commit code
4. The Spark commit code sees the exception and executes it's abort method
So the fix is basically never ever throw an exception once the commit has
occurred, regardless of 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.
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]
