pvary commented on a change in pull request #2328:
URL: https://github.com/apache/iceberg/pull/2328#discussion_r599362241



##########
File path: 
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java
##########
@@ -270,29 +270,33 @@ private CommitStatus checkCommitStatus(String 
newMetadataLocation, TableMetadata
     int maxAttempts = PropertyUtil.propertyAsInt(config.properties(), 
COMMIT_NUM_STATUS_CHECKS,
         COMMIT_NUM_STATUS_CHECKS_DEFAULT);
 
-    for (int attempt = 1; attempt <= maxAttempts; attempt++) {
-      try {
-        Thread.sleep(COMMIT_STATUS_RECHECK_SLEEP);
-        TableMetadata metadata = refresh();
-        String metadataLocation = metadata.metadataFileLocation();
-        boolean commitSuccess = metadataLocation.equals(newMetadataLocation) ||
-            metadata.previousFiles().stream().anyMatch(log -> 
log.file().equals(newMetadataLocation));
-        if (commitSuccess) {
-          LOG.info("Commit status check: Commit to {}.{} of {} succeeded", 
database, tableName, newMetadataLocation);
-          return CommitStatus.SUCCESS;
-        } else {
-          LOG.info("Commit status check: Commit to {}.{} of {} failed", 
database, tableName, newMetadataLocation);
-          return CommitStatus.FAILURE;
-        }
-      } catch (Throwable checkFailure) {
-        LOG.error("Cannot check if commit to {}.{} exists. Retry attempt {} of 
{}.",
-            database, tableName, attempt, maxAttempts, checkFailure);
-      }
-    }
-
-    LOG.error("Cannot determine commit state to {}.{}. Failed to check {} 
times. Treating commit state as unknown.",
+    AtomicReference<CommitStatus> status = new 
AtomicReference<>(CommitStatus.UNKNOWN);
+
+    Tasks
+        .foreach(newMetadataLocation)
+        .retry(maxAttempts)
+        .suppressFailureWhenFinished()
+        .exponentialBackoff(COMMIT_STATUS_RECHECK_SLEEP, 
COMMIT_STATUS_RECHECK_SLEEP, Long.MAX_VALUE, 1.0)

Review comment:
       FYI: In Hive we are considering creating a new Catalog implementation 
which uses the HMS client from the session. That one is a 
`RetryingMetaStoreClient` which have retries in itself.
   
   If we do so, we might have to change the `maxAttempts` to 0/1.




-- 
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.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to