From b8e5752cab6715c1817939c1e1cb22c8b9a9c5a1 Mon Sep 17 00:00:00 2001 From: Ashish Singhi Date: Thu, 18 Jun 2015 18:50:21 +0530 Subject: [PATCH] HBASE-13925 Use zookeeper multi to clear znodes in ZKProcedureUtil --- .../org/apache/hadoop/hbase/zookeeper/ZKUtil.java | 118 +++++++++++++++++---- .../hadoop/hbase/procedure/ZKProcedureUtil.java | 20 ++-- .../apache/hadoop/hbase/zookeeper/TestZKMulti.java | 74 +++++++++++++ 3 files changed, 184 insertions(+), 28 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java index 6b2cadd..3e5ad3f 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java @@ -1382,24 +1382,7 @@ public class ZKUtil { */ public static void deleteNodeRecursively(ZooKeeperWatcher zkw, String node) throws KeeperException { - try { - List children = ZKUtil.listChildrenNoWatch(zkw, node); - // the node is already deleted, so we just finish - if (children == null) return; - - if(!children.isEmpty()) { - for(String child : children) { - deleteNodeRecursively(zkw, joinZNode(node, child)); - } - } - //Zookeeper Watches are one time triggers; When children of parent nodes are deleted - //recursively, must set another watch, get notified of delete node - if (zkw.getRecoverableZooKeeper().exists(node, zkw) != null){ - zkw.getRecoverableZooKeeper().delete(node, -1); - } - } catch(InterruptedException ie) { - zkw.interruptedException(ie); - } + deleteNodeRecursivelyMultiOrSequential(zkw, true, node); } /** @@ -1474,6 +1457,69 @@ public class ZKUtil { } /** + * Delete the specified node and its children. This traverse the + * znode tree for listing the children and then delete + * these znodes including the parent using multi-update api or + * sequential based on the specified configurations. + *

+ * Sets no watches. Throws all exceptions besides dealing with deletion of + * children. + *

+ * If hbase.zookeeper.useMulti is true, use ZooKeeper's multi-update + * functionality. Otherwise, run the list of operations sequentially. + *

+ * If all of the following are true: + *

+ * on calling multi, we get a ZooKeeper exception that can be handled by a + * sequential call(*), we retry the operations one-by-one (sequentially). + * + * @param zkw + * - zk reference + * @param runSequentialOnMultiFailure + * - if true when we get a ZooKeeper exception that could retry the + * operations one-by-one (sequentially) + * @param pathRoots + * - path of the parent node(s) + * @throws KeeperException.NotEmptyException + * if node has children while deleting + * @throws KeeperException + * if unexpected ZooKeeper exception + * @throws IllegalArgumentException + * if an invalid path is specified + */ + public static void deleteNodeRecursivelyMultiOrSequential(ZooKeeperWatcher zkw, + boolean runSequentialOnMultiFailure, String... pathRoots) throws KeeperException { + if (pathRoots == null || pathRoots.length <= 0) { + LOG.warn("Given path is not valid!"); + return; + } + List ops = new ArrayList(); + for (String eachRoot : pathRoots) { + // Zookeeper Watches are one time triggers; When children of parent nodes are deleted + // recursively, must set another watch, get notified of delete node + List children = listChildrenBFSAndWatchThem(zkw, eachRoot); + // Delete the leaves first and eventually get rid of the root + for (int i = children.size() - 1; i >= 0; --i) { + ops.add(ZKUtilOp.deleteNodeFailSilent(children.get(i))); + } + try { + if (zkw.getRecoverableZooKeeper().exists(eachRoot, zkw) != null) { + ops.add(ZKUtilOp.deleteNodeFailSilent(eachRoot)); + } + } catch (InterruptedException e) { + zkw.interruptedException(e); + } + } + // atleast one element should exist + if (ops.size() > 0) { + multiOrSequential(zkw, ops, runSequentialOnMultiFailure); + } + } + + /** * BFS Traversal of all the children under path, with the entries in the list, * in the same order as that of the traversal. Lists all the children without * setting any watches. @@ -1510,6 +1556,42 @@ public class ZKUtil { } /** + * BFS Traversal of all the children under path, with the entries in the list, + * in the same order as that of the traversal. + * Lists all the children and set watches on to them. + * + * @param zkw + * - zk reference + * @param znode + * - path of node + * @return list of children znodes under the path + * @throws KeeperException + * if unexpected ZooKeeper exception + */ + private static List listChildrenBFSAndWatchThem(ZooKeeperWatcher zkw, final String znode) + throws KeeperException { + Deque queue = new LinkedList(); + List tree = new ArrayList(); + queue.add(znode); + while (true) { + String node = queue.pollFirst(); + if (node == null) { + break; + } + List children = listChildrenAndWatchThem(zkw, node); + if (children == null) { + continue; + } + for (final String child : children) { + final String childPath = node + "/" + child; + queue.add(childPath); + tree.add(childPath); + } + } + return tree; + } + + /** * Represents an action taken by ZKUtil, e.g. createAndFailSilent. * These actions are higher-level than ZKOp actions, which represent * individual actions in the ZooKeeper API, like create. diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/ZKProcedureUtil.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/ZKProcedureUtil.java index 9e0ef7f..8171218 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/ZKProcedureUtil.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/ZKProcedureUtil.java @@ -267,29 +267,29 @@ public abstract class ZKProcedureUtil } public void clearChildZNodes() throws KeeperException { - // TODO This is potentially racy since not atomic. update when we support zk that has multi LOG.info("Clearing all procedure znodes: " + acquiredZnode + " " + reachedZnode + " " + abortZnode); // If the coordinator was shutdown mid-procedure, then we are going to lose // an procedure that was previously started by cleaning out all the previous state. Its much // harder to figure out how to keep an procedure going and the subject of HBASE-5487. - ZKUtil.deleteChildrenRecursively(watcher, acquiredZnode); - ZKUtil.deleteChildrenRecursively(watcher, reachedZnode); - ZKUtil.deleteChildrenRecursively(watcher, abortZnode); + ZKUtil.deleteChildrenRecursivelyMultiOrSequential(watcher, true, acquiredZnode, reachedZnode, + abortZnode); } public void clearZNodes(String procedureName) throws KeeperException { - // TODO This is potentially racy since not atomic. update when we support zk that has multi LOG.info("Clearing all znodes for procedure " + procedureName + "including nodes " + acquiredZnode + " " + reachedZnode + " " + abortZnode); // Make sure we trigger the watches on these nodes by creating them. (HBASE-13885) - ZKUtil.createAndFailSilent(watcher, getAcquiredBarrierNode(procedureName)); - ZKUtil.createAndFailSilent(watcher, getAbortZNode(procedureName)); + String acquiredBarrierNode = getAcquiredBarrierNode(procedureName); + String reachedBarrierNode = getReachedBarrierNode(procedureName); + String abortZNode = getAbortZNode(procedureName); - ZKUtil.deleteNodeRecursively(watcher, getAcquiredBarrierNode(procedureName)); - ZKUtil.deleteNodeRecursively(watcher, getReachedBarrierNode(procedureName)); - ZKUtil.deleteNodeRecursively(watcher, getAbortZNode(procedureName)); + ZKUtil.createAndFailSilent(watcher, acquiredBarrierNode); + ZKUtil.createAndFailSilent(watcher, abortZNode); + + ZKUtil.deleteNodeRecursivelyMultiOrSequential(watcher, true, acquiredBarrierNode, + reachedBarrierNode, abortZNode); } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKMulti.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKMulti.java index b4b1f9b..15bce62 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKMulti.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKMulti.java @@ -339,6 +339,80 @@ public class TestZKMulti { } } + /** + * Verifies that for the given root node, it should delete all the nodes recursively using + * multi-update api. + */ + @Test(timeout = 60000) + public void testDeleteNodeRecursivelyMulti() throws Exception { + String parentZNode = "/testdeleteNodeRecursivelyMulti"; + createZNodeTree(parentZNode); + + ZKUtil.deleteNodeRecursively(zkw, parentZNode); + assertTrue("Parent znode should be deleted.", ZKUtil.checkExists(zkw, parentZNode) == -1); + } + + /** + * Verifies that for the given root node, it should delete all the nodes recursively using + * normal sequential way. + */ + @Test(timeout = 60000) + public void testDeleteNodeRecursivelySequential() throws Exception { + String parentZNode = "/testdeleteNodeRecursivelySequential"; + createZNodeTree(parentZNode); + boolean useMulti = zkw.getConfiguration().getBoolean("hbase.zookeeper.useMulti", false); + zkw.getConfiguration().setBoolean("hbase.zookeeper.useMulti", false); + try { + // disables the multi-update api execution + ZKUtil.deleteNodeRecursively(zkw, parentZNode); + assertTrue("Parent znode should be deleted.", ZKUtil.checkExists(zkw, parentZNode) == -1); + } finally { + // sets back the multi-update api execution + zkw.getConfiguration().setBoolean("hbase.zookeeper.useMulti", useMulti); + } + } + + @Test(timeout = 60000) + public void testDeleteNodeRecursivelyMultiOrSequential() throws Exception { + String parentZNode1 = "/testdeleteNode1"; + String parentZNode2 = "/testdeleteNode2"; + String parentZNode3 = "/testdeleteNode3"; + createZNodeTree(parentZNode1); + createZNodeTree(parentZNode2); + createZNodeTree(parentZNode3); + + ZKUtil.deleteNodeRecursivelyMultiOrSequential(zkw, false, parentZNode1, parentZNode2, + parentZNode3); + assertTrue("Parent znode 1 should be deleted.", ZKUtil.checkExists(zkw, parentZNode1) == -1); + assertTrue("Parent znode 2 should be deleted.", ZKUtil.checkExists(zkw, parentZNode2) == -1); + assertTrue("Parent znode 3 should be deleted.", ZKUtil.checkExists(zkw, parentZNode3) == -1); + } + + @Test(timeout = 60000) + public void testDeleteChildrenRecursivelyMultiOrSequential() throws Exception { + String parentZNode1 = "/testdeleteChildren1"; + String parentZNode2 = "/testdeleteChildren2"; + String parentZNode3 = "/testdeleteChildren3"; + createZNodeTree(parentZNode1); + createZNodeTree(parentZNode2); + createZNodeTree(parentZNode3); + + ZKUtil.deleteChildrenRecursivelyMultiOrSequential(zkw, true, parentZNode1, parentZNode2, + parentZNode3); + + assertTrue("Wrongly deleted parent znode 1!", ZKUtil.checkExists(zkw, parentZNode1) > -1); + List children = zkw.getRecoverableZooKeeper().getChildren(parentZNode1, false); + assertTrue("Failed to delete child znodes of parent znode 1!", 0 == children.size()); + + assertTrue("Wrongly deleted parent znode 2!", ZKUtil.checkExists(zkw, parentZNode2) > -1); + children = zkw.getRecoverableZooKeeper().getChildren(parentZNode2, false); + assertTrue("Failed to delete child znodes of parent znode 1!", 0 == children.size()); + + assertTrue("Wrongly deleted parent znode 3!", ZKUtil.checkExists(zkw, parentZNode3) > -1); + children = zkw.getRecoverableZooKeeper().getChildren(parentZNode3, false); + assertTrue("Failed to delete child znodes of parent znode 1!", 0 == children.size()); + } + private void createZNodeTree(String rootZNode) throws KeeperException, InterruptedException { List opList = new ArrayList(); -- 1.9.2.msysgit.0