You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@curator.apache.org by ra...@apache.org on 2013/11/10 03:48:09 UTC
[1/7] git commit: Use a more reasonable retry for better test
reliability
Updated Branches:
refs/heads/CURATOR-72 [created] d8df3b13d
Use a more reasonable retry for better test reliability
Project: http://git-wip-us.apache.org/repos/asf/curator/repo
Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/aea4dd88
Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/aea4dd88
Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/aea4dd88
Branch: refs/heads/CURATOR-72
Commit: aea4dd88236c44c84adc24457e84cd3eb474a0db
Parents: 987430a
Author: randgalt <ra...@apache.org>
Authored: Sat Nov 9 18:43:26 2013 -0800
Committer: randgalt <ra...@apache.org>
Committed: Sat Nov 9 18:43:26 2013 -0800
----------------------------------------------------------------------
.../recipes/locks/TestInterProcessMutexBase.java | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/curator/blob/aea4dd88/curator-recipes/src/test/java/org/apache/curator/framework/recipes/locks/TestInterProcessMutexBase.java
----------------------------------------------------------------------
diff --git a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/locks/TestInterProcessMutexBase.java b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/locks/TestInterProcessMutexBase.java
index 73b530c..249f36c 100644
--- a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/locks/TestInterProcessMutexBase.java
+++ b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/locks/TestInterProcessMutexBase.java
@@ -132,7 +132,7 @@ public abstract class TestInterProcessMutexBase extends BaseClassForTests
{
final Timing timing = new Timing();
- CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), timing.session(), timing.connection(), new RetryOneTime(1));
+ CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), timing.session(), timing.connection(), new ExponentialBackoffRetry(100, 3));
client.start();
try
{
@@ -186,7 +186,7 @@ public abstract class TestInterProcessMutexBase extends BaseClassForTests
{
CuratorFramework client = CuratorFrameworkFactory.builder().
connectString(server.getConnectString()).
- retryPolicy(new RetryOneTime(1)).
+ retryPolicy(new ExponentialBackoffRetry(100, 3)).
namespace("test").
build();
client.start();
@@ -208,7 +208,7 @@ public abstract class TestInterProcessMutexBase extends BaseClassForTests
{
final int THREAD_QTY = 10;
- CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), new RetryOneTime(1));
+ CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), new ExponentialBackoffRetry(100, 3));
client.start();
try
{
@@ -272,7 +272,7 @@ public abstract class TestInterProcessMutexBase extends BaseClassForTests
@Test
public void testReentrant2Threads() throws Exception
{
- CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), new RetryOneTime(1));
+ CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), new ExponentialBackoffRetry(100, 3));
client.start();
try
{
@@ -318,7 +318,7 @@ public abstract class TestInterProcessMutexBase extends BaseClassForTests
@Test
public void testReentrant() throws Exception
{
- CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), new RetryOneTime(1));
+ CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), new ExponentialBackoffRetry(100, 3));
client.start();
try
{
@@ -370,8 +370,8 @@ public abstract class TestInterProcessMutexBase extends BaseClassForTests
CuratorFramework client2 = null;
try
{
- client1 = CuratorFrameworkFactory.newClient(server.getConnectString(), new RetryOneTime(1));
- client2 = CuratorFrameworkFactory.newClient(server.getConnectString(), new RetryOneTime(1));
+ client1 = CuratorFrameworkFactory.newClient(server.getConnectString(), new ExponentialBackoffRetry(100, 3));
+ client2 = CuratorFrameworkFactory.newClient(server.getConnectString(), new ExponentialBackoffRetry(100, 3));
client1.start();
client2.start();
[6/7] git commit: Background operations were not checking if the
client was connected. Further,
the connection timeout was not being respected as it is in foreground
operations
Posted by ra...@apache.org.
Background operations were not checking if the client was connected. Further, the connection timeout was not being respected as it is in foreground operations
Project: http://git-wip-us.apache.org/repos/asf/curator/repo
Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/9299e9bb
Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/9299e9bb
Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/9299e9bb
Branch: refs/heads/CURATOR-72
Commit: 9299e9bbf889470dae8b7c14956a23f73e8097db
Parents: 28b63b0
Author: randgalt <ra...@apache.org>
Authored: Sat Nov 9 18:46:59 2013 -0800
Committer: randgalt <ra...@apache.org>
Committed: Sat Nov 9 18:46:59 2013 -0800
----------------------------------------------------------------------
.../framework/imps/CuratorFrameworkImpl.java | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/curator/blob/9299e9bb/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
----------------------------------------------------------------------
diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
index 87353bf..d56c9a4 100644
--- a/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
+++ b/curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorFrameworkImpl.java
@@ -50,6 +50,7 @@ import java.util.concurrent.DelayQueue;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
public class CuratorFrameworkImpl implements CuratorFramework
@@ -669,7 +670,20 @@ public class CuratorFrameworkImpl implements CuratorFramework
{
try
{
- operationAndData.callPerformBackgroundOperation();
+ if ( client.isConnected() )
+ {
+ operationAndData.callPerformBackgroundOperation();
+ }
+ else
+ {
+ client.getZooKeeper(); // important - allow connection resets, timeouts, etc. to occur
+ if ( operationAndData.getElapsedTimeMs() >= client.getConnectionTimeoutMs() )
+ {
+ throw new CuratorConnectionLossException();
+ }
+ operationAndData.sleepFor(1, TimeUnit.SECONDS);
+ queueOperation(operationAndData);
+ }
}
catch ( Throwable e )
{
[7/7] git commit: It turns out to be incorrect to not try to recreate
the node when in the suspended state. The side effect of this was that if all
retries failed, etc. the node might never get recreated.
Posted by ra...@apache.org.
It turns out to be incorrect to not try to recreate the node when in the suspended state. The side effect of this was that if all retries failed, etc. the node might never get recreated.
Project: http://git-wip-us.apache.org/repos/asf/curator/repo
Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/d8df3b13
Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/d8df3b13
Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/d8df3b13
Branch: refs/heads/CURATOR-72
Commit: d8df3b13d616d2b53a33ab8f824ec89642f7f0d8
Parents: 9299e9b
Author: randgalt <ra...@apache.org>
Authored: Sat Nov 9 18:47:57 2013 -0800
Committer: randgalt <ra...@apache.org>
Committed: Sat Nov 9 18:47:57 2013 -0800
----------------------------------------------------------------------
.../framework/recipes/nodes/PersistentEphemeralNode.java | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/curator/blob/d8df3b13/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 b178a00..053965b 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
@@ -39,7 +39,6 @@ import java.io.IOException;
import java.util.Arrays;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
/**
@@ -63,7 +62,6 @@ public class PersistentEphemeralNode implements Closeable
private final Mode mode;
private final AtomicReference<byte[]> data = new AtomicReference<byte[]>();
private final AtomicReference<State> state = new AtomicReference<State>(State.LATENT);
- private final AtomicBoolean isSuspended = new AtomicBoolean(false);
private final BackgroundCallback backgroundCallback;
private final Watcher watcher = new Watcher()
{
@@ -81,7 +79,6 @@ public class PersistentEphemeralNode implements Closeable
@Override
public void stateChanged(CuratorFramework client, ConnectionState newState)
{
- isSuspended.set((newState != ConnectionState.RECONNECTED) && (newState != ConnectionState.CONNECTED));
if ( newState == ConnectionState.RECONNECTED )
{
createNode();
@@ -378,6 +375,6 @@ public class PersistentEphemeralNode implements Closeable
private boolean isActive()
{
- return (state.get() == State.STARTED) && !isSuspended.get();
+ return (state.get() == State.STARTED);
}
}
\ No newline at end of file
[4/7] git commit: Added a getter for connectionTimeoutMs
Posted by ra...@apache.org.
Added a getter for connectionTimeoutMs
Project: http://git-wip-us.apache.org/repos/asf/curator/repo
Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/d4135aef
Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/d4135aef
Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/d4135aef
Branch: refs/heads/CURATOR-72
Commit: d4135aef462841027dd53f738a56678db37529dc
Parents: aa5337b
Author: randgalt <ra...@apache.org>
Authored: Sat Nov 9 18:45:04 2013 -0800
Committer: randgalt <ra...@apache.org>
Committed: Sat Nov 9 18:45:04 2013 -0800
----------------------------------------------------------------------
.../java/org/apache/curator/CuratorZookeeperClient.java | 10 ++++++++++
1 file changed, 10 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/curator/blob/d4135aef/curator-client/src/main/java/org/apache/curator/CuratorZookeeperClient.java
----------------------------------------------------------------------
diff --git a/curator-client/src/main/java/org/apache/curator/CuratorZookeeperClient.java b/curator-client/src/main/java/org/apache/curator/CuratorZookeeperClient.java
index ee9cf88..f4e56f9 100644
--- a/curator-client/src/main/java/org/apache/curator/CuratorZookeeperClient.java
+++ b/curator-client/src/main/java/org/apache/curator/CuratorZookeeperClient.java
@@ -269,6 +269,16 @@ public class CuratorZookeeperClient implements Closeable
return state.getEnsembleProvider().getConnectionString();
}
+ /**
+ * Return the configured connection timeout
+ *
+ * @return timeout
+ */
+ public int getConnectionTimeoutMs()
+ {
+ return connectionTimeoutMs;
+ }
+
void addParentWatcher(Watcher watcher)
{
state.addParentWatcher(watcher);
[2/7] git commit: Use a more reasonable retry for better test
reliability
Posted by ra...@apache.org.
Use a more reasonable retry for better test reliability
Project: http://git-wip-us.apache.org/repos/asf/curator/repo
Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/ad033cf1
Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/ad033cf1
Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/ad033cf1
Branch: refs/heads/CURATOR-72
Commit: ad033cf1397c84f9c9f530bb9e877faaf693aa17
Parents: aea4dd8
Author: randgalt <ra...@apache.org>
Authored: Sat Nov 9 18:43:45 2013 -0800
Committer: randgalt <ra...@apache.org>
Committed: Sat Nov 9 18:43:45 2013 -0800
----------------------------------------------------------------------
.../curator/framework/recipes/leader/TestLeaderLatchCluster.java | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/curator/blob/ad033cf1/curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/TestLeaderLatchCluster.java
----------------------------------------------------------------------
diff --git a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/TestLeaderLatchCluster.java b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/TestLeaderLatchCluster.java
index bcb2ac4..90ec122 100644
--- a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/TestLeaderLatchCluster.java
+++ b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/TestLeaderLatchCluster.java
@@ -22,6 +22,7 @@ import com.google.common.collect.Lists;
import com.google.common.io.Closeables;
import org.apache.curator.framework.CuratorFramework;
import org.apache.curator.framework.CuratorFrameworkFactory;
+import org.apache.curator.retry.ExponentialBackoffRetry;
import org.apache.curator.retry.RetryOneTime;
import org.apache.curator.test.InstanceSpec;
import org.apache.curator.test.TestingCluster;
@@ -64,7 +65,7 @@ public class TestLeaderLatchCluster
List<InstanceSpec> instances = Lists.newArrayList(cluster.getInstances());
for ( int i = 0; i < PARTICIPANT_QTY; ++i )
{
- CuratorFramework client = CuratorFrameworkFactory.newClient(instances.get(i).getConnectString(), timing.session(), timing.connection(), new RetryOneTime(1));
+ CuratorFramework client = CuratorFrameworkFactory.newClient(instances.get(i).getConnectString(), timing.session(), timing.connection(), new ExponentialBackoffRetry(100, 3));
LeaderLatch latch = new LeaderLatch(client, "/latch");
clients.add(new ClientAndLatch(client, latch, i));
[3/7] git commit: Avoid a hung test - put a time limit on the queue
access
Posted by ra...@apache.org.
Avoid a hung test - put a time limit on the queue access
Project: http://git-wip-us.apache.org/repos/asf/curator/repo
Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/aa5337b9
Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/aa5337b9
Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/aa5337b9
Branch: refs/heads/CURATOR-72
Commit: aa5337b9c56776339bcdbf6acf260757e73d233f
Parents: ad033cf
Author: randgalt <ra...@apache.org>
Authored: Sat Nov 9 18:44:30 2013 -0800
Committer: randgalt <ra...@apache.org>
Committed: Sat Nov 9 18:44:30 2013 -0800
----------------------------------------------------------------------
.../org/apache/curator/framework/imps/TestNeverConnected.java | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/curator/blob/aa5337b9/curator-framework/src/test/java/org/apache/curator/framework/imps/TestNeverConnected.java
----------------------------------------------------------------------
diff --git a/curator-framework/src/test/java/org/apache/curator/framework/imps/TestNeverConnected.java b/curator-framework/src/test/java/org/apache/curator/framework/imps/TestNeverConnected.java
index 49ac850..0402c45 100644
--- a/curator-framework/src/test/java/org/apache/curator/framework/imps/TestNeverConnected.java
+++ b/curator-framework/src/test/java/org/apache/curator/framework/imps/TestNeverConnected.java
@@ -26,15 +26,19 @@ import org.apache.curator.framework.CuratorFrameworkFactory;
import org.apache.curator.framework.state.ConnectionState;
import org.apache.curator.framework.state.ConnectionStateListener;
import org.apache.curator.retry.RetryOneTime;
+import org.apache.curator.test.Timing;
import org.testng.Assert;
import org.testng.annotations.Test;
import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.TimeUnit;
public class TestNeverConnected
{
@Test
public void testNeverConnected() throws Exception
{
+ Timing timing = new Timing();
+
// use a connection string to a non-existent server
CuratorFramework client = CuratorFrameworkFactory.newClient("localhost:1111", 100, 100, new RetryOneTime(1));
try
@@ -53,7 +57,8 @@ public class TestNeverConnected
client.create().inBackground().forPath("/");
- Assert.assertEquals(queue.take(), ConnectionState.LOST);
+ ConnectionState polled = queue.poll(timing.forWaiting().seconds(), TimeUnit.SECONDS);
+ Assert.assertEquals(polled, ConnectionState.LOST);
}
finally
{
[5/7] git commit: Added test to expose issue CURATOR-72
Posted by ra...@apache.org.
Added test to expose issue CURATOR-72
Project: http://git-wip-us.apache.org/repos/asf/curator/repo
Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/28b63b0a
Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/28b63b0a
Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/28b63b0a
Branch: refs/heads/CURATOR-72
Commit: 28b63b0a1ef5f56d8d40e276b75b7e66a70c335b
Parents: d4135ae
Author: randgalt <ra...@apache.org>
Authored: Sat Nov 9 18:45:41 2013 -0800
Committer: randgalt <ra...@apache.org>
Committed: Sat Nov 9 18:45:41 2013 -0800
----------------------------------------------------------------------
.../framework/imps/TestFrameworkBackground.java | 52 ++++++++++++++++++++
1 file changed, 52 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/curator/blob/28b63b0a/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkBackground.java
----------------------------------------------------------------------
diff --git a/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkBackground.java b/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkBackground.java
index 5aa1353..0439e4a 100644
--- a/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkBackground.java
+++ b/curator-framework/src/test/java/org/apache/curator/framework/imps/TestFrameworkBackground.java
@@ -25,8 +25,11 @@ import org.apache.curator.framework.CuratorFramework;
import org.apache.curator.framework.CuratorFrameworkFactory;
import org.apache.curator.framework.api.BackgroundCallback;
import org.apache.curator.framework.api.CuratorEvent;
+import org.apache.curator.framework.state.ConnectionState;
+import org.apache.curator.framework.state.ConnectionStateListener;
import org.apache.curator.retry.RetryNTimes;
import org.apache.curator.retry.RetryOneTime;
+import org.apache.curator.test.TestingServer;
import org.apache.curator.test.Timing;
import org.apache.zookeeper.KeeperException.Code;
import org.testng.Assert;
@@ -34,11 +37,60 @@ import org.testng.annotations.Test;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.atomic.AtomicReference;
public class TestFrameworkBackground extends BaseClassForTests
{
@Test
+ public void testListenerConnectedAtStart() throws Exception
+ {
+ server.close();
+
+ Timing timing = new Timing(2);
+ CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), timing.session(), timing.connection(), new RetryNTimes(0, 0));
+ try
+ {
+ client.start();
+
+ final CountDownLatch connectedLatch = new CountDownLatch(1);
+ final AtomicBoolean firstListenerAction = new AtomicBoolean(true);
+ final AtomicReference<ConnectionState> firstListenerState = new AtomicReference<ConnectionState>();
+ ConnectionStateListener listener = new ConnectionStateListener()
+ {
+ @Override
+ public void stateChanged(CuratorFramework client, ConnectionState newState)
+ {
+ if ( firstListenerAction.compareAndSet(true, false) ) {
+ firstListenerState.set(newState);
+ System.out.println("First listener state is " + newState);
+ }
+ if ( newState == ConnectionState.CONNECTED )
+ {
+ connectedLatch.countDown();
+ }
+ }
+ };
+ client.getConnectionStateListenable().addListener(listener);
+
+ // due to CURATOR-72, this was causing a LOST event to precede the CONNECTED event
+ client.create().inBackground().forPath("/foo");
+
+ server = new TestingServer(server.getPort());
+
+ Assert.assertTrue(timing.awaitLatch(connectedLatch));
+ Assert.assertFalse(firstListenerAction.get());
+ ConnectionState firstconnectionState = firstListenerState.get();
+ Assert.assertEquals(firstconnectionState, ConnectionState.CONNECTED, "First listener state MUST BE CONNECTED but is " + firstconnectionState);
+ }
+ finally
+ {
+ Closeables.closeQuietly(client);
+ }
+ }
+
+ @Test
public void testRetries() throws Exception
{
final int SLEEP = 1000;