amogh-jahagirdar commented on code in PR #12818:
URL: https://github.com/apache/iceberg/pull/12818#discussion_r2113105286
##########
core/src/main/java/org/apache/iceberg/rest/ErrorHandlers.java:
##########
@@ -91,7 +91,22 @@ public void accept(ErrorResponse error) {
case 404:
throw new NoSuchTableException("%s", error.message());
case 409:
- throw new CommitFailedException("Commit failed: %s",
error.message());
+ if (error.wasRetried()) {
+ // If the request was retried, and if its final error was 409, It
could probably also
+ // mean that HTTP retries when happened the IRC service could have
actually applied
+ // the commit in their persistence, while giving back the client
still a 5xx.
+ // If so, since the base has changed, it could conflict with
itself.
+ // In cases like this its best not to mark this as failed instead
+ // make this is a commit state unknown, so some reconciliation can
happen and
+ // the metadata clean-up is not triggered.
Review Comment:
Nit on comment:
```
/**
* If a retried request finally fails with 409,
* the IRC service may have persisted the commit
* despite initial 5xx errors, resulting in a self-conflict on retry
* due to the base changing.
* Mark this failure as commit state unknown rather than failed to prevent
file cleanup.
*/
```
--
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
