IGNITE-8189 Improved ZkDistributedCollectDataFuture#deleteFutureData implementation - Fixes #4537.
Signed-off-by: Alexey Goncharuk <alexey.goncha...@gmail.com> Project: http://git-wip-us.apache.org/repos/asf/ignite/repo Commit: http://git-wip-us.apache.org/repos/asf/ignite/commit/2cccfae4 Tree: http://git-wip-us.apache.org/repos/asf/ignite/tree/2cccfae4 Diff: http://git-wip-us.apache.org/repos/asf/ignite/diff/2cccfae4 Branch: refs/heads/ignite-5960 Commit: 2cccfae49106f36a4f0b2b52a81889a295f52677 Parents: d1b3882 Author: NSAmelchev <nsamelc...@gmail.com> Authored: Fri Aug 31 15:47:16 2018 +0300 Committer: Alexey Goncharuk <alexey.goncha...@gmail.com> Committed: Fri Aug 31 15:47:16 2018 +0300 ---------------------------------------------------------------------- .../ZkDistributedCollectDataFuture.java | 19 ++++------ .../discovery/zk/internal/ZookeeperClient.java | 40 ++++++++------------ .../zk/internal/ZookeeperDiscoveryImpl.java | 39 +++++++------------ .../zk/internal/ZookeeperClientTest.java | 13 ++----- 4 files changed, 40 insertions(+), 71 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ignite/blob/2cccfae4/modules/zookeeper/src/main/java/org/apache/ignite/spi/discovery/zk/internal/ZkDistributedCollectDataFuture.java ---------------------------------------------------------------------- diff --git a/modules/zookeeper/src/main/java/org/apache/ignite/spi/discovery/zk/internal/ZkDistributedCollectDataFuture.java b/modules/zookeeper/src/main/java/org/apache/ignite/spi/discovery/zk/internal/ZkDistributedCollectDataFuture.java index e9b28e1..e710055 100644 --- a/modules/zookeeper/src/main/java/org/apache/ignite/spi/discovery/zk/internal/ZkDistributedCollectDataFuture.java +++ b/modules/zookeeper/src/main/java/org/apache/ignite/spi/discovery/zk/internal/ZkDistributedCollectDataFuture.java @@ -18,6 +18,7 @@ package org.apache.ignite.spi.discovery.zk.internal; import java.util.Iterator; +import java.util.LinkedList; import java.util.List; import java.util.Set; import java.util.UUID; @@ -145,23 +146,19 @@ class ZkDistributedCollectDataFuture extends GridFutureAdapter<Void> { UUID futId, IgniteLogger log ) throws Exception { - // TODO ZK: https://issues.apache.org/jira/browse/IGNITE-8189 + List<String> batch = new LinkedList<>(); + String evtDir = paths.distributedFutureBasePath(futId); - try { - client.deleteAll(evtDir, - client.getChildrenIfPathExists(evtDir), - -1); - } - catch (KeeperException.NoNodeException e) { - U.log(log, "Node for deletion was not found: " + e.getPath()); + if (client.exists(evtDir)) { + batch.addAll(client.getChildrenPaths(evtDir)); - // TODO ZK: https://issues.apache.org/jira/browse/IGNITE-8189 + batch.add(evtDir); } - client.deleteIfExists(evtDir, -1); + batch.add(paths.distributedFutureResultPath(futId)); - client.deleteIfExists(paths.distributedFutureResultPath(futId), -1); + client.deleteAll(batch, -1); } /** http://git-wip-us.apache.org/repos/asf/ignite/blob/2cccfae4/modules/zookeeper/src/main/java/org/apache/ignite/spi/discovery/zk/internal/ZookeeperClient.java ---------------------------------------------------------------------- diff --git a/modules/zookeeper/src/main/java/org/apache/ignite/spi/discovery/zk/internal/ZookeeperClient.java b/modules/zookeeper/src/main/java/org/apache/ignite/spi/discovery/zk/internal/ZookeeperClient.java index b58f0ce..39417c2 100644 --- a/modules/zookeeper/src/main/java/org/apache/ignite/spi/discovery/zk/internal/ZookeeperClient.java +++ b/modules/zookeeper/src/main/java/org/apache/ignite/spi/discovery/zk/internal/ZookeeperClient.java @@ -504,9 +504,7 @@ public class ZookeeperClient implements Watcher { * @throws ZookeeperClientFailedException If connection to zk was lost. * @throws InterruptedException If interrupted. */ - List<String> getChildren(String path) - throws ZookeeperClientFailedException, InterruptedException - { + List<String> getChildren(String path) throws ZookeeperClientFailedException, InterruptedException { for (;;) { long connStartTime = this.connStartTime; @@ -520,29 +518,23 @@ public class ZookeeperClient implements Watcher { } /** + * Get children paths. + * * @param path Path. - * @return Children nodes. - * @throws KeeperException.NoNodeException If provided path does not exist. + * @return Children paths. * @throws ZookeeperClientFailedException If connection to zk was lost. * @throws InterruptedException If interrupted. */ - List<String> getChildrenIfPathExists(String path) throws - KeeperException.NoNodeException, InterruptedException, ZookeeperClientFailedException { - for (;;) { - long connStartTime = this.connStartTime; + List<String> getChildrenPaths(String path) throws ZookeeperClientFailedException, InterruptedException { + List<String> children = getChildren(path); - try { - return zk.getChildren(path, false); - } - catch (KeeperException.NoNodeException e) { - throw e; - } - catch (Exception e) { - onZookeeperError(connStartTime, e); - } - } - } + ArrayList<String> paths = new ArrayList(children.size()); + for (String child : children) + paths.add(path + "/" + child); + + return paths; + } /** * @param path Path. @@ -593,7 +585,7 @@ public class ZookeeperClient implements Watcher { * @throws ZookeeperClientFailedException If connection to zk was lost. * @throws InterruptedException If interrupted. */ - void deleteAll(@Nullable String parent, List<String> paths, int ver) + void deleteAll(List<String> paths, int ver) throws ZookeeperClientFailedException, InterruptedException { if (paths.isEmpty()) return; @@ -605,10 +597,8 @@ public class ZookeeperClient implements Watcher { List<Op> batch = new LinkedList<>(); for (String path : paths) { - String path0 = parent != null ? parent + "/" + path : path; - //TODO ZK: https://issues.apache.org/jira/browse/IGNITE-8187 - int size = requestOverhead(path0) + 17 /* overhead */; + int size = requestOverhead(path) + 17 /* overhead */; assert size <= MAX_REQ_SIZE; @@ -620,7 +610,7 @@ public class ZookeeperClient implements Watcher { batchSize = 0; } - batch.add(Op.delete(path0, ver)); + batch.add(Op.delete(path, ver)); batchSize += size; } http://git-wip-us.apache.org/repos/asf/ignite/blob/2cccfae4/modules/zookeeper/src/main/java/org/apache/ignite/spi/discovery/zk/internal/ZookeeperDiscoveryImpl.java ---------------------------------------------------------------------- diff --git a/modules/zookeeper/src/main/java/org/apache/ignite/spi/discovery/zk/internal/ZookeeperDiscoveryImpl.java b/modules/zookeeper/src/main/java/org/apache/ignite/spi/discovery/zk/internal/ZookeeperDiscoveryImpl.java index 4579efd..069b3e2 100644 --- a/modules/zookeeper/src/main/java/org/apache/ignite/spi/discovery/zk/internal/ZookeeperDiscoveryImpl.java +++ b/modules/zookeeper/src/main/java/org/apache/ignite/spi/discovery/zk/internal/ZookeeperDiscoveryImpl.java @@ -27,6 +27,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; @@ -2263,33 +2264,27 @@ public class ZookeeperDiscoveryImpl { ZookeeperClient client = rtState.zkClient; - // TODO ZK: use multi, better batching + max-size safe + NoNodeException safe. - List<String> evtChildren = rtState.zkClient.getChildren(zkPaths.evtsPath); + List<String> batch = new LinkedList<>(); - for (String evtPath : evtChildren) { - String evtDir = zkPaths.evtsPath + "/" + evtPath; + List<String> evtChildren = client.getChildrenPaths(zkPaths.evtsPath); - removeChildren(evtDir); - } + for (String evtPath : evtChildren) + batch.addAll(client.getChildrenPaths(evtPath)); + + batch.addAll(evtChildren); - client.deleteAll(zkPaths.evtsPath, evtChildren, -1); + batch.addAll(client.getChildrenPaths(zkPaths.customEvtsDir)); - client.deleteAll(zkPaths.customEvtsDir, - client.getChildren(zkPaths.customEvtsDir), - -1); + batch.addAll(client.getChildrenPaths(zkPaths.customEvtsPartsDir)); - rtState.zkClient.deleteAll(zkPaths.customEvtsPartsDir, - rtState.zkClient.getChildren(zkPaths.customEvtsPartsDir), - -1); + batch.addAll(client.getChildrenPaths(zkPaths.customEvtsAcksDir)); - rtState.zkClient.deleteAll(zkPaths.customEvtsAcksDir, - rtState.zkClient.getChildren(zkPaths.customEvtsAcksDir), - -1); + client.deleteAll(batch, -1); if (startInternalOrder > 0) { - for (String alive : rtState.zkClient.getChildren(zkPaths.aliveNodesDir)) { + for (String alive : client.getChildren(zkPaths.aliveNodesDir)) { if (ZkIgnitePaths.aliveInternalId(alive) < startInternalOrder) - rtState.zkClient.deleteIfExists(zkPaths.aliveNodesDir + "/" + alive, -1); + client.deleteIfExists(zkPaths.aliveNodesDir + "/" + alive, -1); } } @@ -2302,14 +2297,6 @@ public class ZookeeperDiscoveryImpl { } /** - * @param path Path. - * @throws Exception If failed. - */ - private void removeChildren(String path) throws Exception { - rtState.zkClient.deleteAll(path, rtState.zkClient.getChildren(path), -1); - } - - /** * @param zkClient Client. * @param evtPath Event path. * @param sndNodeId Sender node ID. http://git-wip-us.apache.org/repos/asf/ignite/blob/2cccfae4/modules/zookeeper/src/test/java/org/apache/ignite/spi/discovery/zk/internal/ZookeeperClientTest.java ---------------------------------------------------------------------- diff --git a/modules/zookeeper/src/test/java/org/apache/ignite/spi/discovery/zk/internal/ZookeeperClientTest.java b/modules/zookeeper/src/test/java/org/apache/ignite/spi/discovery/zk/internal/ZookeeperClientTest.java index 0d64980..7c9ec51 100644 --- a/modules/zookeeper/src/test/java/org/apache/ignite/spi/discovery/zk/internal/ZookeeperClientTest.java +++ b/modules/zookeeper/src/test/java/org/apache/ignite/spi/discovery/zk/internal/ZookeeperClientTest.java @@ -196,12 +196,12 @@ public class ZookeeperClientTest extends GridCommonAbstractTest { client.createIfNeeded("/apacheIgnite/1", null, CreateMode.PERSISTENT); client.createIfNeeded("/apacheIgnite/2", null, CreateMode.PERSISTENT); - client.deleteAll("/apacheIgnite", Arrays.asList("1", "2"), -1); + client.deleteAll(Arrays.asList("/apacheIgnite/1", "/apacheIgnite/2"), -1); assertTrue(client.getChildren("/apacheIgnite").isEmpty()); client.createIfNeeded("/apacheIgnite/1", null, CreateMode.PERSISTENT); - client.deleteAll("/apacheIgnite", Collections.singletonList("1"), -1); + client.deleteAll(Collections.singletonList("/apacheIgnite/1"), -1); assertTrue(client.getChildren("/apacheIgnite").isEmpty()); } @@ -227,12 +227,7 @@ public class ZookeeperClientTest extends GridCommonAbstractTest { assertEquals(cnt, client.getChildren("/apacheIgnite").size()); - List<String> subPaths = new ArrayList<>(cnt); - - for (int i = 0; i < cnt; i++) - subPaths.add(String.valueOf(i)); - - client.deleteAll("/apacheIgnite", subPaths, -1); + client.deleteAll(paths, -1); assertTrue(client.getChildren("/apacheIgnite").isEmpty()); } @@ -249,7 +244,7 @@ public class ZookeeperClientTest extends GridCommonAbstractTest { client.createIfNeeded("/apacheIgnite/1", null, CreateMode.PERSISTENT); client.createIfNeeded("/apacheIgnite/2", null, CreateMode.PERSISTENT); - client.deleteAll("/apacheIgnite", Arrays.asList("1", "2", "3"), -1); + client.deleteAll(Arrays.asList("/apacheIgnite/1", "/apacheIgnite/2", "/apacheIgnite/3"), -1); assertTrue(client.getChildren("/apacheIgnite").isEmpty()); }
