curator-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From randg...@apache.org
Subject curator git commit: More uses of WatcherRemoveCuratorFramework, more tests, etc.
Date Wed, 20 May 2015 03:33:44 GMT
Repository: curator
Updated Branches:
  refs/heads/CURATOR-217 a95d52e53 -> 2c921d62f


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-217
Commit: 2c921d62fc2894f136d088d93870ac8c9b026dcb
Parents: a95d52e
Author: randgalt <randgalt@apache.org>
Authored: Tue May 19 20:33:40 2015 -0700
Committer: randgalt <randgalt@apache.org>
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;
     }


Mime
View raw message