rdblue commented on code in PR #8397:
URL: https://github.com/apache/iceberg/pull/8397#discussion_r1314015172
##########
core/src/main/java/org/apache/iceberg/BaseTransaction.java:
##########
@@ -319,15 +320,7 @@ private void commitCreateTransaction() {
} catch (RuntimeException e) {
// the commit failed and no files were committed. clean up each update.
- Tasks.foreach(updates)
- .suppressFailureWhenFinished()
- .run(
- update -> {
- if (update instanceof SnapshotProducer) {
- ((SnapshotProducer) update).cleanAll();
- }
- });
-
+ computeUncommittedFiles();
Review Comment:
I think this needs to be refactored more. If the `RuntimeException` was
something like a 500 error for a create table or replace table transaction,
then the table may have been created or replaced. For strict cleanup, I think
we still need to check for a cleanable exception in those cases.
I think these should be:
```java
} catch (RuntimeException e) {
if (!ops.requiresStrictCleanup() || e instanceof CleanableFailure) {
cleanUpOnCommitFailure();
}
}
Tasks.foreach(deletedFiles).run(ops.io()::delete);
```
--
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]
