rdblue commented on a change in pull request #736: Add timeout for acquiring 
locks in HiveTableOperations
URL: https://github.com/apache/incubator-iceberg/pull/736#discussion_r367009330
 
 

 ##########
 File path: hive/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java
 ##########
 @@ -249,13 +257,32 @@ private long acquireLock() throws UnknownHostException, 
TException, InterruptedE
     LockResponse lockResponse = metaClients.run(client -> 
client.lock(lockRequest));
     LockState state = lockResponse.getState();
     long lockId = lockResponse.getLockid();
-    //TODO add timeout
+
+    final long start = System.currentTimeMillis();
+    long duration = 0;
+    boolean timeout = false;
     while (state.equals(LockState.WAITING)) {
       lockResponse = metaClients.run(client -> client.checkLock(lockId));
       state = lockResponse.getState();
+
+      // check timeout
+      duration = System.currentTimeMillis() - start;
+      if (duration > lockAcquireTimeout) {
+        timeout = true;
+        break;
 
 Review comment:
   Instead of break, can we use `timeout` in the `while` condition?
   
   ```java
   while (!timeout && state.equals(LockState.WAITING)) { ... }
   ```
   
   As long as we are setting `timeout`, I think it is nice to use it to avoid 
control flow breaks. We would also need to move `Thread.sleep(50)` into an 
`else`.

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to