amogh-jahagirdar commented on code in PR #6500:
URL: https://github.com/apache/iceberg/pull/6500#discussion_r1058674867
##########
aws/src/main/java/org/apache/iceberg/aws/glue/GlueTableOperations.java:
##########
@@ -184,9 +185,7 @@ protected void doCommit(TableMetadata base, TableMetadata
metadata) {
if (persistFailure instanceof AwsServiceException) {
int statusCode = ((AwsServiceException) persistFailure).statusCode();
- if (statusCode >= 500 && statusCode < 600) {
- commitStatus = CommitStatus.FAILURE;
- } else {
+ if (statusCode < 500 && statusCode >= 600) {
Review Comment:
Not entirely sure about this change considering the presence of commit
retries and checkCommitStatus being a heavy operation.
Is the following a possible sequence of events? :
1.) The operation fails to commit due to some non AwsServiceException, so
commitStatus is set to something else.
2.) A retry commit is performed which fails with a status code >= 500 and <
600 then commit status then falls through to the checkCommitStatus again which
is heavier since it has to get the latest metadata location via a Glue call.
In step 2 we know it's a failure case immediately instead of having to do a
checkCommitStatus.
--
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
