amogh-jahagirdar commented on code in PR #12818:
URL: https://github.com/apache/iceberg/pull/12818#discussion_r2049703938
##########
core/src/main/java/org/apache/iceberg/rest/RESTTableOperations.java:
##########
@@ -155,13 +158,21 @@ public void commit(TableMetadata base, TableMetadata
metadata) {
// the error handler will throw necessary exceptions like
CommitFailedException and
// UnknownCommitStateException
// TODO: ensure that the HTTP client lib passes HTTP client errors to the
error handler
- LoadTableResponse response =
- client.post(path, request, LoadTableResponse.class, headers,
errorHandler);
+ try {
+ LoadTableResponse response =
+ client.post(path, request, LoadTableResponse.class, headers,
errorHandler);
+ // all future commits should be simple commits
+ this.updateType = UpdateType.SIMPLE;
- // all future commits should be simple commits
- this.updateType = UpdateType.SIMPLE;
-
- updateCurrentMetadata(response);
+ updateCurrentMetadata(response);
+ } catch (RESTException e) {
+ if (e.getCause() != null && e.getCause() instanceof IOException) {
+ // any IOException or unhandled Exception should be considered as
commit unknown
+ // so that caller can attempt to potentially reconcile
+ throw new CommitStateUnknownException(e);
+ }
+ throw e;
+ }
Review Comment:
Correct me if I'm wrong but if I'm following the issue/trace, during the
commit we fail with a generic RestException wrapping a SocketTimeout exception.
If that's the case we'd be hitting
https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/SnapshotProducer.java#L472
Since it's a generic RestException which isn't a cleanable failure, we'd
skip the cleanup in that logic and bubble up the exception.
Is it possible that we're actually doing some cleanup when we shouldn't
after the exception is further bubbled up? Just want to make sure that we're
fixing in the right place, since I know getting a test repro may be difficult
for this case
--
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
