From 7a64aa2eaa6c4a85a9ad7b95aa83cc1bbde43b8b Mon Sep 17 00:00:00 2001 From: BuildTools <42jpattiz@gmail.com> Date: Fri, 13 Jul 2018 23:45:51 -0700 Subject: [PATCH] Added a test for double barrier timeout behavior. Also added a fix that would remove our node immediately before returning false if we are timing out --- .../barriers/DistributedDoubleBarrier.java | 4 ++++ .../barriers/TestDistributedDoubleBarrier.java | 17 +++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/barriers/DistributedDoubleBarrier.java b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/barriers/DistributedDoubleBarrier.java index b3bdf2cc9c..a4df0d5251 100644 --- a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/barriers/DistributedDoubleBarrier.java +++ b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/barriers/DistributedDoubleBarrier.java @@ -121,6 +121,10 @@ public boolean enter(long maxWait, TimeUnit unit) throws Exception client.create().creatingParentContainersIfNeeded().withMode(CreateMode.EPHEMERAL).forPath(ourPath); boolean result = (readyPathExists || internalEnter(startMs, hasMaxWait, maxWaitMs)); + if ( !result ) + { + client.delete().deletingChildrenIfNeeded().forPath(ourPath); + } if ( connectionLost.get() ) { throw new KeeperException.ConnectionLossException(); diff --git a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/barriers/TestDistributedDoubleBarrier.java b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/barriers/TestDistributedDoubleBarrier.java index 24fd3cb1f2..260e0d0826 100644 --- a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/barriers/TestDistributedDoubleBarrier.java +++ b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/barriers/TestDistributedDoubleBarrier.java @@ -38,11 +38,28 @@ import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import static org.testng.Assert.assertFalse; public class TestDistributedDoubleBarrier extends BaseClassForTests { private static final int QTY = 5; + @Test + public void testMultipleBarrierConnectWithTimeout() throws Exception{ + // the issue is that after the first clients attempt to enter the barrier returns false, + // that client has been told that it is not in the barrier. But when the second call is made (by client 2), + // client 2 perceives client 1 as being in the barrier, so it returns true despite there really only being + // one client at the barrier + CuratorFramework client1 = CuratorFrameworkFactory.newClient(server.getConnectString(), new RetryOneTime(1)); + CuratorFramework client2 = CuratorFrameworkFactory.newClient(server.getConnectString(), new RetryOneTime(1)); + client1.start(); + client2.start(); + DistributedDoubleBarrier barrierForClient1 = new DistributedDoubleBarrier(client1,"/barrier",2); + DistributedDoubleBarrier barrierForClient2 = new DistributedDoubleBarrier(client2,"/barrier",2); + assertFalse(barrierForClient1.enter(2, TimeUnit.SECONDS)); + assertFalse(barrierForClient2.enter(2,TimeUnit.SECONDS)); + } + @Test public void testMultiClient() throws Exception {