On 6/21/11 12:45 PM, Patrick Hunt wrote:
Such uses of sleep are just asking for trouble. Take a look at the use
of sleep in testSessionMove in the same class for a better way to do
this. I had gone through all the tests a while back, replacing all the
"sleep(x)" with something like this testSessionMove pattern (retry
with a max limit that's very long). During reviews we should look for
anti-patterns like this and address them before commit.
Patrick
Thanks a lot for bringing this up, Camille. I had exactly this problem
(QuorumTest.testFollowersStartAfterLeader failing) yesterday and today .
Would the attached patch be the fix in the spirit of the pattern you're
describing, Patrick?
-Eugene
diff --git src/java/test/org/apache/zookeeper/test/QuorumTest.java
src/java/test/org/apache/zookeeper/test/QuorumTest.java
index 11c5480..73196aa 100644
--- src/java/test/org/apache/zookeeper/test/QuorumTest.java
+++ src/java/test/org/apache/zookeeper/test/QuorumTest.java
@@ -316,11 +316,22 @@ public class QuorumTest extends QuorumBase {
QuorumBase.waitForServerUp(
"127.0.0.1:" + qu.getPeer(2).clientPort,
CONNECTION_TIMEOUT));
- Thread.sleep(1000);
- // zk should have reconnected already
- zk.create("/test", "test".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE,
- CreateMode.PERSISTENT);
+
+ for (int i = 0; i < 30; i++) {
+ try {
+ // zk should have reconnected already
+ zk.create("/test", "test".getBytes(),
ZooDefs.Ids.OPEN_ACL_UNSAFE,
+ CreateMode.PERSISTENT);
+ break;
+ } catch(KeeperException.ConnectionLossException e) {
+ Thread.sleep(1000);
+ }
+ // test fails if we still can't connect to the quorum after 30
seconds.
+ Assert.assertTrue(false);
+ }
+
+
zk.close();
}
