diff --git a/helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java b/helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java index 15e532d6f9..8d6264c1c1 100644 --- a/helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java +++ b/helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java @@ -273,10 +273,12 @@ public void dropInstance(String clusterName, InstanceConfig instanceConfig) { "Node " + instanceName + " is still alive for cluster " + clusterName + ", can't drop."); } - dropInstancePathsRecursively(instanceName, instancePath, instanceConfigPath); + dropInstancePathsRecursively(clusterName, instanceName); } - private void dropInstancePathsRecursively(String instanceName, String instancePath, String instanceConfigPath) { + private void dropInstancePathsRecursively(String clusterName, String instanceName) { + String instanceConfigPath = PropertyPathBuilder.instanceConfig(clusterName, instanceName); + String instancePath = PropertyPathBuilder.instance(clusterName, instanceName); int retryCnt = 0; while (true) { try { @@ -328,9 +330,7 @@ public void purgeOfflineInstances(String clusterName, long offlineDuration) { private void purgeInstance(String clusterName, String instanceName) { logger.info("Purge instance {} from cluster {}.", instanceName, clusterName); - String instanceConfigPath = PropertyPathBuilder.instanceConfig(clusterName, instanceName); - String instancePath = PropertyPathBuilder.instance(clusterName, instanceName); - dropInstancePathsRecursively(instanceName, instancePath, instanceConfigPath); + dropInstancePathsRecursively(clusterName, instanceName); } @Override diff --git a/helix-core/src/test/java/org/apache/helix/manager/zk/TestZkBaseDataAccessor.java b/helix-core/src/test/java/org/apache/helix/manager/zk/TestZkBaseDataAccessor.java index ca2dc06892..56337f5477 100644 --- a/helix-core/src/test/java/org/apache/helix/manager/zk/TestZkBaseDataAccessor.java +++ b/helix-core/src/test/java/org/apache/helix/manager/zk/TestZkBaseDataAccessor.java @@ -49,7 +49,6 @@ import org.testng.Assert; import org.testng.annotations.AfterMethod; import org.testng.annotations.Test; -import org.apache.zookeeper.Op; public class TestZkBaseDataAccessor extends ZkUnitTestBase { // serialize/deserialize integer list to byte array diff --git a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java index f24aa4673b..94337e040a 100644 --- a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java +++ b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java @@ -22,14 +22,15 @@ import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Date; -import java.util.HashSet; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.OptionalLong; +import java.util.Queue; import java.util.Set; -import java.util.Stack; import java.util.concurrent.Callable; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArraySet; @@ -1861,8 +1862,8 @@ public void deleteRecursivelyAtomic(List paths) { } /** - * Get the list of operations to delete the given root and all its children. Performs simple BFS to put delete - * operations for leaf nodes first before parent nodes. + * Get the list of operations to delete the given root and all its children. Ops will be ordered so that deletion of + * children will come before parent nodes. * @param root the root node to delete * @return the list of ZK operations to delete the given root and all its children */ @@ -1873,26 +1874,19 @@ private List getOpsForRecursiveDelete(String root) { return ops; } - HashSet visited = new HashSet<>(); - Stack nodes = new Stack<>(); - nodes.push(root); - + Queue nodes = new LinkedList<>(); + nodes.offer(root); while (!nodes.isEmpty()) { - String node = nodes.peek(); - List children = getChildren(node, false); - if (children.isEmpty() || visited.contains(node)) { - nodes.pop(); - ops.add(Op.delete(node, -1)); - } else { - for (String child : children) { - nodes.push(node + "/" + child); - } - } - visited.add(node); + String node = nodes.poll(); + getChildren(node, false).stream().forEach(child -> nodes.offer(node + "/" + child)); + ops.add(Op.delete(node, -1)); } + + Collections.reverse(ops); return ops; } + private void processDataOrChildChange(WatchedEvent event, long notificationTime) { final String path = event.getPath(); final boolean pathExists = event.getType() != EventType.NodeDeleted;