southernriver commented on PR #4507: URL: https://github.com/apache/iceberg/pull/4507#issuecomment-1094464138
> I'm not sure how the changes in this PR would help, since it just makes a warning more broad. The catch clauses that are updated here don't actually change what happens other than to log a warning in unrelated cases. > > I'm also skeptical that this is the right thing to do. The chance of an OOM causing a commit to appear like it did not succeed when it actually did is incredibly small... and besides, cleanup is run when a `RuntimeException` is caught. That means that an Error wouldn't actually cause the commit path to remove live data files. > > @southernriver, It sounds like the problem that you actually hit was with Spark's error handling and cleanup instead? Maybe we should change the Spark writer instead to avoid triggering its abort path in certain cases. That's a very different change. @rdblue Thanks for your comment. Iceberg itself implements the SparkWrite#abort method and could delete datafile with uncatched `java.lang.Error` or `java.lang.Exception`. ` RuntimeException` can not cover`OutOfMemoryError` , so I did catch all `java.lang.Throwable` to avoid clean-up live data files. -- 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]
