amogh-jahagirdar commented on PR #10373: URL: https://github.com/apache/iceberg/pull/10373#issuecomment-2173768003
Sorry for the delayed reply everyone, just getting back into this PR. @stevenzwu >I like the introduction of the CleanableFailure marker interface for this purpose. Engines can potentially updated to reflect that intention. On the other hand, the challenge is that this is not defined in the spec. It may be less obvious to engine/catalog implementer. Some engines (like Spark) have learned to handle CommitStateUnknownException. Can we standardize on CleanableFailure for Java implementation? Can we add the behavior clarification to the spec? I think it's a bit hard to add to the spec because the spec should be generalizable to all programming langauges/implementations, where a Java exception does not really fit that mold. However, I do like the idea of standardizing the exception usage in the Java implementation and the JVM engine integrations! >Seems that this PR is trying to standardize uncleanable commit errors to CommiteStateUnknownException. I like the safer approach of explicitly defining cleanable errors (instead of defining uncleanable errors). Engines/catalogs can handle cleanable errors explicitly. It is true that this PR is attempting to standardizing uncleanable commit errors to CommitStateUnknownException, but to be clear everything except CleanableFailure is considered CommitStateUnknownException which is intentional for guaranteeing safe cleanup.In the current implementation the standardization of uncleanable to `CommitStateUnknownException` was intentional since engines already handle that, and I was trying to avoid changing the different engine integrations. However, you bring up a good point that it may be better to go ahead and change the engines to explicitly handle CleanableFailure. I'll look into this! -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
