diff options
author | Mohammad Arshad <arshad@apache.org> | 2022-04-06 13:27:16 +0530 |
---|---|---|
committer | Mohammad Arshad <arshad@apache.org> | 2022-04-06 13:27:16 +0530 |
commit | 54cb5c39a445c63967ec7c3a46724fe87908440b (patch) | |
tree | 94a8e9b561cfb7a785910e33ebcd862db40254f9 | |
parent | 5b12b0e4eb010dca9b46047f559c6847d7e0b5f7 (diff) | |
download | zookeeper-54cb5c39a445c63967ec7c3a46724fe87908440b.tar.gz |
ZOOKEEPER-4504: ZKUtil#deleteRecursive causing deadlock in HDFS HA functionality
Make ZKUtil#deleteRecursive API fully compatible with older versions
Author: Mohammad Arshad <arshad@apache.org>
Reviewers: Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org>
Closes #1843 from arshadmohammad/ZOOKEEPER-4504-DeleteRecursive and squashes the following commits:
851bb1ee9 [Mohammad Arshad] Added javadoc for ZKUtil#deleteRecursive(zk, pathRoot, batchSize) API
e7b33116c [Mohammad Arshad] Added test case to verify ZKUtil.deleteRecursive() in sync and async mode
008b2bd4a [Mohammad Arshad] ZOOKEEPER-4504: ZKUtil#deleteRecursive causing deadlock in HDFS HA functionality
-rw-r--r-- | zookeeper-server/src/main/java/org/apache/zookeeper/ZKUtil.java | 22 | ||||
-rw-r--r-- | zookeeper-server/src/test/java/org/apache/zookeeper/ZKUtilTest.java | 50 |
2 files changed, 69 insertions, 3 deletions
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/ZKUtil.java b/zookeeper-server/src/main/java/org/apache/zookeeper/ZKUtil.java index d20634144..2ccdfc877 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/ZKUtil.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/ZKUtil.java @@ -49,7 +49,14 @@ public class ZKUtil { * If there is an error with deleting one of the sub-nodes in the tree, * this operation would abort and would be the responsibility of the app to handle the same. * - * + * @param zk Zookeeper client + * @param pathRoot path to be deleted + * @param batchSize number of delete operations to be submitted in one call. + * batchSize is also used to decide sync and async delete API invocation. + * If batchSize>0 then async otherwise sync delete API is invoked. batchSize>0 + * gives better performance. batchSize<=0 scenario is handled to preserve + * backward compatibility. + * @return true if given node and all its sub nodes are deleted successfully otherwise false * @throws IllegalArgumentException if an invalid path is specified */ public static boolean deleteRecursive( @@ -61,7 +68,15 @@ public class ZKUtil { List<String> tree = listSubTreeBFS(zk, pathRoot); LOG.debug("Deleting tree: {}", tree); - return deleteInBatch(zk, tree, batchSize); + if (batchSize > 0) { + return deleteInBatch(zk, tree, batchSize); + } else { + for (int i = tree.size() - 1; i >= 0; --i) { + //Delete the leaves first and eventually get rid of the root + zk.delete(tree.get(i), -1); //Delete all versions of the node with -1. + } + return true; + } } /** @@ -73,7 +88,8 @@ public class ZKUtil { public static void deleteRecursive( ZooKeeper zk, final String pathRoot) throws InterruptedException, KeeperException { - deleteRecursive(zk, pathRoot, 1000); + // batchSize=0 is passed to preserve the backward compatibility with older clients. + deleteRecursive(zk, pathRoot, 0); } private static class BatchedDeleteCbContext { diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/ZKUtilTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/ZKUtilTest.java index 51df90702..a73ff2fa7 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/ZKUtilTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/ZKUtilTest.java @@ -25,10 +25,12 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import java.io.File; import java.io.IOException; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.UUID; +import org.apache.zookeeper.data.Stat; import org.apache.zookeeper.test.ClientBase; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -119,4 +121,52 @@ public class ZKUtilTest extends ClientBase { assertEquals(1, bList.size()); assertIterableEquals(Collections.singletonList("/a/b"), bList); } + + @Test + public void testDeleteRecursiveInAsyncMode() throws Exception { + int batchSize = 10; + testDeleteRecursiveInSyncAsyncMode(batchSize); + } + + @Test + public void testDeleteRecursiveInSyncMode() throws Exception { + int batchSize = 0; + testDeleteRecursiveInSyncAsyncMode(batchSize); + } + + // batchSize>0 is async mode otherwise it is sync mode + private void testDeleteRecursiveInSyncAsyncMode(int batchSize) + throws IOException, InterruptedException, KeeperException { + TestableZooKeeper zk = createClient(); + String parentPath = "/a"; + zk.create(parentPath, "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + int numberOfNodes = 50; + List<Op> ops = new ArrayList<>(); + for (int i = 0; i < numberOfNodes; i++) { + ops.add(Op.create(parentPath + "/a" + i, "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, + CreateMode.PERSISTENT)); + } + zk.multi(ops); + ops.clear(); + + // check nodes create successfully + List<String> children = zk.getChildren(parentPath, false); + assertEquals(numberOfNodes, children.size()); + + // create one more level of z nodes + String subNode = "/a/a0"; + for (int i = 0; i < numberOfNodes; i++) { + ops.add(Op.create(subNode + "/b" + i, "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, + CreateMode.PERSISTENT)); + } + zk.multi(ops); + + // check sub nodes created successfully + children = zk.getChildren(subNode, false); + assertEquals(numberOfNodes, children.size()); + + ZKUtil.deleteRecursive(zk, parentPath, batchSize); + Stat exists = zk.exists(parentPath, false); + assertNull(exists, "ZKUtil.deleteRecursive() could not delete all the z nodes"); + } } |