RussellSpitzer commented on code in PR #4673:
URL: https://github.com/apache/iceberg/pull/4673#discussion_r862295750
##########
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:
@stevenzwu It's not in our code, it's in Spark's code where it doesn't have
a defined exception for either of those states. Apache Spark basically has this
code
```java
try {
DataSource.commit()
} catch (anything) {
datasource.abort()
}
```
[source](https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/WriteToDataSourceV2Exec.scala#L392-L400)
Spark doesn't know about any of our particular exceptions, it just wants to
know if it's call to commit failed or not. In this particular instance the
worry is that the commit did succeed and no exceptions were thrown but
additional code ran after the commit (like our clean up code or notify
listeners) which throws another exception.
Spark itself cannot distinguish between an exception that happened before
the actual commit and after the actual commit within our DataSource commit
method. If we wanted to let engines handle this we could have our Iceberg
SparkWrite try to handle this but I think that would probably be difficult to
manage as well. I think it more sense to say that once committed, we suppress
all exceptions. Our Commit method in SnapshotProducer only throws exceptions
when the commit fails and in *no* other circumstances.
--
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]
