Repository: curator Updated Branches: refs/heads/CURATOR-498 ea505f542 -> 5751526d4
CURATOR-498 1. Abstracted protected mode fields into a new class for clarity. 2. CURATOR-498's fix is making TestTreeCache.testKilledSession fail for zk 3.4 emulation mode. It's strange, but simple to work around. Should likely be revisited in the future. Project: http://git-wip-us.apache.org/repos/asf/curator/repo Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/5751526d Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/5751526d Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/5751526d Branch: refs/heads/CURATOR-498 Commit: 5751526d4819d95d721f8ff7d26a29aa781996dc Parents: ea505f5 Author: randgalt <[email protected]> Authored: Tue Jan 29 09:17:35 2019 -0500 Committer: randgalt <[email protected]> Committed: Tue Jan 29 09:17:35 2019 -0500 ---------------------------------------------------------------------- .../framework/imps/CreateBuilderImpl.java | 68 ++++------- .../curator/framework/imps/ProtectedMode.java | 112 +++++++++++++++++++ .../framework/recipes/cache/TestTreeCache.java | 18 ++- 3 files changed, 145 insertions(+), 53 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/curator/blob/5751526d/curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java ---------------------------------------------------------------------- diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java index d1ff2bb..a02a6be 100644 --- a/curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java +++ b/curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java @@ -52,6 +52,7 @@ public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, Backgro { private final Logger log = LoggerFactory.getLogger(getClass()); private final CuratorFrameworkImpl client; + private final ProtectedMode protectedMode = new ProtectedMode(); private CreateMode createMode; private Backgrounding backgrounding; private boolean createParentsIfNeeded; @@ -62,9 +63,6 @@ public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, Backgro private ACLing acling; private Stat storingStat; private long ttl; - private boolean doProtected; - private String protectedId; - private long initialSessionId; @VisibleForTesting boolean failNextCreateForTesting = false; @@ -84,10 +82,6 @@ public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, Backgro setDataIfExists = false; storingStat = null; ttl = -1; - initialSessionId = 0; - - doProtected = false; - protectedId = null; } public CreateBuilderImpl(CuratorFrameworkImpl client, CreateMode createMode, Backgrounding backgrounding, boolean createParentsIfNeeded, boolean createParentsAsContainers, boolean doProtected, boolean compress, boolean setDataIfExists, List<ACL> aclList, Stat storingStat, long ttl) @@ -104,7 +98,7 @@ public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, Backgro this.ttl = ttl; if ( doProtected ) { - setProtected(); + protectedMode.setProtectedMode(); } } @@ -481,14 +475,14 @@ public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, Backgro @Override public ACLCreateModeStatBackgroundPathAndBytesable<String> withProtection() { - setProtected(); + protectedMode.setProtectedMode(); return asACLCreateModeStatBackgroundPathAndBytesable(); } @Override public ACLPathAndBytesable<String> withProtectedEphemeralSequential() { - setProtected(); + protectedMode.setProtectedMode(); createMode = CreateMode.EPHEMERAL_SEQUENTIAL; return new ACLPathAndBytesable<String>() @@ -616,18 +610,18 @@ public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, Backgro { ThreadUtils.checkInterrupted(e); if ( ( e instanceof KeeperException.ConnectionLossException || - !( e instanceof KeeperException )) && protectedId != null ) + !( e instanceof KeeperException )) && protectedMode.doProtected() ) { /* * CURATOR-45 + CURATOR-79: we don't know if the create operation was successful or not, * register the znode to be sure it is deleted later. */ - new FindAndDeleteProtectedNodeInBackground(client, ZKPaths.getPathAndNode(adjustedPath).getPath(), protectedId).execute(); + new FindAndDeleteProtectedNodeInBackground(client, ZKPaths.getPathAndNode(adjustedPath).getPath(), protectedMode.protectedId()).execute(); /* * The current UUID is scheduled to be deleted, it is not safe to use it again. * If this builder is used again later create a new UUID */ - protectedId = UUID.randomUUID().toString(); + protectedMode.resetProtectedId(); } throw e; @@ -865,12 +859,6 @@ public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, Backgro client.processBackgroundOperation(operationAndData, event); } - private void setProtected() - { - doProtected = true; - protectedId = UUID.randomUUID().toString(); - } - private ACLCreateModePathAndBytesable<String> asACLCreateModePathAndBytesable() { return new ACLCreateModePathAndBytesable<String>() @@ -1093,12 +1081,12 @@ public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, Backgro { public void retriesExhausted(OperationAndData<PathAndBytes> operationAndData) { - if ( doProtected ) + if ( protectedMode.doProtected() ) { // all retries have failed, findProtectedNodeInForeground(..) included, schedule a clean up - new FindAndDeleteProtectedNodeInBackground(client, ZKPaths.getPathAndNode(path).getPath(), protectedId).execute(); + new FindAndDeleteProtectedNodeInBackground(client, ZKPaths.getPathAndNode(path).getPath(), protectedMode.protectedId()).execute(); // assign a new id if this builder is used again later - protectedId = UUID.randomUUID().toString(); + protectedMode.resetProtectedId(); } } }, @@ -1110,11 +1098,9 @@ public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, Backgro { boolean callSuper = true; boolean localFirstTime = firstTime.getAndSet(false) && !debugForceFindProtectedNode; - if ( initialSessionId == 0 ) - { - initialSessionId = client.getZooKeeper().getSessionId(); - } - if ( !localFirstTime && doProtected ) + + protectedMode.checkSetSessionId(client, createMode); + if ( !localFirstTime && protectedMode.doProtected() ) { debugForceFindProtectedNode = false; String createdPath = null; @@ -1172,13 +1158,10 @@ public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, Backgro public String call() throws Exception { boolean localFirstTime = firstTime.getAndSet(false) && !debugForceFindProtectedNode; - if ( initialSessionId == 0 ) - { - initialSessionId = client.getZooKeeper().getSessionId(); - } + protectedMode.checkSetSessionId(client, createMode); String createdPath = null; - if ( !localFirstTime && doProtected ) + if ( !localFirstTime && protectedMode.doProtected() ) { debugForceFindProtectedNode = false; createdPath = findProtectedNodeInForeground(path); @@ -1266,23 +1249,10 @@ public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, Backgro final ZKPaths.PathAndNode pathAndNode = ZKPaths.getPathAndNode(path); List<String> children = client.getZooKeeper().getChildren(pathAndNode.getPath(), false); - foundNode = findNode(children, pathAndNode.getPath(), protectedId); + foundNode = findNode(children, pathAndNode.getPath(), protectedMode.protectedId()); log.debug("Protected mode findNode result: {}", foundNode); - if ( doProtected && createMode.isEphemeral() ) - { - if ( initialSessionId != client.getZooKeeper().getSessionId() ) - { - log.info("Session has changed during protected mode with ephemeral. old: {} new: {}", initialSessionId, client.getZooKeeper().getSessionId()); - if ( foundNode != null ) - { - log.info("Deleted old session's found node: {}", foundNode); - client.getFailedDeleteManager().executeGuaranteedOperationInBackground(foundNode); - foundNode = null; - } - initialSessionId = client.getZooKeeper().getSessionId(); - } - } + foundNode = protectedMode.validateFoundNode(client, createMode, foundNode); } catch ( KeeperException.NoNodeException ignore ) { @@ -1300,10 +1270,10 @@ public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, Backgro @VisibleForTesting String adjustPath(String path) throws Exception { - if ( doProtected ) + if ( protectedMode.doProtected() ) { ZKPaths.PathAndNode pathAndNode = ZKPaths.getPathAndNode(path); - String name = getProtectedPrefix(protectedId) + pathAndNode.getNode(); + String name = getProtectedPrefix(protectedMode.protectedId()) + pathAndNode.getNode(); path = ZKPaths.makePath(pathAndNode.getPath(), name); } return path; http://git-wip-us.apache.org/repos/asf/curator/blob/5751526d/curator-framework/src/main/java/org/apache/curator/framework/imps/ProtectedMode.java ---------------------------------------------------------------------- diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/ProtectedMode.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/ProtectedMode.java new file mode 100644 index 0000000..2a4500e --- /dev/null +++ b/curator-framework/src/main/java/org/apache/curator/framework/imps/ProtectedMode.java @@ -0,0 +1,112 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.curator.framework.imps; + +import org.apache.zookeeper.CreateMode; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import java.util.UUID; + +/** + * manage the protected mode state for {@link org.apache.curator.framework.imps.CreateBuilderImpl} + */ +class ProtectedMode +{ + private final Logger log = LoggerFactory.getLogger(getClass()); + private boolean doProtected = false; + private String protectedId = null; + private long sessionId = 0; + + /** + * Enable protected mode + */ + void setProtectedMode() + { + doProtected = true; + resetProtectedId(); + } + + /** + * Update the protected mode ID + */ + void resetProtectedId() + { + protectedId = UUID.randomUUID().toString(); + } + + /** + * @return true if protected mode has been enabled + */ + boolean doProtected() + { + return doProtected; + } + + /** + * @return the protected mode ID if protected mode is enabled + */ + String protectedId() + { + return protectedId; + } + + /** + * Record the current session ID if needed + * + * @param client current client + * @param createMode create mode in use + * @throws Exception errors + */ + void checkSetSessionId(CuratorFrameworkImpl client, CreateMode createMode) throws Exception + { + if ( (sessionId == 0) && createMode.isEphemeral() ) + { + sessionId = client.getZooKeeper().getSessionId(); + } + } + + /** + * Validate the found protected-mode node based on the set session ID, etc. + * + * @param client current client + * @param createMode create mode in use + * @param foundNode the found node + * @return either the found node or null - client should always use the returned value + * @throws Exception errors + */ + String validateFoundNode(CuratorFrameworkImpl client, CreateMode createMode, String foundNode) throws Exception + { + if ( doProtected && createMode.isEphemeral() ) + { + if ( sessionId != client.getZooKeeper().getSessionId() ) + { + log.info("Session has changed during protected mode with ephemeral. old: {} new: {}", sessionId, client.getZooKeeper().getSessionId()); + if ( foundNode != null ) + { + log.info("Deleted old session's found node: {}", foundNode); + client.getFailedDeleteManager().executeGuaranteedOperationInBackground(foundNode); + foundNode = null; + } + sessionId = client.getZooKeeper().getSessionId(); + } + } + return foundNode; + } +} http://git-wip-us.apache.org/repos/asf/curator/blob/5751526d/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestTreeCache.java ---------------------------------------------------------------------- diff --git a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestTreeCache.java b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestTreeCache.java index 762bdd8..9631d12 100644 --- a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestTreeCache.java +++ b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/cache/TestTreeCache.java @@ -426,10 +426,20 @@ public class TestTreeCache extends BaseTestTreeCache assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test/me"); KillSession2.kill(client.getZookeeperClient().getZooKeeper()); - assertEvent(TreeCacheEvent.Type.CONNECTION_LOST); - assertEvent(TreeCacheEvent.Type.CONNECTION_RECONNECTED); - assertEvent(TreeCacheEvent.Type.INITIALIZED); - assertEvent(TreeCacheEvent.Type.NODE_REMOVED, "/test/me", "data".getBytes()); + if ( client.isZk34CompatibilityMode() ) + { + assertEvent(TreeCacheEvent.Type.CONNECTION_LOST); + assertEvent(TreeCacheEvent.Type.NODE_REMOVED, "/test/me", "data".getBytes()); + assertEvent(TreeCacheEvent.Type.CONNECTION_RECONNECTED); + assertEvent(TreeCacheEvent.Type.INITIALIZED); + } + else + { + assertEvent(TreeCacheEvent.Type.CONNECTION_LOST); + assertEvent(TreeCacheEvent.Type.CONNECTION_RECONNECTED); + assertEvent(TreeCacheEvent.Type.INITIALIZED); + assertEvent(TreeCacheEvent.Type.NODE_REMOVED, "/test/me", "data".getBytes()); + } assertNoMoreEvents(); }
