You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zookeeper.apache.org by ar...@apache.org on 2022/04/06 08:17:26 UTC
[zookeeper] branch branch-3.7 updated: ZOOKEEPER-4504: ZKUtil#deleteRecursive causing deadlock in HDFS HA functionality
This is an automated email from the ASF dual-hosted git repository.
arshad pushed a commit to branch branch-3.7
in repository https://gitbox.apache.org/repos/asf/zookeeper.git
The following commit(s) were added to refs/heads/branch-3.7 by this push:
new 168b62151 ZOOKEEPER-4504: ZKUtil#deleteRecursive causing deadlock in HDFS HA functionality
168b62151 is described below
commit 168b6215157d2511567b81ec1651c663230914cd
Author: Mohammad Arshad <ar...@apache.org>
AuthorDate: Wed Apr 6 13:27:16 2022 +0530
ZOOKEEPER-4504: ZKUtil#deleteRecursive causing deadlock in HDFS HA functionality
Make ZKUtil#deleteRecursive API fully compatible with older versions
Author: Mohammad Arshad <ar...@apache.org>
Reviewers: Enrico Olivelli <eo...@apache.org>, Mate Szalay-Beko <sy...@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
(cherry picked from commit 54cb5c39a445c63967ec7c3a46724fe87908440b)
Signed-off-by: Mohammad Arshad <ar...@apache.org>
---
.../src/main/java/org/apache/zookeeper/ZKUtil.java | 22 +++++++--
.../test/java/org/apache/zookeeper/ZKUtilTest.java | 55 +++++++++++++++++++++-
2 files changed, 73 insertions(+), 4 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 2e29cc75c..48ac142dc 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 190465386..7ad96b6ae 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/ZKUtilTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/ZKUtilTest.java
@@ -24,11 +24,16 @@ 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.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;
-public class ZKUtilTest {
+public class ZKUtilTest extends ClientBase {
private static final File testData = new File(System.getProperty("test.data.dir", "build/test/data"));
@@ -85,4 +90,52 @@ public class ZKUtilTest {
assertEquals(expectedMessage, error);
}
+
+ @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");
+ }
}