[GitHub] [accumulo] dlmarion commented on a change in pull request #1896: re #1086: modify ZooLock to use the native zookeeper client

[GitBox <gi...@apache.org>]

dlmarion commented on a change in pull request #1896:
URL: https://github.com/apache/accumulo/pull/1896#discussion_r568122017



##########
File path: core/src/main/java/org/apache/accumulo/fate/zookeeper/ZooLock.java
##########
@@ -158,13 +157,41 @@ public synchronized boolean tryLock(LockWatcher lw, byte[] data)
       String pathToDelete = path + "/" + createdNodeName;
       LOG.debug("[{}] Failed to acquire lock in tryLock(), deleting all at path: {}", vmLockPrefix,
           pathToDelete);
-      zooKeeper.recursiveDelete(pathToDelete, NodeMissingPolicy.SKIP);
+      recursiveDelete(pathToDelete, NodeMissingPolicy.SKIP);
       createdNodeName = null;
     }
 
     return false;
   }
 
+  /**
+   * This method will delete a node and all its children.
+   */
+  private void recursiveDelete(String zPath, NodeMissingPolicy policy)
+      throws KeeperException, InterruptedException {
+    if (policy == NodeMissingPolicy.CREATE) {
+      throw new IllegalArgumentException(policy.name() + " is invalid for this operation");
+    }
+    try {
+      // delete children
+      for (String child : zooKeeper.getChildren(zPath, null)) {
+        recursiveDelete(zPath + "/" + child, NodeMissingPolicy.SKIP);
+      }
+
+      // delete self
+      zooKeeper.delete(zPath, -1);
+    } catch (KeeperException e) {
+      // new child appeared; try again
+      if (e.code() == Code.NOTEMPTY) {
+        recursiveDelete(zPath, policy);
+      }
+      if (policy == NodeMissingPolicy.SKIP && e.code() == Code.NONODE) {
+        return;
+      }
+      throw e;
+    }
+  }
+

Review comment:
       @edcoleman @ctubbsii - the `recursiveDelete` method here is just a copy of the ZooReaderWriter.recursiveDelete method. Looking at the ZKUtil.recursiveDelete it appears that this method with the NodeMissingPolicy.SKIP option might be better. The ZKUtil.recursiveDelete might throw an error in a concurrent delete 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org