From d1b4cbf070046ace5c047cc46d99c0ae71f749a5 Mon Sep 17 00:00:00 2001 From: randgalt Date: Fri, 28 Aug 2015 13:39:22 -0500 Subject: [PATCH 1/2] Switch to new injectSessionExpiration() --- .../curator/TestSessionFailRetryLoop.java | 9 +- .../recipes/cache/TestTreeCache.java | 1 - .../org/apache/curator/test/KillSession.java | 87 ++++--------------- 3 files changed, 21 insertions(+), 76 deletions(-) diff --git a/curator-client/src/test/java/org/apache/curator/TestSessionFailRetryLoop.java b/curator-client/src/test/java/org/apache/curator/TestSessionFailRetryLoop.java index c17b5bf324..937726e6d3 100644 --- a/curator-client/src/test/java/org/apache/curator/TestSessionFailRetryLoop.java +++ b/curator-client/src/test/java/org/apache/curator/TestSessionFailRetryLoop.java @@ -18,6 +18,7 @@ */ package org.apache.curator; +import org.apache.curator.retry.ExponentialBackoffRetry; import org.apache.curator.test.BaseClassForTests; import org.apache.curator.utils.CloseableUtils; import org.apache.curator.retry.RetryOneTime; @@ -34,7 +35,7 @@ public class TestSessionFailRetryLoop extends BaseClassForTests public void testRetry() throws Exception { Timing timing = new Timing(); - final CuratorZookeeperClient client = new CuratorZookeeperClient(server.getConnectString(), timing.session(), timing.connection(), null, new RetryOneTime(1)); + final CuratorZookeeperClient client = new CuratorZookeeperClient(server.getConnectString(), timing.session(), timing.connection(), null, new ExponentialBackoffRetry(100, 3)); SessionFailRetryLoop retryLoop = client.newSessionFailRetryLoop(SessionFailRetryLoop.Mode.RETRY); retryLoop.start(); try @@ -103,7 +104,7 @@ public Void call() throws Exception public void testRetryStatic() throws Exception { Timing timing = new Timing(); - final CuratorZookeeperClient client = new CuratorZookeeperClient(server.getConnectString(), timing.session(), timing.connection(), null, new RetryOneTime(1)); + final CuratorZookeeperClient client = new CuratorZookeeperClient(server.getConnectString(), timing.session(), timing.connection(), null, new ExponentialBackoffRetry(100, 3)); SessionFailRetryLoop retryLoop = client.newSessionFailRetryLoop(SessionFailRetryLoop.Mode.RETRY); retryLoop.start(); try @@ -175,7 +176,7 @@ public Void call() throws Exception public void testBasic() throws Exception { Timing timing = new Timing(); - final CuratorZookeeperClient client = new CuratorZookeeperClient(server.getConnectString(), timing.session(), timing.connection(), null, new RetryOneTime(1)); + final CuratorZookeeperClient client = new CuratorZookeeperClient(server.getConnectString(), timing.session(), timing.connection(), null, new ExponentialBackoffRetry(100, 3)); SessionFailRetryLoop retryLoop = client.newSessionFailRetryLoop(SessionFailRetryLoop.Mode.FAIL); retryLoop.start(); try @@ -230,7 +231,7 @@ public Void call() throws Exception public void testBasicStatic() throws Exception { Timing timing = new Timing(); - final CuratorZookeeperClient client = new CuratorZookeeperClient(server.getConnectString(), timing.session(), timing.connection(), null, new RetryOneTime(1)); + final CuratorZookeeperClient client = new CuratorZookeeperClient(server.getConnectString(), timing.session(), timing.connection(), null, new ExponentialBackoffRetry(100, 3)); SessionFailRetryLoop retryLoop = client.newSessionFailRetryLoop(SessionFailRetryLoop.Mode.FAIL); retryLoop.start(); try 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 0bccb54edf..151ea7e694 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 @@ -377,7 +377,6 @@ public void testKilledSession() throws Exception assertEvent(TreeCacheEvent.Type.NODE_ADDED, "/test/me"); KillSession.kill(client.getZookeeperClient().getZooKeeper(), server.getConnectString()); - assertEvent(TreeCacheEvent.Type.CONNECTION_SUSPENDED); assertEvent(TreeCacheEvent.Type.CONNECTION_LOST); assertEvent(TreeCacheEvent.Type.CONNECTION_RECONNECTED); assertEvent(TreeCacheEvent.Type.NODE_REMOVED, "/test/me"); diff --git a/curator-test/src/main/java/org/apache/curator/test/KillSession.java b/curator-test/src/main/java/org/apache/curator/test/KillSession.java index 63b7f2aa2f..ce2b7e605f 100644 --- a/curator-test/src/main/java/org/apache/curator/test/KillSession.java +++ b/curator-test/src/main/java/org/apache/curator/test/KillSession.java @@ -18,37 +18,37 @@ */ package org.apache.curator.test; -import org.apache.zookeeper.WatchedEvent; -import org.apache.zookeeper.Watcher; import org.apache.zookeeper.ZooKeeper; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; /** *

- * Utility to simulate a ZK session dying. See: ZooKeeper FAQ + * Utility to simulate a ZK session dying. *

- * - *
- * In the case of testing we want to cause a problem, so to explicitly expire a session an - * application connects to ZooKeeper, saves the session id and password, creates another - * ZooKeeper handle with that id and password, and then closes the new handle. Since both - * handles reference the same session, the close on second handle will invalidate the session - * causing a SESSION_EXPIRED on the first handle. - *
*/ public class KillSession { + /** + * Kill the given ZK session + * + * @param client the client to kill + * @since 3.0.0 + */ + public static void kill(ZooKeeper client) + { + client.getTestable().injectSessionExpiration(); + } + /** * Kill the given ZK session * * @param client the client to kill * @param connectString server connection string * @throws Exception errors + * @deprecated use {@link #kill(ZooKeeper)} instead */ public static void kill(ZooKeeper client, String connectString) throws Exception { - kill(client, connectString, new Timing().forWaiting().milliseconds()); + kill(client); } /** @@ -58,65 +58,10 @@ public static void kill(ZooKeeper client, String connectString) throws Excep * @param connectString server connection string * @param maxMs max time ms to wait for kill * @throws Exception errors + * @deprecated use {@link #kill(ZooKeeper)} instead */ public static void kill(ZooKeeper client, String connectString, int maxMs) throws Exception { - long startTicks = System.currentTimeMillis(); - - final CountDownLatch sessionLostLatch = new CountDownLatch(1); - Watcher sessionLostWatch = new Watcher() - { - @Override - public void process(WatchedEvent event) - { - sessionLostLatch.countDown(); - } - }; - client.exists("/___CURATOR_KILL_SESSION___" + System.nanoTime(), sessionLostWatch); - - final CountDownLatch connectionLatch = new CountDownLatch(1); - Watcher connectionWatcher = new Watcher() - { - @Override - public void process(WatchedEvent event) - { - if ( event.getState() == Event.KeeperState.SyncConnected ) - { - connectionLatch.countDown(); - } - } - }; - ZooKeeper zk = new ZooKeeper(connectString, maxMs, connectionWatcher, client.getSessionId(), client.getSessionPasswd()); - try - { - if ( !connectionLatch.await(maxMs, TimeUnit.MILLISECONDS) ) - { - throw new Exception("KillSession could not establish duplicate session"); - } - try - { - zk.close(); - } - finally - { - zk = null; - } - - while ( client.getState().isConnected() && !sessionLostLatch.await(100, TimeUnit.MILLISECONDS) ) - { - long elapsed = System.currentTimeMillis() - startTicks; - if ( elapsed > maxMs ) - { - throw new Exception("KillSession timed out waiting for session to expire"); - } - } - } - finally - { - if ( zk != null ) - { - zk.close(); - } - } + kill(client); } } From 145da217ff35df0178823da784f4dd1618851c5e Mon Sep 17 00:00:00 2001 From: randgalt Date: Fri, 28 Aug 2015 13:39:54 -0500 Subject: [PATCH 2/2] New injectSessionExpiration() operates much faster than previously. It exposes an assumption in the tests. Added a debug hook to work around --- .../recipes/nodes/PersistentEphemeralNode.java | 16 ++++++++++++++++ .../nodes/TestPersistentEphemeralNode.java | 17 +++++++++-------- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentEphemeralNode.java b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentEphemeralNode.java index 0b482ef772..e889bd8588 100644 --- a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentEphemeralNode.java +++ b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentEphemeralNode.java @@ -19,6 +19,7 @@ package org.apache.curator.framework.recipes.nodes; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import org.apache.curator.framework.CuratorFramework; @@ -111,11 +112,26 @@ public void stateChanged(CuratorFramework client, ConnectionState newState) { if ( newState == ConnectionState.RECONNECTED ) { + if ( debugReconnectLatch != null ) + { + try + { + debugReconnectLatch.await(); + } + catch ( InterruptedException e ) + { + Thread.currentThread().interrupt(); + e.printStackTrace(); + } + } createNode(); } } }; + @VisibleForTesting + volatile CountDownLatch debugReconnectLatch = null; + private enum State { LATENT, diff --git a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentEphemeralNode.java b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentEphemeralNode.java index c81cc65133..3a0d5643bd 100644 --- a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentEphemeralNode.java +++ b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentEphemeralNode.java @@ -297,6 +297,7 @@ public void testDeletesNodeWhenSessionDisconnects() throws Exception CuratorFramework observer = newCurator(); PersistentEphemeralNode node = new PersistentEphemeralNode(curator, PersistentEphemeralNode.Mode.EPHEMERAL, PATH, new byte[0]); + node.debugReconnectLatch = new CountDownLatch(1); node.start(); try { @@ -307,10 +308,11 @@ public void testDeletesNodeWhenSessionDisconnects() throws Exception Trigger deletedTrigger = Trigger.deleted(); observer.checkExists().usingWatcher(deletedTrigger).forPath(node.getActualPath()); - killSession(curator); + KillSession.kill(curator.getZookeeperClient().getZooKeeper()); // Make sure the node got deleted assertTrue(deletedTrigger.firedWithin(timing.forWaiting().seconds(), TimeUnit.SECONDS)); + node.debugReconnectLatch.countDown(); } finally { @@ -325,6 +327,7 @@ public void testRecreatesNodeWhenSessionReconnects() throws Exception CuratorFramework observer = newCurator(); PersistentEphemeralNode node = new PersistentEphemeralNode(curator, PersistentEphemeralNode.Mode.EPHEMERAL, PATH, new byte[0]); + node.debugReconnectLatch = new CountDownLatch(1); node.start(); try { @@ -334,10 +337,11 @@ public void testRecreatesNodeWhenSessionReconnects() throws Exception Trigger deletedTrigger = Trigger.deleted(); observer.checkExists().usingWatcher(deletedTrigger).forPath(node.getActualPath()); - killSession(curator); + KillSession.kill(curator.getZookeeperClient().getZooKeeper()); // Make sure the node got deleted... assertTrue(deletedTrigger.firedWithin(timing.forWaiting().seconds(), TimeUnit.SECONDS)); + node.debugReconnectLatch.countDown(); // Check for it to be recreated... Trigger createdTrigger = Trigger.created(); @@ -367,14 +371,16 @@ public void testRecreatesNodeWhenSessionReconnectsMultipleTimes() throws Excepti // We should be able to disconnect multiple times and each time the node should be recreated. for ( int i = 0; i < 5; i++ ) { + node.debugReconnectLatch = new CountDownLatch(1); Trigger deletionTrigger = Trigger.deleted(); observer.checkExists().usingWatcher(deletionTrigger).forPath(path); // Kill the session, thus cleaning up the node... - killSession(curator); + KillSession.kill(curator.getZookeeperClient().getZooKeeper()); // Make sure the node ended up getting deleted... assertTrue(deletionTrigger.firedWithin(timing.forWaiting().seconds(), TimeUnit.SECONDS)); + node.debugReconnectLatch.countDown(); // Now put a watch in the background looking to see if it gets created... Trigger creationTrigger = Trigger.created(); @@ -634,11 +640,6 @@ private CuratorFramework newCurator() throws IOException return client; } - public void killSession(CuratorFramework curator) throws Exception - { - KillSession.kill(curator.getZookeeperClient().getZooKeeper(), curator.getZookeeperClient().getCurrentConnectionString()); - } - private static final class Trigger implements Watcher { private final Event.EventType type;