yadavay-amzn commented on PR #16237: URL: https://github.com/apache/iceberg/pull/16237#issuecomment-4466512844
@laskoviymishka @Baunsgaard Heads up — I made a change after your approvals that I want to flag: The exception handling now uses a tiered approach instead of rethrowing everything: 1. `CommitFailedException` / `CommitStateUnknownException` → log + retry (transient conflicts) 2. Other `CleanableFailure` exceptions → log + retry (Iceberg's marker for recoverable errors) 3. Everything else → log ERROR + rethrow (fatal, kills the coordinator) The reason: rethrowing ALL non-`CommitFailedException` exceptions was killing the coordinator on transient errors during the integration tests (the commit path can throw various `CleanableFailure` subtypes during normal catalog contention). The `CleanableFailure` interface is Iceberg's built-in classification for retryable exceptions, so using it as the boundary felt like the right approach. CI is now green. Let me know if you'd prefer a different approach — e.g., a consecutive-failure counter that rethrows after N failures regardless of exception type. -- 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]
