You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@curator.apache.org by ca...@apache.org on 2015/08/25 06:09:07 UTC

[47/50] curator git commit: CURATOR-218 Reorder ConnectionState process event

CURATOR-218 Reorder ConnectionState process event

Address a race condition in ConnectionState.process where it will
trigger watchers first before updating its own state. This can lead to
inconsistencies when blocking until connected.


Project: http://git-wip-us.apache.org/repos/asf/curator/repo
Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/061ed0a6
Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/061ed0a6
Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/061ed0a6

Branch: refs/heads/CURATOR-167
Commit: 061ed0a6d4630fd166df7ba3d16acde3a231c716
Parents: 2266ca1
Author: Mike Drob <md...@apache.org>
Authored: Wed Jul 8 11:04:30 2015 -0500
Committer: Mike Drob <md...@apache.org>
Committed: Mon Aug 24 07:08:16 2015 -0500

----------------------------------------------------------------------
 .../org/apache/curator/ConnectionState.java     | 26 +++++++++-----------
 .../framework/imps/TestBlockUntilConnected.java | 24 ++++++++++++++++++
 2 files changed, 36 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/curator/blob/061ed0a6/curator-client/src/main/java/org/apache/curator/ConnectionState.java
----------------------------------------------------------------------
diff --git a/curator-client/src/main/java/org/apache/curator/ConnectionState.java b/curator-client/src/main/java/org/apache/curator/ConnectionState.java
index d3900a1..46ae9fd 100644
--- a/curator-client/src/main/java/org/apache/curator/ConnectionState.java
+++ b/curator-client/src/main/java/org/apache/curator/ConnectionState.java
@@ -41,7 +41,7 @@ class ConnectionState implements Watcher, Closeable
 {
     private static final int MAX_BACKGROUND_EXCEPTIONS = 10;
     private static final boolean LOG_EVENTS = Boolean.getBoolean(DebugUtils.PROPERTY_LOG_EVENTS);
-    private final Logger log = LoggerFactory.getLogger(getClass());
+    private static final Logger log = LoggerFactory.getLogger(ConnectionState.class);
     private final HandleHolder zooKeeper;
     private final AtomicBoolean isConnected = new AtomicBoolean(false);
     private final EnsembleProvider ensembleProvider;
@@ -145,24 +145,22 @@ class ConnectionState implements Watcher, Closeable
             log.debug("ConnectState watcher: " + event);
         }
 
-        for ( Watcher parentWatcher : parentWatchers )
-        {
-            TimeTrace timeTrace = new TimeTrace("connection-state-parent-process", tracer.get());
-            parentWatcher.process(event);
-            timeTrace.commit();
-        }
-
-        boolean wasConnected = isConnected.get();
-        boolean newIsConnected = wasConnected;
         if ( event.getType() == Watcher.Event.EventType.None )
         {
-            newIsConnected = checkState(event.getState(), wasConnected);
+            boolean wasConnected = isConnected.get();
+            boolean newIsConnected = checkState(event.getState(), wasConnected);
+            if ( newIsConnected != wasConnected )
+            {
+                isConnected.set(newIsConnected);
+                connectionStartMs = System.currentTimeMillis();
+            }
         }
 
-        if ( newIsConnected != wasConnected )
+        for ( Watcher parentWatcher : parentWatchers )
         {
-            isConnected.set(newIsConnected);
-            connectionStartMs = System.currentTimeMillis();
+            TimeTrace timeTrace = new TimeTrace("connection-state-parent-process", tracer.get());
+            parentWatcher.process(event);
+            timeTrace.commit();
         }
     }
 

http://git-wip-us.apache.org/repos/asf/curator/blob/061ed0a6/curator-framework/src/test/java/org/apache/curator/framework/imps/TestBlockUntilConnected.java
----------------------------------------------------------------------
diff --git a/curator-framework/src/test/java/org/apache/curator/framework/imps/TestBlockUntilConnected.java b/curator-framework/src/test/java/org/apache/curator/framework/imps/TestBlockUntilConnected.java
index f649afb..a6dc7ab 100644
--- a/curator-framework/src/test/java/org/apache/curator/framework/imps/TestBlockUntilConnected.java
+++ b/curator-framework/src/test/java/org/apache/curator/framework/imps/TestBlockUntilConnected.java
@@ -232,4 +232,28 @@ public class TestBlockUntilConnected extends BaseClassForTests
             CloseableUtils.closeQuietly(client);
         }
     }
+
+    /**
+     * Test that we are actually connected every time that we block until connection is established in a tight loop.
+     */
+    @Test
+    public void testBlockUntilConnectedTightLoop() throws InterruptedException
+    {
+        CuratorFramework client;
+        for(int i = 0 ; i < 50 ; i++)
+        {
+            client = CuratorFrameworkFactory.newClient(server.getConnectString(), new RetryOneTime(100));
+            try
+            {
+                client.start();
+                client.blockUntilConnected();
+
+                Assert.assertTrue(client.getZookeeperClient().isConnected(), "Not connected after blocking for connection #" + i);
+            }
+            finally
+            {
+                client.close();
+            }
+        }
+    }
 }