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)
Also, not sure if it would be possible / desirable, but maybe some kind of
sanity check before calling a recursive delete? One thing that comes to mind
is checking that the path starts with /accumulo/[uuid]/lock or whatever the
appropriate base path for the lock is?
----------------------------------------------------------------
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]
