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

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

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



##########
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:
       With ZooKeeper 3.5.x there is a static utility method `ZKUtil.deleteRecursive(zooKeeper, path);`  that may eliminate the need for most, if not all of this code. (I'm not sure what the NodeMissingPolicy is expected to do here, so that may be a factor)




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