CURATOR-498
authorrandgalt <randgalt@apache.org>
Tue, 5 Feb 2019 14:06:48 +0000 (09:06 -0500)
committerrandgalt <randgalt@apache.org>
Tue, 5 Feb 2019 14:06:48 +0000 (09:06 -0500)
Tightened LeaderLatch's timed await method so it's more consistent. The two calls to hasLeadership() were affecting the testWatchedNodeDeletedOnReconnect() test. In practice this is likely not an issue, but it's more consistent now. Also, reworked the testWatchedNodeDeletedOnReconnect() a bit.

curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderLatch.java
curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/TestLeaderLatch.java

index 446b7cb..7107efa 100644 (file)
@@ -381,15 +381,29 @@ public class LeaderLatch implements Closeable
 
         synchronized(this)
         {
-            while ( (waitNanos > 0) && (state.get() == State.STARTED) && !hasLeadership.get() )
+            while ( true )
             {
+                if ( state.get() != State.STARTED )
+                {
+                    return false;
+                }
+
+                if ( hasLeadership() )
+                {
+                    return true;
+                }
+
+                if ( waitNanos <= 0 )
+                {
+                    return false;
+                }
+
                 long startNanos = System.nanoTime();
                 TimeUnit.NANOSECONDS.timedWait(this, waitNanos);
                 long elapsed = System.nanoTime() - startNanos;
                 waitNanos -= elapsed;
             }
         }
-        return hasLeadership();
     }
 
     /**
index e3e0aeb..083bdc5 100644 (file)
@@ -73,32 +73,34 @@ public class TestLeaderLatch extends BaseClassForTests
     public void testWatchedNodeDeletedOnReconnect() throws Exception
     {
         final String latchPath = "/foo/bar";
-        Timing timing = new Timing();
-        try ( CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), new RetryOneTime(1)) )
+        Timing2 timing = new Timing2();
+        try ( CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), timing.session(), timing.connection(), new RetryOneTime(1)) )
         {
             client.start();
-            try (LeaderLatch latch1 = new LeaderLatch(client, latchPath) )
+            LeaderLatch latch1 = new LeaderLatch(client, latchPath, "1");
+            try ( LeaderLatch latch2 = new LeaderLatch(client, latchPath, "2") )
             {
                 latch1.start();
                 latch1.await();
 
-                try ( LeaderLatch latch2 = new LeaderLatch(client, latchPath) )
-                {
-                    latch2.start(); // will get a watcher on latch1's node
-                    timing.sleepABit();
-
-                    latch2.debugCheckLeaderShipLatch = new CountDownLatch(1);
-                    client.delete().forPath(latch1.getOurPath());   // simulate the leader's path getting deleted
-                    timing.sleepABit(); // after this, latch2 should be blocked just before getting the path in checkLeadership()
+                latch2.start(); // will get a watcher on latch1's node
+                timing.sleepABit();
 
-                    latch2.reset(); // force the internal "ourPath" to get reset
-                    latch2.debugCheckLeaderShipLatch.countDown();   // allow checkLeadership() to continue
+                latch2.debugCheckLeaderShipLatch = new CountDownLatch(1);
+                latch1.close();   // simulate the leader's path getting deleted
+                latch1 = null;
+                timing.sleepABit(); // after this, latch2 should be blocked just before getting the path in checkLeadership()
 
-                    Assert.assertTrue(latch2.await(timing.forWaiting().milliseconds(), TimeUnit.MILLISECONDS));
+                latch2.reset(); // force the internal "ourPath" to get reset
+                latch2.debugCheckLeaderShipLatch.countDown();   // allow checkLeadership() to continue
 
-                    Assert.assertEquals(client.getChildren().forPath(latchPath).size(), 1);
-                    Assert.assertEquals(latch1.getLeader(), latch2.getLeader());
-                }
+                Assert.assertTrue(latch2.await(timing.forSessionSleep().forWaiting().milliseconds(), TimeUnit.MILLISECONDS));
+                timing.sleepABit();
+                Assert.assertEquals(client.getChildren().forPath(latchPath).size(), 1);
+            }
+            finally
+            {
+                CloseableUtils.closeQuietly(latch1);
             }
         }
     }