More uses of WatcherRemoveCuratorFramework, more tests, etc.
Project: http://git-wip-us.apache.org/repos/asf/curator/repo Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/2c921d62 Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/2c921d62 Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/2c921d62 Branch: refs/heads/CURATOR-3.0 Commit: 2c921d62fc2894f136d088d93870ac8c9b026dcb Parents: a95d52e Author: randgalt <[email protected]> Authored: Tue May 19 20:33:40 2015 -0700 Committer: randgalt <[email protected]> Committed: Tue May 19 20:33:40 2015 -0700 ---------------------------------------------------------------------- .../org/apache/curator/utils/DebugUtils.java | 1 + .../imps/RemoveWatchesBuilderImpl.java | 16 +++++++++--- .../framework/imps/WatcherRemovalManager.java | 3 +-- .../recipes/nodes/PersistentEphemeralNode.java | 3 ++- .../framework/recipes/shared/SharedValue.java | 2 +- .../curator/framework/imps/TestCleanState.java | 11 +++------ .../recipes/leader/TestLeaderLatch.java | 26 ++++++++++---------- .../nodes/TestPersistentEphemeralNode.java | 17 +++++++------ .../recipes/shared/TestSharedCount.java | 11 +++++---- .../apache/curator/test/BaseClassForTests.java | 14 +++++++++++ 10 files changed, 64 insertions(+), 40 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/curator/blob/2c921d62/curator-client/src/main/java/org/apache/curator/utils/DebugUtils.java ---------------------------------------------------------------------- diff --git a/curator-client/src/main/java/org/apache/curator/utils/DebugUtils.java b/curator-client/src/main/java/org/apache/curator/utils/DebugUtils.java index ce751ec..e84e06b 100644 --- a/curator-client/src/main/java/org/apache/curator/utils/DebugUtils.java +++ b/curator-client/src/main/java/org/apache/curator/utils/DebugUtils.java @@ -23,6 +23,7 @@ public class DebugUtils public static final String PROPERTY_LOG_EVENTS = "curator-log-events"; public static final String PROPERTY_DONT_LOG_CONNECTION_ISSUES = "curator-dont-log-connection-problems"; public static final String PROPERTY_LOG_ONLY_FIRST_CONNECTION_ISSUE_AS_ERROR_LEVEL = "curator-log-only-first-connection-issue-as-error-level"; + public static final String PROPERTY_REMOVE_WATCHERS_IN_FOREGROUND = "curator-remove-watchers-in-foreground"; private DebugUtils() { http://git-wip-us.apache.org/repos/asf/curator/blob/2c921d62/curator-framework/src/main/java/org/apache/curator/framework/imps/RemoveWatchesBuilderImpl.java ---------------------------------------------------------------------- diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/RemoveWatchesBuilderImpl.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/RemoveWatchesBuilderImpl.java index 8f61878..d872ced 100644 --- a/curator-framework/src/main/java/org/apache/curator/framework/imps/RemoveWatchesBuilderImpl.java +++ b/curator-framework/src/main/java/org/apache/curator/framework/imps/RemoveWatchesBuilderImpl.java @@ -33,6 +33,7 @@ import org.apache.curator.framework.api.Pathable; import org.apache.curator.framework.api.RemoveWatchesLocal; import org.apache.curator.framework.api.RemoveWatchesBuilder; import org.apache.curator.framework.api.RemoveWatchesType; +import org.apache.curator.utils.DebugUtils; import org.apache.zookeeper.AsyncCallback; import org.apache.zookeeper.KeeperException; import org.apache.zookeeper.Watcher; @@ -61,13 +62,22 @@ public class RemoveWatchesBuilderImpl implements RemoveWatchesBuilder, RemoveWat this.backgrounding = new Backgrounding(); } - void prepInternalRemoval(Watcher watcher) + void internalRemoval(Watcher watcher, String path) throws Exception { this.watcher = watcher; watcherType = WatcherType.Any; quietly = true; - this.backgrounding = new Backgrounding(true); guaranteed = true; + if ( Boolean.getBoolean(DebugUtils.PROPERTY_REMOVE_WATCHERS_IN_FOREGROUND) ) + { + this.backgrounding = new Backgrounding(); + pathInForeground(path); + } + else + { + this.backgrounding = new Backgrounding(true); + pathInBackground(path); + } } @Override @@ -191,7 +201,7 @@ public class RemoveWatchesBuilderImpl implements RemoveWatchesBuilder, RemoveWat return null; } - void pathInBackground(final String path) + private void pathInBackground(final String path) { OperationAndData.ErrorCallback<String> errorCallback = null; http://git-wip-us.apache.org/repos/asf/curator/blob/2c921d62/curator-framework/src/main/java/org/apache/curator/framework/imps/WatcherRemovalManager.java ---------------------------------------------------------------------- diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/WatcherRemovalManager.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/WatcherRemovalManager.java index 72430ee..a691a94 100644 --- a/curator-framework/src/main/java/org/apache/curator/framework/imps/WatcherRemovalManager.java +++ b/curator-framework/src/main/java/org/apache/curator/framework/imps/WatcherRemovalManager.java @@ -68,8 +68,7 @@ public class WatcherRemovalManager { log.debug("Removing watcher for path: " + entry.path); RemoveWatchesBuilderImpl builder = new RemoveWatchesBuilderImpl(client); - builder.prepInternalRemoval(entry); - builder.pathInBackground(entry.path); + builder.internalRemoval(entry, entry.path); } catch ( Exception e ) { http://git-wip-us.apache.org/repos/asf/curator/blob/2c921d62/curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentEphemeralNode.java ---------------------------------------------------------------------- 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 3bad8e3..98b09c9 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 @@ -269,7 +269,6 @@ public class PersistentEphemeralNode implements Closeable return; } - client.removeWatchers(); client.getConnectionStateListenable().removeListener(connectionStateListener); try @@ -280,6 +279,8 @@ public class PersistentEphemeralNode implements Closeable { throw new IOException(e); } + + client.removeWatchers(); } /** http://git-wip-us.apache.org/repos/asf/curator/blob/2c921d62/curator-recipes/src/main/java/org/apache/curator/framework/recipes/shared/SharedValue.java ---------------------------------------------------------------------- diff --git a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/shared/SharedValue.java b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/shared/SharedValue.java index 7e7ad56..e6d8157 100644 --- a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/shared/SharedValue.java +++ b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/shared/SharedValue.java @@ -234,9 +234,9 @@ public class SharedValue implements Closeable, SharedValueReader @Override public void close() throws IOException { + state.set(State.CLOSED); client.removeWatchers(); client.getConnectionStateListenable().removeListener(connectionStateListener); - state.set(State.CLOSED); listeners.clear(); } http://git-wip-us.apache.org/repos/asf/curator/blob/2c921d62/curator-recipes/src/test/java/org/apache/curator/framework/imps/TestCleanState.java ---------------------------------------------------------------------- diff --git a/curator-recipes/src/test/java/org/apache/curator/framework/imps/TestCleanState.java b/curator-recipes/src/test/java/org/apache/curator/framework/imps/TestCleanState.java index 8ca8409..82de1fc 100644 --- a/curator-recipes/src/test/java/org/apache/curator/framework/imps/TestCleanState.java +++ b/curator-recipes/src/test/java/org/apache/curator/framework/imps/TestCleanState.java @@ -35,25 +35,20 @@ public class TestCleanState try { CuratorFrameworkImpl internalClient = (CuratorFrameworkImpl)client; - if ( !internalClient.getNamespaceWatcherMap().isEmpty() ) - { - throw new AssertionError("NamespaceWatcherMap is not empty"); - } - ZooKeeper zooKeeper = internalClient.getZooKeeper(); if ( zooKeeper != null ) { if ( WatchersDebug.getChildWatches(zooKeeper).size() != 0 ) { - throw new AssertionError("One or more child watchers are still registered"); + throw new AssertionError("One or more child watchers are still registered: " + WatchersDebug.getChildWatches(zooKeeper)); } if ( WatchersDebug.getExistWatches(zooKeeper).size() != 0 ) { - throw new AssertionError("One or more exists watchers are still registered"); + throw new AssertionError("One or more exists watchers are still registered: " + WatchersDebug.getExistWatches(zooKeeper)); } if ( WatchersDebug.getDataWatches(zooKeeper).size() != 0 ) { - throw new AssertionError("One or more data watchers are still registered"); + throw new AssertionError("One or more data watchers are still registered: " + WatchersDebug.getDataWatches(zooKeeper)); } } } http://git-wip-us.apache.org/repos/asf/curator/blob/2c921d62/curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/TestLeaderLatch.java ---------------------------------------------------------------------- diff --git a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/TestLeaderLatch.java b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/TestLeaderLatch.java index 96e6d45..3742fb7 100644 --- a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/TestLeaderLatch.java +++ b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/TestLeaderLatch.java @@ -24,6 +24,7 @@ import com.google.common.collect.Lists; import com.google.common.util.concurrent.ThreadFactoryBuilder; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.CuratorFrameworkFactory; +import org.apache.curator.framework.imps.TestCleanState; import org.apache.curator.framework.state.ConnectionState; import org.apache.curator.framework.state.ConnectionStateListener; import org.apache.curator.retry.RetryNTimes; @@ -96,7 +97,7 @@ public class TestLeaderLatch extends BaseClassForTests finally { CloseableUtils.closeQuietly(latch); - CloseableUtils.closeQuietly(client); + TestCleanState.closeAndTestClean(client); } } @@ -126,7 +127,7 @@ public class TestLeaderLatch extends BaseClassForTests finally { CloseableUtils.closeQuietly(latch); - CloseableUtils.closeQuietly(client); + TestCleanState.closeAndTestClean(client); } } @@ -158,7 +159,7 @@ public class TestLeaderLatch extends BaseClassForTests } finally { - CloseableUtils.closeQuietly(client); + TestCleanState.closeAndTestClean(client); } } @@ -213,7 +214,7 @@ public class TestLeaderLatch extends BaseClassForTests { CloseableUtils.closeQuietly(latch); } - CloseableUtils.closeQuietly(client); + TestCleanState.closeAndTestClean(client); } } @@ -256,9 +257,8 @@ public class TestLeaderLatch extends BaseClassForTests { CloseableUtils.closeQuietly(latch); } - CloseableUtils.closeQuietly(client); + TestCleanState.closeAndTestClean(client); } - } @Test @@ -320,7 +320,7 @@ public class TestLeaderLatch extends BaseClassForTests finally { executorService.shutdownNow(); - CloseableUtils.closeQuietly(client); + TestCleanState.closeAndTestClean(client); } } @@ -416,7 +416,7 @@ public class TestLeaderLatch extends BaseClassForTests CloseableUtils.closeQuietly(latch); } } - CloseableUtils.closeQuietly(client); + TestCleanState.closeAndTestClean(client); } } @@ -504,7 +504,7 @@ public class TestLeaderLatch extends BaseClassForTests CloseableUtils.closeQuietly(latch); } } - CloseableUtils.closeQuietly(client); + TestCleanState.closeAndTestClean(client); } } @@ -583,7 +583,7 @@ public class TestLeaderLatch extends BaseClassForTests { CloseableUtils.closeQuietly(notifiedLeader); } - CloseableUtils.closeQuietly(client); + TestCleanState.closeAndTestClean(client); } } @@ -639,7 +639,7 @@ public class TestLeaderLatch extends BaseClassForTests finally { CloseableUtils.closeQuietly(leader); - CloseableUtils.closeQuietly(client); + TestCleanState.closeAndTestClean(client); CloseableUtils.closeQuietly(server); } } @@ -709,7 +709,7 @@ public class TestLeaderLatch extends BaseClassForTests { CloseableUtils.closeQuietly(latch); } - CloseableUtils.closeQuietly(client); + TestCleanState.closeAndTestClean(client); } } @@ -745,6 +745,6 @@ public class TestLeaderLatch extends BaseClassForTests { Timing timing = new Timing(); CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), timing.session(), timing.connection(), new RetryOneTime(1)); - LeaderLatch latch = new LeaderLatch(client, "parent"); + new LeaderLatch(client, "parent"); } } http://git-wip-us.apache.org/repos/asf/curator/blob/2c921d62/curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentEphemeralNode.java ---------------------------------------------------------------------- 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 47ae757..5a58b2a 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 @@ -22,6 +22,7 @@ import com.google.common.base.Throwables; import com.google.common.collect.Lists; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.CuratorFrameworkFactory; +import org.apache.curator.framework.imps.TestCleanState; import org.apache.curator.framework.state.ConnectionState; import org.apache.curator.framework.state.ConnectionStateListener; import org.apache.curator.retry.RetryOneTime; @@ -36,6 +37,7 @@ import org.apache.zookeeper.Watcher; import org.apache.zookeeper.data.Stat; import org.testng.Assert; import org.testng.annotations.AfterMethod; +import org.testng.annotations.AfterTest; import org.testng.annotations.Test; import java.io.IOException; import java.util.Arrays; @@ -57,6 +59,7 @@ public class TestPersistentEphemeralNode extends BaseClassForTests private final Timing timing = new Timing(); @AfterMethod + @Override public void teardown() throws Exception { for ( PersistentEphemeralNode node : createdNodes ) @@ -66,10 +69,8 @@ public class TestPersistentEphemeralNode extends BaseClassForTests for ( CuratorFramework curator : curatorInstances ) { - curator.close(); + TestCleanState.closeAndTestClean(curator); } - - super.teardown(); } @Test @@ -115,7 +116,7 @@ public class TestPersistentEphemeralNode extends BaseClassForTests } finally { - CloseableUtils.closeQuietly(client); + TestCleanState.closeAndTestClean(client); } } @@ -125,10 +126,11 @@ public class TestPersistentEphemeralNode extends BaseClassForTests server.close(); CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), timing.session(), timing.connection(), new RetryOneTime(1)); + PersistentEphemeralNode node = null; try { client.start(); - PersistentEphemeralNode node = new PersistentEphemeralNode(client, PersistentEphemeralNode.Mode.EPHEMERAL, "/abc/node", "hello".getBytes()); + node = new PersistentEphemeralNode(client, PersistentEphemeralNode.Mode.EPHEMERAL, "/abc/node", "hello".getBytes()); node.start(); final CountDownLatch connectedLatch = new CountDownLatch(1); @@ -157,7 +159,8 @@ public class TestPersistentEphemeralNode extends BaseClassForTests } finally { - CloseableUtils.closeQuietly(client); + CloseableUtils.closeQuietly(node); + TestCleanState.closeAndTestClean(client); } } @@ -240,7 +243,7 @@ public class TestPersistentEphemeralNode extends BaseClassForTests { node.close(); } - client.close(); + TestCleanState.closeAndTestClean(client); } } http://git-wip-us.apache.org/repos/asf/curator/blob/2c921d62/curator-recipes/src/test/java/org/apache/curator/framework/recipes/shared/TestSharedCount.java ---------------------------------------------------------------------- diff --git a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/shared/TestSharedCount.java b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/shared/TestSharedCount.java index 659154a..2bdd278 100644 --- a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/shared/TestSharedCount.java +++ b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/shared/TestSharedCount.java @@ -23,6 +23,7 @@ import com.google.common.collect.Lists; import com.google.common.util.concurrent.ThreadFactoryBuilder; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.CuratorFrameworkFactory; +import org.apache.curator.framework.imps.TestCleanState; import org.apache.curator.framework.state.ConnectionState; import org.apache.curator.retry.RetryOneTime; import org.apache.curator.test.BaseClassForTests; @@ -147,7 +148,7 @@ public class TestSharedCount extends BaseClassForTests } for ( CuratorFramework client : clients ) { - CloseableUtils.closeQuietly(client); + TestCleanState.closeAndTestClean(client); } } } @@ -170,7 +171,7 @@ public class TestSharedCount extends BaseClassForTests finally { CloseableUtils.closeQuietly(count); - CloseableUtils.closeQuietly(client); + TestCleanState.closeAndTestClean(client); } } @@ -215,7 +216,7 @@ public class TestSharedCount extends BaseClassForTests finally { CloseableUtils.closeQuietly(count); - CloseableUtils.closeQuietly(client); + TestCleanState.closeAndTestClean(client); } } @@ -252,8 +253,8 @@ public class TestSharedCount extends BaseClassForTests { CloseableUtils.closeQuietly(count2); CloseableUtils.closeQuietly(count1); - CloseableUtils.closeQuietly(client2); - CloseableUtils.closeQuietly(client1); + TestCleanState.closeAndTestClean(client2); + TestCleanState.closeAndTestClean(client1); } } } http://git-wip-us.apache.org/repos/asf/curator/blob/2c921d62/curator-test/src/main/java/org/apache/curator/test/BaseClassForTests.java ---------------------------------------------------------------------- diff --git a/curator-test/src/main/java/org/apache/curator/test/BaseClassForTests.java b/curator-test/src/main/java/org/apache/curator/test/BaseClassForTests.java index d676a9b..d5c434f 100644 --- a/curator-test/src/main/java/org/apache/curator/test/BaseClassForTests.java +++ b/curator-test/src/main/java/org/apache/curator/test/BaseClassForTests.java @@ -34,6 +34,7 @@ public class BaseClassForTests private static final int RETRY_WAIT_MS = 5000; private static final String INTERNAL_PROPERTY_DONT_LOG_CONNECTION_ISSUES; + private static final String INTERNAL_PROPERTY_REMOVE_WATCHERS_IN_FOREGROUND; static { String s = null; @@ -47,6 +48,17 @@ public class BaseClassForTests e.printStackTrace(); } INTERNAL_PROPERTY_DONT_LOG_CONNECTION_ISSUES = s; + + try + { + // use reflection to avoid adding a circular dependency in the pom + s = (String)Class.forName("org.apache.curator.utils.DebugUtils").getField("PROPERTY_REMOVE_WATCHERS_IN_FOREGROUND").get(null); + } + catch ( Exception e ) + { + e.printStackTrace(); + } + INTERNAL_PROPERTY_REMOVE_WATCHERS_IN_FOREGROUND = s; } @BeforeSuite(alwaysRun = true) @@ -65,6 +77,7 @@ public class BaseClassForTests { System.setProperty(INTERNAL_PROPERTY_DONT_LOG_CONNECTION_ISSUES, "true"); } + System.setProperty(INTERNAL_PROPERTY_REMOVE_WATCHERS_IN_FOREGROUND, "true"); while ( server == null ) { @@ -83,6 +96,7 @@ public class BaseClassForTests @AfterMethod public void teardown() throws Exception { + System.clearProperty(INTERNAL_PROPERTY_REMOVE_WATCHERS_IN_FOREGROUND); server.close(); server = null; }
