diff --git a/zookeeper-docs/src/main/resources/markdown/zookeeperCLI.md b/zookeeper-docs/src/main/resources/markdown/zookeeperCLI.md index 53da2c435ab..5af2108b55c 100644 --- a/zookeeper-docs/src/main/resources/markdown/zookeeperCLI.md +++ b/zookeeper-docs/src/main/resources/markdown/zookeeperCLI.md @@ -43,7 +43,7 @@ ZooKeeper -server host:port cmd args connect host:port create [-s] [-e] [-c] [-t ttl] path [data] [acl] delete [-v version] path - deleteall path + deleteall path [-b batch size] [-c] delquota [-n|-b] path get [-s] [-w] path getAcl [-s] path @@ -181,6 +181,26 @@ zkshell: 1] ls /config Node does not exist: /config ``` +Use `-b ` to set the delete batch size (default 1000). Use `-c` to delete only the +descendants and keep the node itself. With `-c` children are listed with pagination, so it also works on +nodes whose children list exceeds `jute.maxbuffer`; this requires a server that supports paginated +`getChildren`, and the client must be started with a `jute.maxbuffer` at least as large as the server's, +since each page is bounded by the server's limit. `-c` is rejected for `/` unless the client is connected +with a chroot. Quiesce writers to the subtree first: children created or deleted while the command runs +can make it report that children remain, or abort it part-way. Re-running is safe. If children remain +when the command finishes, `-c` reports an error and exits non-zero. The subtree is listed in client +memory before the first delete, at one round trip per node, so draining a node with very many +descendants needs a larger client heap (`ZK_CLIENT_HEAP`, default `-Xmx256m`) and takes time +proportional to the number of descendants. + +```bash +zkshell: 1] ls /config/topics + [a, b] +[zkshell: 2] deleteall /config/topics -c +[zkshell: 3] ls /config/topics + [] +``` + ## delquota Delete the quota under a path 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 19e16fb76d2..9b35190a236 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/ZKUtil.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/ZKUtil.java @@ -34,6 +34,7 @@ import org.apache.zookeeper.KeeperException.Code; import org.apache.zookeeper.common.PathUtils; import org.apache.zookeeper.data.ACL; +import org.apache.zookeeper.data.Stat; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -41,6 +42,17 @@ public class ZKUtil { private static final Logger LOG = LoggerFactory.getLogger(ZKUtil.class); private static final Map permCache = new ConcurrentHashMap(); + /** + * Child count above which {@link #deleteChildrenRecursive(ZooKeeper, String, int)} warns that the + * listing it is about to build may not fit in the client's heap. zkCli defaults to -Xmx256m + * (ZK_CLIENT_HEAP in bin/zkEnv.sh), and the subtree is materialized client-side before any delete + * is issued, so a node with enough children can exhaust that heap. The threshold is only a hint: + * it counts direct children, which is exact for the flat nodes this option targets but an + * underestimate for a deep subtree. It sits an order of magnitude above the documented guideline + * of under 10,000 children per parent, so it fires only for nodes already well outside it -- the + * pathological nodes this option exists to drain. + */ + private static final int LARGE_CHILD_WARN_THRESHOLD = 100_000; /** * Recursively delete the node with the given path. *

@@ -69,7 +81,7 @@ public static boolean deleteRecursive( LOG.debug("Deleting tree: {}", tree); if (batchSize > 0) { - return deleteInBatch(zk, tree, batchSize); + return deleteInBatch(zk, tree, batchSize, 0); } else { for (int i = tree.size() - 1; i >= 0; --i) { //Delete the leaves first and eventually get rid of the root @@ -92,6 +104,83 @@ public static void deleteRecursive( deleteRecursive(zk, pathRoot, 0); } + /** + * Recursively delete all descendants of the node with the given path, keeping the node itself. + *

+ * Unlike {@link #deleteRecursive(ZooKeeper, String, int)}, the subtree is listed using + * {@link ZooKeeper#getAllChildrenPaginated(String, boolean)}, so it also works on nodes whose + * children list does not fit in a single {@code jute.maxbuffer} sized response. This requires a + * server that supports paginated getChildren. Note that each page is only bounded by the + * server's {@code jute.maxbuffer}, while the client enforces its own limit on the incoming + * packet, so the client's {@code jute.maxbuffer} must be at least as large as the server's. + *

+ * The subtree is listed into client memory before the first delete is issued, so draining a node + * with a very large number of children needs a correspondingly large client heap. zkCli defaults + * to {@code -Xmx256m} ({@code ZK_CLIENT_HEAP} in {@code bin/zkEnv.sh}); raise it if the client + * runs out of memory. A warning is logged above {@value #LARGE_CHILD_WARN_THRESHOLD} children. + *

+ * The listing also costs one sequential round trip per node, leaves included, so the time to + * drain a node grows with the number of descendants rather than with the data they hold. + *

+ * 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. + * Children created or deleted concurrently with this call can cause it to return false or to + * abort part-way, so writers to the subtree should be quiesced first. + * + * @param zk Zookeeper client + * @param pathRoot path of the node whose descendants are to be deleted + * @param batchSize number of delete operations to be submitted in one call; see + * {@link #deleteRecursive(ZooKeeper, String, int)}. + * @return true if all descendants are deleted successfully and the node has no children + * left afterwards, otherwise false (for example if a child was created concurrently). + * Returns true if the node has no children. + * @throws IllegalArgumentException if an invalid path is specified, or if the path is the root + * of the server's namespace ("/" on a client without a chroot). The root can never be + * emptied because its children include the protected /zookeeper subtree, so this is + * rejected unconditionally and the check must not be relaxed. Paginated listing of "/" + * is additionally broken on servers without the root-pagination fix. + */ + public static boolean deleteChildrenRecursive( + ZooKeeper zk, + final String pathRoot, + final int batchSize) throws InterruptedException, KeeperException { + PathUtils.validatePath(pathRoot); + if ("/".equals(pathRoot) && zk.cnxn.chrootPath == null) { + throw new IllegalArgumentException("Deleting all children of the root node is not supported"); + } + + // The whole subtree is listed into memory before the first delete is issued, so warn before + // building a listing that may not fit in the client's heap. See LARGE_CHILD_WARN_THRESHOLD. + Stat before = zk.exists(pathRoot, false); + if (before != null && before.getNumChildren() > LARGE_CHILD_WARN_THRESHOLD) { + LOG.warn("Draining {} children of {}. The subtree is listed in client memory before any" + + " delete is issued; raise ZK_CLIENT_HEAP (zkCli defaults to -Xmx256m) if the" + + " client runs out of memory.", before.getNumChildren(), pathRoot); + } + + List tree = listSubTreeBFS(zk, pathRoot, true); + LOG.debug("Deleting children of {}: {}", pathRoot, tree); + + // tree.get(0) is always the root, which is kept + if (tree.size() > 1) { + if (batchSize > 0) { + if (!deleteInBatch(zk, tree, batchSize, 1)) { + return false; + } + } else { + for (int i = tree.size() - 1; i >= 1; --i) { + zk.delete(tree.get(i), -1); //Delete all versions of the node with -1. + } + } + } + // The root is kept, so unlike deleteRecursive nothing fails if a child was created after the + // listing. Verify using the server's own child count rather than another listing, so that an + // incomplete listing is not reported as success. This is a point-in-time read of the + // contacted server, not a lock, so a create racing this check can still be missed. + Stat stat = zk.exists(pathRoot, false); + return stat != null && stat.getNumChildren() == 0; + } + private static class BatchedDeleteCbContext { public Semaphore sem; @@ -104,7 +193,9 @@ public BatchedDeleteCbContext(int rateLimit) { } - private static boolean deleteInBatch(ZooKeeper zk, List tree, int batchSize) throws InterruptedException { + // Deletes tree[minIndex..] leaves first; minIndex 0 includes the root. + private static boolean deleteInBatch(ZooKeeper zk, List tree, int batchSize, int minIndex) + throws InterruptedException { int rateLimit = 10; List ops = new ArrayList<>(); BatchedDeleteCbContext context = new BatchedDeleteCbContext(rateLimit); @@ -116,11 +207,11 @@ private static boolean deleteInBatch(ZooKeeper zk, List tree, int batchS }; // Delete the leaves first and eventually get rid of the root - for (int i = tree.size() - 1; i >= 0; --i) { + for (int i = tree.size() - 1; i >= minIndex; --i) { // Create Op to delete all versions of the node with -1. ops.add(Op.delete(tree.get(i), -1)); - if (ops.size() == batchSize || i == 0) { + if (ops.size() == batchSize || i == minIndex) { if (!context.success.get()) { // fail fast break; @@ -201,15 +292,36 @@ public static String validateFileInput(String filePath) { public static List listSubTreeBFS( ZooKeeper zk, final String pathRoot) throws KeeperException, InterruptedException { + return listSubTreeBFS(zk, pathRoot, false); + } + + /** + * Same as {@link #listSubTreeBFS(ZooKeeper, String)} but optionally lists the children of each + * node using {@link ZooKeeper#getAllChildrenPaginated(String, boolean)}, which works for nodes + * whose children list exceeds {@code jute.maxbuffer}. This requires a server that supports + * paginated getChildren. + *

+ * Deliberately not public: with {@code paginated} set, passing the root of the server's + * namespace ("/" on a client without a chroot) can silently return an incomplete subtree on + * servers that do not handle paginated listing of "/". Callers must reject that case first, as + * {@link #deleteChildrenRecursive(ZooKeeper, String, int)} does. + * + * @param paginated true to list children with pagination + */ + static List listSubTreeBFS( + ZooKeeper zk, + final String pathRoot, + boolean paginated) throws KeeperException, InterruptedException { Queue queue = new ArrayDeque<>(); List tree = new ArrayList(); queue.add(pathRoot); tree.add(pathRoot); while (!queue.isEmpty()) { String node = queue.poll(); - List children = zk.getChildren(node, false); + List children = paginated ? zk.getAllChildrenPaginated(node, false) : zk.getChildren(node, false); for (final String child : children) { - final String childPath = node + "/" + child; + // the legacy listing builds "//child" for the root; only the paginated listing handles "/" + final String childPath = paginated && "/".equals(node) ? "/" + child : node + "/" + child; queue.add(childPath); tree.add(childPath); } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/cli/DeleteAllCommand.java b/zookeeper-server/src/main/java/org/apache/zookeeper/cli/DeleteAllCommand.java index 537a9b05ba6..6b001e1df54 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/cli/DeleteAllCommand.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/cli/DeleteAllCommand.java @@ -37,6 +37,7 @@ public class DeleteAllCommand extends CliCommand { static { options.addOption(new Option("b", true, "batch size")); + options.addOption(new Option("c", false, "delete only the children, keep the node itself")); } public DeleteAllCommand() { @@ -44,7 +45,7 @@ public DeleteAllCommand() { } public DeleteAllCommand(String cmdStr) { - super(cmdStr, "path [-b batch size]"); + super(cmdStr, "path [-b batch size] [-c]"); } @Override @@ -74,8 +75,19 @@ public boolean exec() throws CliException { String path = args[1]; try { - boolean success = ZKUtil.deleteRecursive(zk, path, batchSize); + boolean success = cl.hasOption("c") + ? ZKUtil.deleteChildrenRecursive(zk, path, batchSize) + : ZKUtil.deleteRecursive(zk, path, batchSize); if (!success) { + if (cl.hasOption("c")) { + // The node itself is kept, so "false" here means children are still present: + // either a delete failed, or a writer created new children during the delete. + // Thrown rather than printed so the process exits non-zero: ZooKeeperMain only + // sets a failing exit code when a CliException escapes. + throw new CliException("Failed to delete all children of " + path + + ": child nodes remain (a delete may have failed, or children may have " + + "been created concurrently)."); + } err.println("Failed to delete some node(s) in the subtree!"); } } catch (IllegalArgumentException ex) { 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 2259ed9c7c6..376833b94a3 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/ZKUtilTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/ZKUtilTest.java @@ -19,14 +19,23 @@ package org.apache.zookeeper; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.junit.Assume.assumeTrue; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.spy; import java.io.File; import java.io.IOException; import java.util.ArrayList; import java.util.List; import java.util.UUID; +import java.util.concurrent.atomic.AtomicInteger; +import org.apache.jute.BinaryInputArchive; +import org.apache.zookeeper.common.PathUtils; import org.apache.zookeeper.data.Stat; import org.apache.zookeeper.test.ClientBase; import org.junit.BeforeClass; @@ -136,4 +145,228 @@ private void testDeleteRecursiveInSyncAsyncMode(int batchSize) Stat exists = zk.exists(parentPath, false); assertNull("ZKUtil.deleteRecursive() could not delete all the z nodes", exists); } + + @Test + public void testDeleteChildrenRecursiveKeepsRootInAsyncMode() throws Exception { + testDeleteChildrenRecursiveKeepsRoot(10); + } + + @Test + public void testDeleteChildrenRecursiveKeepsRootInSyncMode() throws Exception { + testDeleteChildrenRecursiveKeepsRoot(0); + } + + private void testDeleteChildrenRecursiveKeepsRoot(int batchSize) throws Exception { + TestableZooKeeper zk = createClient(); + String parentPath = "/keep"; + byte[] data = "payload".getBytes(); + zk.create(parentPath, data, ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + List ops = new ArrayList<>(); + for (int i = 0; i < 25; i++) { + ops.add(Op.create(parentPath + "/c" + i, "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, + CreateMode.PERSISTENT)); + } + zk.multi(ops); + zk.create(parentPath + "/c0/nested", "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + + assertTrue(ZKUtil.deleteChildrenRecursive(zk, parentPath, batchSize)); + + assertNotNull("root must be kept", zk.exists(parentPath, false)); + assertEquals(0, zk.getChildren(parentPath, false).size()); + assertEquals("root data must be untouched", new String(data), new String(zk.getData(parentPath, false, null))); + } + + @Test + public void testDeleteChildrenRecursiveOnChildlessRoot() throws Exception { + TestableZooKeeper zk = createClient(); + zk.create("/empty", "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + + assertTrue(ZKUtil.deleteChildrenRecursive(zk, "/empty", 10)); + assertTrue(ZKUtil.deleteChildrenRecursive(zk, "/empty", 0)); + + assertNotNull(zk.exists("/empty", false)); + } + + @Test(timeout = 120000) + public void testDeleteChildrenRecursiveBeyondJuteMaxBuffer() throws Exception { + TestableZooKeeper zk = createClient(); + String parentPath = "/huge"; + zk.create(parentPath, "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + + // enough children for a plain getChildren response to exceed jute.maxbuffer + int numChildren = BinaryInputArchive.maxBuffer / UUID.randomUUID().toString().length() + 1; + for (int i = 0; i < numChildren; i += 1000) { + List ops = new ArrayList<>(); + for (int j = i; j < i + 1000 && j < numChildren; j++) { + ops.add(Op.create(parentPath + "/" + UUID.randomUUID(), "".getBytes(), + ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT)); + } + zk.multi(ops); + } + + assertTrue(ZKUtil.deleteChildrenRecursive(zk, parentPath, 1000)); + + assertNotNull(zk.exists(parentPath, false)); + assertEquals(0, zk.getChildren(parentPath, false).size()); + } + + @Test + public void testDeleteChildrenRecursiveBatchBoundaries() throws Exception { + TestableZooKeeper zk = createClient(); + // tree of 1 root + 5 children + 1 grandchild = 6 descendants + // batch sizes: 1, divides evenly (2, 3, 6), does not divide evenly (4), larger than tree (100) + for (int batchSize : new int[] {1, 2, 3, 4, 6, 100}) { + String root = "/boundary" + batchSize; + zk.create(root, "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + for (int i = 0; i < 5; i++) { + zk.create(root + "/c" + i, "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + } + zk.create(root + "/c0/g", "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + + assertTrue("batchSize=" + batchSize, ZKUtil.deleteChildrenRecursive(zk, root, batchSize)); + + assertNotNull("root must be kept, batchSize=" + batchSize, zk.exists(root, false)); + assertEquals("batchSize=" + batchSize, 0, zk.getChildren(root, false).size()); + } + } + + @Test + public void testDeleteChildrenRecursiveDoesNotTouchSiblingsOrParent() throws Exception { + TestableZooKeeper zk = createClient(); + zk.create("/p", "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + zk.create("/p/target", "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + zk.create("/p/target/x", "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + zk.create("/p/sibling", "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + zk.create("/p/sibling/y", "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + + assertTrue(ZKUtil.deleteChildrenRecursive(zk, "/p/target", 10)); + + assertNotNull(zk.exists("/p/target", false)); + assertNull(zk.exists("/p/target/x", false)); + assertNotNull(zk.exists("/p/sibling/y", false)); + } + + @Test(expected = KeeperException.NoNodeException.class) + public void testDeleteChildrenRecursiveOnMissingNode() throws Exception { + ZKUtil.deleteChildrenRecursive(createClient(), "/does-not-exist", 10); + } + + @Test(expected = IllegalArgumentException.class) + public void testDeleteChildrenRecursiveInvalidPath() throws Exception { + ZKUtil.deleteChildrenRecursive(createClient(), "no-leading-slash", 10); + } + + @Test + public void testListSubTreeBFSPaginatedMatchesDefault() throws Exception { + TestableZooKeeper zk = createClient(); + zk.create("/t", "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + for (int i = 0; i < 20; i++) { + zk.create("/t/c" + i, "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + zk.create("/t/c" + i + "/g", "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + } + + List legacy = ZKUtil.listSubTreeBFS(zk, "/t"); + List paginated = ZKUtil.listSubTreeBFS(zk, "/t", true); + + assertEquals(41, legacy.size()); + assertEquals("/t", legacy.get(0)); + assertEquals("/t", paginated.get(0)); + assertEquals(new java.util.HashSet<>(legacy), new java.util.HashSet<>(paginated)); + assertEquals(legacy, ZKUtil.listSubTreeBFS(zk, "/t", false)); + } + + @Test + public void testDeleteRecursiveStillDeletesRoot() throws Exception { + TestableZooKeeper zk = createClient(); + zk.create("/r", "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + zk.create("/r/c", "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + + assertTrue(ZKUtil.deleteRecursive(zk, "/r", 10)); + assertNull(zk.exists("/r", false)); + + // an empty (leaf) node is also deleted, as before + zk.create("/leaf", "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + assertTrue(ZKUtil.deleteRecursive(zk, "/leaf", 10)); + assertNull(zk.exists("/leaf", false)); + } + + @Test + public void testListSubTreeBFSPaginatedFromRootBuildsValidPaths() throws Exception { + TestableZooKeeper zk = createClient(); + zk.create("/rootchild", "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + zk.create("/rootchild/g", "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + + List paginated = ZKUtil.listSubTreeBFS(zk, "/", true); + + assertEquals("/", paginated.get(0)); + assertTrue(paginated.contains("/rootchild")); + assertTrue(paginated.contains("/rootchild/g")); + for (String path : paginated) { + PathUtils.validatePath(path); + } + } + + @Test + public void testDeleteChildrenRecursiveRejectsUnchrootedRoot() throws Exception { + TestableZooKeeper zk = createClient(); + zk.create("/a", "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + zk.create("/a/b", "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + + for (int batchSize : new int[] {0, 10}) { + try { + ZKUtil.deleteChildrenRecursive(zk, "/", batchSize); + fail("expected IllegalArgumentException"); + } catch (IllegalArgumentException expected) { + // expected + } + } + + // nothing may have been deleted + assertNotNull(zk.exists("/a/b", false)); + assertEquals(2, zk.getChildren("/", false).size()); + } + + @Test + public void testDeleteChildrenRecursiveOnChrootedRoot() throws Exception { + TestableZooKeeper plain = createClient(); + plain.create("/chroot", "keep".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + plain.create("/outside", "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + TestableZooKeeper chrooted = createClient(hostPort + "/chroot"); + + for (int batchSize : new int[] {0, 10}) { + for (int i = 0; i < 3; i++) { + chrooted.create("/x" + i, "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + chrooted.create("/x" + i + "/y", "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + } + + assertTrue(ZKUtil.deleteChildrenRecursive(chrooted, "/", batchSize)); + + assertEquals(0, chrooted.getChildren("/", false).size()); + assertEquals("keep", new String(plain.getData("/chroot", false, null))); + assertNotNull(plain.exists("/outside", false)); + } + } + + @Test + public void testDeleteChildrenRecursiveReportsChildCreatedConcurrently() throws Exception { + TestableZooKeeper zk = createClient(); + zk.create("/p", "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + zk.create("/p/a", "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + + // deleteChildrenRecursive calls exists("/p", false) twice: once for the large-child warning + // before the subtree is listed, and once for the final check. Create the child only before + // the final one, so the new child is not picked up by the listing. + AtomicInteger existsCalls = new AtomicInteger(); + TestableZooKeeper spied = spy(zk); + doAnswer(invocation -> { + if (existsCalls.incrementAndGet() == 2) { + zk.create("/p/late", "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + } + return invocation.callRealMethod(); + }).when(spied).exists(eq("/p"), eq(false)); + + assertFalse(ZKUtil.deleteChildrenRecursive(spied, "/p", 10)); + + assertEquals(1, zk.getChildren("/p", false).size()); + } } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java index 48e2468d161..9602bd8fcd8 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java @@ -20,9 +20,13 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.spy; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.PrintStream; @@ -160,6 +164,117 @@ public void testDeleteRecursiveCli() throws IOException, InterruptedException, C assertNull(zk.exists("/a", null)); } + @Test + public void testDeleteChildrenOnlyCli() throws IOException, InterruptedException, CliException, KeeperException { + final ZooKeeper zk = createClient(); + zk.create("/a", "keep".getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + zk.create("/a/b", "some".getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + zk.create("/a/b/v", "some".getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + zk.create("/a/c", "some".getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + + ZooKeeperMain zkMain = new ZooKeeperMain(zk); + zkMain.cl.parseCommand("deleteall /a -c -b 2"); + assertFalse(zkMain.processZKCmd(zkMain.cl)); + + assertNotNull(zk.exists("/a", null)); + assertEquals(0, zk.getChildren("/a", false).size()); + assertEquals("keep", new String(zk.getData("/a", false, null))); + } + + @Test + public void testDeleteChildrenOnlyCliFailsWhenChildrenRemain() + throws IOException, InterruptedException, KeeperException { + final TestableZooKeeper zk = createClient(); + zk.create("/a", "keep".getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + zk.create("/a/b", "x".getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + + // exists("/a", false) is called once for the large-child warning and once for the final + // check; create a child just before the final check so the listing misses it. + AtomicInteger existsCalls = new AtomicInteger(); + TestableZooKeeper spied = spy(zk); + doAnswer(invocation -> { + if (existsCalls.incrementAndGet() == 2) { + zk.create("/a/late", "x".getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + } + return invocation.callRealMethod(); + }).when(spied).exists(eq("/a"), eq(false)); + + ZooKeeperMain zkMain = new ZooKeeperMain(spied); + zkMain.cl.parseCommand("deleteall /a -c"); + try { + zkMain.processZKCmd(zkMain.cl); + fail("expected a CliException so that zkCli exits non-zero"); + } catch (CliException e) { + assertTrue("exit code must be non-zero", e.getExitCode() != 0); + assertTrue(e.getMessage().contains("child nodes remain")); + } + + assertEquals(1, zk.getChildren("/a", false).size()); + } + + @Test + public void testDeleteAllWithoutChildrenOptionStillDeletesNode() + throws IOException, InterruptedException, CliException, KeeperException { + final ZooKeeper zk = createClient(); + zk.create("/a", "x".getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + zk.create("/a/b", "x".getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + + ZooKeeperMain zkMain = new ZooKeeperMain(zk); + zkMain.cl.parseCommand("deleteall /a -b 5"); + assertFalse(zkMain.processZKCmd(zkMain.cl)); + + assertNull(zk.exists("/a", null)); + } + + @Test + public void testDeleteChildrenOnlyCliRejectsRoot() throws IOException, InterruptedException, KeeperException { + final ZooKeeper zk = createClient(); + zk.create("/a", "x".getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + zk.create("/a/b", "x".getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + + ZooKeeperMain zkMain = new ZooKeeperMain(zk); + zkMain.cl.parseCommand("deleteall / -c"); + try { + zkMain.processZKCmd(zkMain.cl); + fail("expected a malformed path error"); + } catch (CliException e) { + assertTrue(e instanceof MalformedPathException); + } + + assertNotNull(zk.exists("/a/b", null)); + } + + @Test + public void testDeleteAllRootWithoutChildrenOptionIsUnchanged() throws IOException, InterruptedException, KeeperException { + final ZooKeeper zk = createClient(); + zk.create("/a", "x".getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + zk.create("/a/b", "x".getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + + ZooKeeperMain zkMain = new ZooKeeperMain(zk); + zkMain.cl.parseCommand("deleteall /"); + try { + zkMain.processZKCmd(zkMain.cl); + fail("expected a malformed path error"); + } catch (CliException e) { + assertTrue(e instanceof MalformedPathException); + } + + assertNotNull(zk.exists("/a/b", null)); + } + + @Test + public void testDeleteChildrenOnlyCliOnMissingNode() throws IOException, InterruptedException { + final ZooKeeper zk = createClient(); + ZooKeeperMain zkMain = new ZooKeeperMain(zk); + zkMain.cl.parseCommand("deleteall /missing -c"); + try { + zkMain.processZKCmd(zkMain.cl); + fail("expected NoNode"); + } catch (CliException e) { + assertTrue(e.getCause() instanceof KeeperException.NoNodeException); + } + } + @Test public void testDeleteRecursiveAsync() throws IOException, InterruptedException, KeeperException { final ZooKeeper zk = createClient();