Repository: curator Updated Branches: refs/heads/CURATOR-498-alt [created] ad6bbee36
CURATOR-498 Alternate fix - simpler and hopefully works. Make note of the ZK session ID at the start of the create operation. When protected mode is engaged, check the session again. If the IDs don't match, delete any found node and return null to cause a new node to get created. Project: http://git-wip-us.apache.org/repos/asf/curator/repo Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/ad6bbee3 Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/ad6bbee3 Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/ad6bbee3 Branch: refs/heads/CURATOR-498-alt Commit: ad6bbee36f06b7d5b0fe661acff141af4b55bc9a Parents: 818e1ed Author: randgalt <[email protected]> Authored: Thu Jan 3 21:07:00 2019 -0500 Committer: randgalt <[email protected]> Committed: Thu Jan 3 21:07:00 2019 -0500 ---------------------------------------------------------------------- .../framework/api/CreateBuilderMain.java | 2 - ...ateProtectACLCreateModePathAndBytesable.java | 2 - .../ProtectACLCreateModePathAndBytesable.java | 2 - .../framework/imps/CreateBuilderImpl.java | 104 +++++++------------ .../framework/imps/TestFrameworkEdges.java | 7 +- .../framework/recipes/leader/LeaderLatch.java | 19 +--- 6 files changed, 41 insertions(+), 95 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/curator/blob/ad6bbee3/curator-framework/src/main/java/org/apache/curator/framework/api/CreateBuilderMain.java ---------------------------------------------------------------------- diff --git a/curator-framework/src/main/java/org/apache/curator/framework/api/CreateBuilderMain.java b/curator-framework/src/main/java/org/apache/curator/framework/api/CreateBuilderMain.java index c6d480a..3d076b2 100644 --- a/curator-framework/src/main/java/org/apache/curator/framework/api/CreateBuilderMain.java +++ b/curator-framework/src/main/java/org/apache/curator/framework/api/CreateBuilderMain.java @@ -83,6 +83,4 @@ public interface CreateBuilderMain extends * @return this */ public ACLCreateModeStatBackgroundPathAndBytesable<String> withProtection(); - - public Watchable<ACLCreateModeStatBackgroundPathAndBytesable<String>> withWatchedProtection(); } http://git-wip-us.apache.org/repos/asf/curator/blob/ad6bbee3/curator-framework/src/main/java/org/apache/curator/framework/api/CreateProtectACLCreateModePathAndBytesable.java ---------------------------------------------------------------------- diff --git a/curator-framework/src/main/java/org/apache/curator/framework/api/CreateProtectACLCreateModePathAndBytesable.java b/curator-framework/src/main/java/org/apache/curator/framework/api/CreateProtectACLCreateModePathAndBytesable.java index 0d6f35b..9e0c840 100755 --- a/curator-framework/src/main/java/org/apache/curator/framework/api/CreateProtectACLCreateModePathAndBytesable.java +++ b/curator-framework/src/main/java/org/apache/curator/framework/api/CreateProtectACLCreateModePathAndBytesable.java @@ -69,6 +69,4 @@ public interface CreateProtectACLCreateModePathAndBytesable<T> extends * @return this */ public ACLCreateModeBackgroundPathAndBytesable<String> withProtection(); - - public Watchable<ACLCreateModeStatBackgroundPathAndBytesable<String>> withWatchedProtection(); } http://git-wip-us.apache.org/repos/asf/curator/blob/ad6bbee3/curator-framework/src/main/java/org/apache/curator/framework/api/ProtectACLCreateModePathAndBytesable.java ---------------------------------------------------------------------- diff --git a/curator-framework/src/main/java/org/apache/curator/framework/api/ProtectACLCreateModePathAndBytesable.java b/curator-framework/src/main/java/org/apache/curator/framework/api/ProtectACLCreateModePathAndBytesable.java index 57f4beb..1d1df10 100644 --- a/curator-framework/src/main/java/org/apache/curator/framework/api/ProtectACLCreateModePathAndBytesable.java +++ b/curator-framework/src/main/java/org/apache/curator/framework/api/ProtectACLCreateModePathAndBytesable.java @@ -51,6 +51,4 @@ public interface ProtectACLCreateModePathAndBytesable<T> extends * @return this */ public ACLCreateModeBackgroundPathAndBytesable<String> withProtection(); - - public Watchable<ACLCreateModeStatBackgroundPathAndBytesable<String>> withWatchedProtection(); } http://git-wip-us.apache.org/repos/asf/curator/blob/ad6bbee3/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 271cef1..3237f2e 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 @@ -50,6 +50,7 @@ import java.util.concurrent.atomic.AtomicBoolean; public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, BackgroundOperation<PathAndBytes>, ErrorListenerPathAndBytesable<String> { + private final Logger log = LoggerFactory.getLogger(getClass()); private final CuratorFrameworkImpl client; private CreateMode createMode; private Backgrounding backgrounding; @@ -63,7 +64,7 @@ public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, Backgro private long ttl; private boolean doProtected; private String protectedId; - private Watching protectedWatching; + private long initialSessionId; @VisibleForTesting boolean failNextCreateForTesting = false; @@ -83,10 +84,10 @@ public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, Backgro setDataIfExists = false; storingStat = null; ttl = -1; + initialSessionId = 0; doProtected = false; protectedId = null; - protectedWatching = new Watching(client); } 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) @@ -403,12 +404,6 @@ public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, Backgro } @Override - public Watchable<ACLCreateModeStatBackgroundPathAndBytesable<String>> withWatchedProtection() - { - return CreateBuilderImpl.this.withWatchedProtection(); - } - - @Override public BackgroundPathAndBytesable<String> withACL(List<ACL> aclList) { return withACL(aclList, false); @@ -491,35 +486,6 @@ public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, Backgro } @Override - public Watchable<ACLCreateModeStatBackgroundPathAndBytesable<String>> withWatchedProtection() - { - setProtected(); - return new Watchable<ACLCreateModeStatBackgroundPathAndBytesable<String>>() - { - @Override - public ACLCreateModeStatBackgroundPathAndBytesable<String> watched() - { - protectedWatching = new Watching(client, true); - return asACLCreateModeStatBackgroundPathAndBytesable(); - } - - @Override - public ACLCreateModeStatBackgroundPathAndBytesable<String> usingWatcher(Watcher watcher) - { - protectedWatching = new Watching(client, watcher); - return asACLCreateModeStatBackgroundPathAndBytesable(); - } - - @Override - public ACLCreateModeStatBackgroundPathAndBytesable<String> usingWatcher(CuratorWatcher watcher) - { - protectedWatching = new Watching(client, watcher); - return asACLCreateModeStatBackgroundPathAndBytesable(); - } - }; - } - - @Override public ACLPathAndBytesable<String> withProtectedEphemeralSequential() { setProtected(); @@ -658,9 +624,9 @@ public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, Backgro */ new FindAndDeleteProtectedNodeInBackground(client, ZKPaths.getPathAndNode(adjustedPath).getPath(), 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 - */ + * 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(); } @@ -725,16 +691,16 @@ public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, Backgro else { CreateZK35.create - ( - client.getZooKeeper(), - operationAndData.getData().getPath(), - data, - acling.getAclList(operationAndData.getData().getPath()), - createMode, - mainCallback, - backgrounding.getContext(), - ttl - ); + ( + client.getZooKeeper(), + operationAndData.getData().getPath(), + data, + acling.getAclList(operationAndData.getData().getPath()), + createMode, + mainCallback, + backgrounding.getContext(), + ttl + ); } } catch ( Throwable e ) @@ -788,7 +754,7 @@ public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, Backgro @Override public ErrorListenerPathAndBytesable<String> inBackground(BackgroundCallback callback, Object context, - Executor executor) { + Executor executor) { return CreateBuilderImpl.this.inBackground(callback, context, executor); } @@ -813,12 +779,6 @@ public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, Backgro } @Override - public Watchable<ACLCreateModeStatBackgroundPathAndBytesable<String>> withWatchedProtection() - { - return CreateBuilderImpl.this.withWatchedProtection(); - } - - @Override public ProtectACLCreateModePathAndBytesable<String> creatingParentsIfNeeded() { return CreateBuilderImpl.this.creatingParentsIfNeeded(); } @@ -1150,6 +1110,10 @@ public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, Backgro { boolean callSuper = true; boolean localFirstTime = firstTime.getAndSet(false) && !debugForceFindProtectedNode; + if ( localFirstTime ) + { + initialSessionId = client.getZooKeeper().getSessionId(); + } if ( !localFirstTime && doProtected ) { debugForceFindProtectedNode = false; @@ -1208,6 +1172,10 @@ public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, Backgro public String call() throws Exception { boolean localFirstTime = firstTime.getAndSet(false) && !debugForceFindProtectedNode; + if ( localFirstTime ) + { + initialSessionId = client.getZooKeeper().getSessionId(); + } String createdPath = null; if ( !localFirstTime && doProtected ) @@ -1284,7 +1252,6 @@ public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, Backgro { OperationTrace trace = client.getZookeeperClient().startAdvancedTracer("CreateBuilderImpl-findProtectedNodeInForeground"); - Logger log = LoggerFactory.getLogger(getClass()); String returnPath = RetryLoop.callWithRetry ( client.getZookeeperClient(), @@ -1301,19 +1268,18 @@ public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, Backgro foundNode = findNode(children, pathAndNode.getPath(), protectedId); log.debug("Protected mode findNode result: {}", foundNode); - if ( (foundNode != null) && protectedWatching.hasWatcher() ) + + if ( doProtected && createMode.isEphemeral() ) { - String foundPath = ZKPaths.makePath(pathAndNode.getPath(), foundNode); - Watcher protectedWatcher = protectedWatching.getWatcher(foundPath); - try - { - client.getZooKeeper().getData(foundPath, protectedWatcher, null); - protectedWatching.commitWatcher(KeeperException.Code.OK.intValue(), false); - } - catch ( KeeperException.NoNodeException ignore ) + if ( initialSessionId != client.getZooKeeper().getSessionId() ) { - log.warn("protectedWatching failed with NoNodeException for node: {}", foundNode); - foundNode = null; + 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); + } + return null; } } } http://git-wip-us.apache.org/repos/asf/curator/blob/ad6bbee3/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkEdges.java ---------------------------------------------------------------------- diff --git a/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkEdges.java b/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkEdges.java index 413eaca..224beca 100644 --- a/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkEdges.java +++ b/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkEdges.java @@ -54,6 +54,7 @@ import org.slf4j.LoggerFactory; import org.testng.Assert; import org.testng.annotations.BeforeClass; import org.testng.annotations.Test; +import java.util.Collections; import java.util.List; import java.util.Random; import java.util.concurrent.ArrayBlockingQueue; @@ -131,8 +132,7 @@ public class TestFrameworkEdges extends BaseClassForTests client.start(); client.create().forPath("/test"); - Watcher protectedNodeWatcher = __ -> {}; - ErrorListenerPathAndBytesable<String> builder = client.create().withWatchedProtection().usingWatcher(protectedNodeWatcher).withMode(CreateMode.EPHEMERAL).inBackground(callback); + ErrorListenerPathAndBytesable<String> builder = client.create().withProtection().withMode(CreateMode.EPHEMERAL).inBackground(callback); ((CreateBuilderImpl)builder).failNextCreateForTesting = true; builder.forPath("/test/hey"); @@ -142,7 +142,8 @@ public class TestFrameworkEdges extends BaseClassForTests cluster.restartServer(instanceSpec0); String path = timing.takeFromQueue(createdNode); - Assert.assertNotNull(path); + List<String> children = client.getChildren().forPath("/test"); + Assert.assertEquals(Collections.singletonList(ZKPaths.getNodeFromPath(path)), children); } } } http://git-wip-us.apache.org/repos/asf/curator/blob/ad6bbee3/curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderLatch.java ---------------------------------------------------------------------- diff --git a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderLatch.java b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderLatch.java index c8e8e22..bb8aa73 100644 --- a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderLatch.java +++ b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderLatch.java @@ -33,7 +33,6 @@ import org.apache.curator.framework.recipes.locks.LockInternalsSorter; import org.apache.curator.framework.recipes.locks.StandardLockInternalsDriver; import org.apache.curator.framework.state.ConnectionState; import org.apache.curator.framework.state.ConnectionStateListener; -import org.apache.curator.utils.PathUtils; import org.apache.curator.utils.ThreadUtils; import org.apache.curator.utils.ZKPaths; import org.apache.zookeeper.CreateMode; @@ -53,6 +52,7 @@ import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; +import org.apache.curator.utils.PathUtils; /** * <p> @@ -84,21 +84,6 @@ public class LeaderLatch implements Closeable } }; - private final Watcher protectedModeWatcher = watchedEvent -> { - if ( watchedEvent.getType() == Watcher.Event.EventType.NodeDeleted ) - { - try - { - log.warn("Protected mode node was deleted. Resetting."); - reset(); - } - catch ( Exception e ) - { - log.error("Could not reset from protectedModeWatcher", e); - } - } - }; - private static final String LOCK_NAME = "latch-"; private static final LockInternalsSorter sorter = new LockInternalsSorter() @@ -519,7 +504,7 @@ public class LeaderLatch implements Closeable } } }; - client.create().creatingParentContainersIfNeeded().withWatchedProtection().usingWatcher(protectedModeWatcher).withMode(CreateMode.EPHEMERAL_SEQUENTIAL).inBackground(callback).forPath(ZKPaths.makePath(latchPath, LOCK_NAME), LeaderSelector.getIdBytes(id)); + client.create().creatingParentContainersIfNeeded().withProtection().withMode(CreateMode.EPHEMERAL_SEQUENTIAL).inBackground(callback).forPath(ZKPaths.makePath(latchPath, LOCK_NAME), LeaderSelector.getIdBytes(id)); } private synchronized void internalStart()
