QuorumTest.testFollowersStartAfterLeader
I see that this has failed a few precommit builds now, and someone reported it failing in their local env regularly. Do we think this is just a general transient test, or was there a change checked in recently that might related to this code and its new transient failures? Perhaps it is just the Thread.sleep(1000) needs to be bumped up. C
Re: QuorumTest.testFollowersStartAfterLeader
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 On Tue, Jun 21, 2011 at 12:38 PM, Fournier, Camille F. wrote: > I see that this has failed a few precommit builds now, and someone reported > it failing in their local env regularly. Do we think this is just a general > transient test, or was there a change checked in recently that might related > to this code and its new transient failures? Perhaps it is just the > Thread.sleep(1000) needs to be bumped up. > > C >
Re: QuorumTest.testFollowersStartAfterLeader
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();
}
Re: QuorumTest.testFollowersStartAfterLeader
Hi Eugene, that looks right to me. (did that fix it for you?)
In addition to the anti-pattern I mentioned earlier, another one to
look for is slow running tests -- often times a test will run slowly
that could be coded in a different way to run much more quickly.
Notice here that we don't even wait the second if the test is already
passing. Although we might run for a much longer time if needed. (this
all adds up when you have hundreds/thousands of tests).
btw, you might put that comment directly into your assert. Also take a
look at Assert.fail("foo") instead of assertTrue(false). (it's a nit
though).
Patrick
On Tue, Jun 21, 2011 at 2:03 PM, Eugene Koontz wrote:
> 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
>
>
>
Re: QuorumTest.testFollowersStartAfterLeader
On 6/21/11 2:12 PM, Patrick Hunt wrote:
Hi Eugene, that looks right to me. (did that fix it for you?)
In addition to the anti-pattern I mentioned earlier, another one to
look for is slow running tests -- often times a test will run slowly
that could be coded in a different way to run much more quickly.
Notice here that we don't even wait the second if the test is already
passing. Although we might run for a much longer time if needed. (this
all adds up when you have hundreds/thousands of tests).
btw, you might put that comment directly into your assert. Also take a
look at Assert.fail("foo") instead of assertTrue(false). (it's a nit
though).
Patrick
Hi Patrick,
I created https://issues.apache.org/jira/browse/ZOOKEEPER-1103 and added
a patch. I used Assert.fail() as you recommended. Your comments
regarding avoiding unnecessary sleep()s are well said - I hate waiting
for long tests! :)
-Eugene
Re: QuorumTest.testFollowersStartAfterLeader
On 6/21/11 2:33 PM, Eugene Koontz wrote: On 6/21/11 2:12 PM, Patrick Hunt wrote: Hi Eugene, that looks right to me. (did that fix it for you?) Forgot to say; yes, it does fix QuorumTest for me. I used the attached script file to run this test repeatedly until failure, and it never exited. -Eugene q_test.sh Description: Bourne shell script
Re: QuorumTest.testFollowersStartAfterLeader
Cool, thanks for the patch. Looks like this effects both 3.3 and trunk. On Tue, Jun 21, 2011 at 2:37 PM, Eugene Koontz wrote: > On 6/21/11 2:33 PM, Eugene Koontz wrote: >> >> On 6/21/11 2:12 PM, Patrick Hunt wrote: >>> >>> Hi Eugene, that looks right to me. (did that fix it for you?) >>> > Forgot to say; yes, it does fix QuorumTest for me. I used the attached > script file to run this test repeatedly until failure, and it never exited. > -Eugene >
[jira] [Created] (ZOOKEEPER-1478) Small bug in QuorumTest.testFollowersStartAfterLeader( )
Alexander Shraer created ZOOKEEPER-1478:
---
Summary: Small bug in QuorumTest.testFollowersStartAfterLeader( )
Key: ZOOKEEPER-1478
URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1478
Project: ZooKeeper
Issue Type: Bug
Components: tests
Reporter: Alexander Shraer
Priority: Minor
The following code appears in QuorumTest.testFollowersStartAfterLeader( ):
for (int i = 0; i < 30; i++) {
try {
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.fail("client could not connect to reestablished quorum: giving up
after 30+ seconds.");
}
>From the comment it looks like the intention was to try to reconnect 30 times
>and only then trigger the Assert, but that's not what this does.
After we fail to connect once and Thread.sleep is executed, Assert.fail will be
executed without retrying create.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (ZOOKEEPER-1478) Small bug in QuorumTest.testFollowersStartAfterLeader( )
[
https://issues.apache.org/jira/browse/ZOOKEEPER-1478?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Alexander Shraer updated ZOOKEEPER-1478:
Attachment: ZOOKEEPER-1478.patch
> Small bug in QuorumTest.testFollowersStartAfterLeader( )
>
>
> Key: ZOOKEEPER-1478
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1478
> Project: ZooKeeper
> Issue Type: Bug
> Components: tests
>Reporter: Alexander Shraer
>Assignee: Alexander Shraer
>Priority: Minor
> Attachments: ZOOKEEPER-1478.patch
>
>
> The following code appears in QuorumTest.testFollowersStartAfterLeader( ):
> for (int i = 0; i < 30; i++) {
> try {
>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.fail("client could not connect to reestablished quorum: giving up
> after 30+ seconds.");
> }
> From the comment it looks like the intention was to try to reconnect 30 times
> and only then trigger the Assert, but that's not what this does.
> After we fail to connect once and Thread.sleep is executed, Assert.fail will
> be executed without retrying create.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (ZOOKEEPER-1478) Small bug in QuorumTest.testFollowersStartAfterLeader( )
[
https://issues.apache.org/jira/browse/ZOOKEEPER-1478?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13288072#comment-13288072
]
Hadoop QA commented on ZOOKEEPER-1478:
--
+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12530678/ZOOKEEPER-1478.patch
against trunk revision 1337029.
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 3 new or modified tests.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac
compiler warnings.
+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9)
warnings.
+1 release audit. The applied patch does not increase the total number of
release audit warnings.
+1 core tests. The patch passed core unit tests.
+1 contrib tests. The patch passed contrib unit tests.
Test results:
https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1086//testReport/
Findbugs warnings:
https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1086//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output:
https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1086//console
This message is automatically generated.
> Small bug in QuorumTest.testFollowersStartAfterLeader( )
>
>
> Key: ZOOKEEPER-1478
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1478
> Project: ZooKeeper
> Issue Type: Bug
> Components: tests
>Reporter: Alexander Shraer
>Assignee: Alexander Shraer
>Priority: Minor
> Attachments: ZOOKEEPER-1478.patch
>
>
> The following code appears in QuorumTest.testFollowersStartAfterLeader( ):
> for (int i = 0; i < 30; i++) {
> try {
>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.fail("client could not connect to reestablished quorum: giving up
> after 30+ seconds.");
> }
> From the comment it looks like the intention was to try to reconnect 30 times
> and only then trigger the Assert, but that's not what this does.
> After we fail to connect once and Thread.sleep is executed, Assert.fail will
> be executed without retrying create.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (ZOOKEEPER-1478) Small bug in QuorumTest.testFollowersStartAfterLeader( )
[
https://issues.apache.org/jira/browse/ZOOKEEPER-1478?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Flavio Junqueira updated ZOOKEEPER-1478:
Attachment: ZOOKEEPER-1478.patch
> Small bug in QuorumTest.testFollowersStartAfterLeader( )
>
>
> Key: ZOOKEEPER-1478
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1478
> Project: ZooKeeper
> Issue Type: Bug
> Components: tests
>Reporter: Alexander Shraer
>Assignee: Alexander Shraer
>Priority: Minor
> Attachments: ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch
>
>
> The following code appears in QuorumTest.testFollowersStartAfterLeader( ):
> for (int i = 0; i < 30; i++) {
> try {
>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.fail("client could not connect to reestablished quorum: giving up
> after 30+ seconds.");
> }
> From the comment it looks like the intention was to try to reconnect 30 times
> and only then trigger the Assert, but that's not what this does.
> After we fail to connect once and Thread.sleep is executed, Assert.fail will
> be executed without retrying create.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (ZOOKEEPER-1478) Small bug in QuorumTest.testFollowersStartAfterLeader( )
[
https://issues.apache.org/jira/browse/ZOOKEEPER-1478?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13396892#comment-13396892
]
Flavio Junqueira commented on ZOOKEEPER-1478:
-
Good catch, Alex. I was wondering why we are trying to create znodes instead of
trying to simply check if it is connected. Is it because we can have a race and
the output of waitForConnected is not reliable? Check the latest patch I have
uploaded to see the approach I'm thinking about.
Other than this issue, the patch you submitted looks good.
> Small bug in QuorumTest.testFollowersStartAfterLeader( )
>
>
> Key: ZOOKEEPER-1478
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1478
> Project: ZooKeeper
> Issue Type: Bug
> Components: tests
>Reporter: Alexander Shraer
>Assignee: Alexander Shraer
>Priority: Minor
> Attachments: ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch
>
>
> The following code appears in QuorumTest.testFollowersStartAfterLeader( ):
> for (int i = 0; i < 30; i++) {
> try {
>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.fail("client could not connect to reestablished quorum: giving up
> after 30+ seconds.");
> }
> From the comment it looks like the intention was to try to reconnect 30 times
> and only then trigger the Assert, but that's not what this does.
> After we fail to connect once and Thread.sleep is executed, Assert.fail will
> be executed without retrying create.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (ZOOKEEPER-1478) Small bug in QuorumTest.testFollowersStartAfterLeader( )
[
https://issues.apache.org/jira/browse/ZOOKEEPER-1478?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13397136#comment-13397136
]
Alexander Shraer commented on ZOOKEEPER-1478:
-
Hi Flavio, this looks good, although I don't know the details of ZK-790 to
which this test is related, so not sure what it is supposed to do exactly. One
thing a bit odd is the line
QuorumBase.waitForServerUp(
"127.0.0.1:" + qu.getPeer(2).clientPort,
CONNECTION_TIMEOUT));
above your changes. Server 2 might not have been even shut down (it should be
"index" ?) and in any case your new line waitForConnected seems to be
sufficient even without the above line. right ?
> Small bug in QuorumTest.testFollowersStartAfterLeader( )
>
>
> Key: ZOOKEEPER-1478
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1478
> Project: ZooKeeper
> Issue Type: Bug
> Components: tests
>Reporter: Alexander Shraer
>Assignee: Alexander Shraer
>Priority: Minor
> Attachments: ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch
>
>
> The following code appears in QuorumTest.testFollowersStartAfterLeader( ):
> for (int i = 0; i < 30; i++) {
> try {
>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.fail("client could not connect to reestablished quorum: giving up
> after 30+ seconds.");
> }
> From the comment it looks like the intention was to try to reconnect 30 times
> and only then trigger the Assert, but that's not what this does.
> After we fail to connect once and Thread.sleep is executed, Assert.fail will
> be executed without retrying create.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (ZOOKEEPER-1478) Small bug in QuorumTest.testFollowersStartAfterLeader( )
[
https://issues.apache.org/jira/browse/ZOOKEEPER-1478?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13397333#comment-13397333
]
Flavio Junqueira commented on ZOOKEEPER-1478:
-
I had a look at ZOOKEEPER-790 and if I checked it right, and this change to run
30 times create was introduced in ZOOKEEPER-1103.
We are just trying to check if the client is able to reconnect to the ensemble
after a leader demotion, so I think the way I'm proposing is cleaner. I'll be
uploading shortly a small change that will guarantee that the race I was
referring to doesn't happen by waiting until we actually observe that the
client has disconnected.
About the waitForServerUp statement, it doesn't look strictly necessary by the
way we create the zookeeper object. It sounds ok to remove.
> Small bug in QuorumTest.testFollowersStartAfterLeader( )
>
>
> Key: ZOOKEEPER-1478
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1478
> Project: ZooKeeper
> Issue Type: Bug
> Components: tests
>Reporter: Alexander Shraer
>Assignee: Alexander Shraer
>Priority: Minor
> Attachments: ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch
>
>
> The following code appears in QuorumTest.testFollowersStartAfterLeader( ):
> for (int i = 0; i < 30; i++) {
> try {
>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.fail("client could not connect to reestablished quorum: giving up
> after 30+ seconds.");
> }
> From the comment it looks like the intention was to try to reconnect 30 times
> and only then trigger the Assert, but that's not what this does.
> After we fail to connect once and Thread.sleep is executed, Assert.fail will
> be executed without retrying create.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (ZOOKEEPER-1478) Small bug in QuorumTest.testFollowersStartAfterLeader( )
[
https://issues.apache.org/jira/browse/ZOOKEEPER-1478?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Flavio Junqueira updated ZOOKEEPER-1478:
Attachment: ZOOKEEPER-1478.patch
Change to wait until client actually observes the disconnection.
> Small bug in QuorumTest.testFollowersStartAfterLeader( )
>
>
> Key: ZOOKEEPER-1478
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1478
> Project: ZooKeeper
> Issue Type: Bug
> Components: tests
>Reporter: Alexander Shraer
>Assignee: Alexander Shraer
>Priority: Minor
> Attachments: ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch,
> ZOOKEEPER-1478.patch
>
>
> The following code appears in QuorumTest.testFollowersStartAfterLeader( ):
> for (int i = 0; i < 30; i++) {
> try {
>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.fail("client could not connect to reestablished quorum: giving up
> after 30+ seconds.");
> }
> From the comment it looks like the intention was to try to reconnect 30 times
> and only then trigger the Assert, but that's not what this does.
> After we fail to connect once and Thread.sleep is executed, Assert.fail will
> be executed without retrying create.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (ZOOKEEPER-1478) Small bug in QuorumTest.testFollowersStartAfterLeader( )
[
https://issues.apache.org/jira/browse/ZOOKEEPER-1478?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13397357#comment-13397357
]
Hadoop QA commented on ZOOKEEPER-1478:
--
+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12532665/ZOOKEEPER-1478.patch
against trunk revision 1351541.
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 4 new or modified tests.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac
compiler warnings.
+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9)
warnings.
+1 release audit. The applied patch does not increase the total number of
release audit warnings.
+1 core tests. The patch passed core unit tests.
+1 contrib tests. The patch passed contrib unit tests.
Test results:
https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1100//testReport/
Findbugs warnings:
https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1100//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output:
https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1100//console
This message is automatically generated.
> Small bug in QuorumTest.testFollowersStartAfterLeader( )
>
>
> Key: ZOOKEEPER-1478
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1478
> Project: ZooKeeper
> Issue Type: Bug
> Components: tests
>Reporter: Alexander Shraer
>Assignee: Alexander Shraer
>Priority: Minor
> Attachments: ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch,
> ZOOKEEPER-1478.patch
>
>
> The following code appears in QuorumTest.testFollowersStartAfterLeader( ):
> for (int i = 0; i < 30; i++) {
> try {
>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.fail("client could not connect to reestablished quorum: giving up
> after 30+ seconds.");
> }
> From the comment it looks like the intention was to try to reconnect 30 times
> and only then trigger the Assert, but that's not what this does.
> After we fail to connect once and Thread.sleep is executed, Assert.fail will
> be executed without retrying create.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (ZOOKEEPER-1478) Small bug in QuorumTest.testFollowersStartAfterLeader( )
[
https://issues.apache.org/jira/browse/ZOOKEEPER-1478?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Alexander Shraer updated ZOOKEEPER-1478:
Attachment: ZOOKEEPER-1478.patch
removed waitForServerUp
> Small bug in QuorumTest.testFollowersStartAfterLeader( )
>
>
> Key: ZOOKEEPER-1478
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1478
> Project: ZooKeeper
> Issue Type: Bug
> Components: tests
>Reporter: Alexander Shraer
>Assignee: Alexander Shraer
>Priority: Minor
> Attachments: ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch,
> ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch
>
>
> The following code appears in QuorumTest.testFollowersStartAfterLeader( ):
> for (int i = 0; i < 30; i++) {
> try {
>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.fail("client could not connect to reestablished quorum: giving up
> after 30+ seconds.");
> }
> From the comment it looks like the intention was to try to reconnect 30 times
> and only then trigger the Assert, but that's not what this does.
> After we fail to connect once and Thread.sleep is executed, Assert.fail will
> be executed without retrying create.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (ZOOKEEPER-1478) Small bug in QuorumTest.testFollowersStartAfterLeader( )
[
https://issues.apache.org/jira/browse/ZOOKEEPER-1478?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13397585#comment-13397585
]
Hadoop QA commented on ZOOKEEPER-1478:
--
+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12532701/ZOOKEEPER-1478.patch
against trunk revision 1351541.
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 4 new or modified tests.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac
compiler warnings.
+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9)
warnings.
+1 release audit. The applied patch does not increase the total number of
release audit warnings.
+1 core tests. The patch passed core unit tests.
+1 contrib tests. The patch passed contrib unit tests.
Test results:
https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1101//testReport/
Findbugs warnings:
https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1101//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output:
https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1101//console
This message is automatically generated.
> Small bug in QuorumTest.testFollowersStartAfterLeader( )
>
>
> Key: ZOOKEEPER-1478
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1478
> Project: ZooKeeper
> Issue Type: Bug
> Components: tests
>Reporter: Alexander Shraer
>Assignee: Alexander Shraer
>Priority: Minor
> Attachments: ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch,
> ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch
>
>
> The following code appears in QuorumTest.testFollowersStartAfterLeader( ):
> for (int i = 0; i < 30; i++) {
> try {
>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.fail("client could not connect to reestablished quorum: giving up
> after 30+ seconds.");
> }
> From the comment it looks like the intention was to try to reconnect 30 times
> and only then trigger the Assert, but that's not what this does.
> After we fail to connect once and Thread.sleep is executed, Assert.fail will
> be executed without retrying create.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (ZOOKEEPER-1478) Small bug in QuorumTest.testFollowersStartAfterLeader( )
[
https://issues.apache.org/jira/browse/ZOOKEEPER-1478?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13398927#comment-13398927
]
Flavio Junqueira commented on ZOOKEEPER-1478:
-
This is a +1 for me, but since I added code to the patch, it might be a good
idea to have someone else having a look at it too.
> Small bug in QuorumTest.testFollowersStartAfterLeader( )
>
>
> Key: ZOOKEEPER-1478
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1478
> Project: ZooKeeper
> Issue Type: Bug
> Components: tests
>Reporter: Alexander Shraer
>Assignee: Alexander Shraer
>Priority: Minor
> Attachments: ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch,
> ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch
>
>
> The following code appears in QuorumTest.testFollowersStartAfterLeader( ):
> for (int i = 0; i < 30; i++) {
> try {
>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.fail("client could not connect to reestablished quorum: giving up
> after 30+ seconds.");
> }
> From the comment it looks like the intention was to try to reconnect 30 times
> and only then trigger the Assert, but that's not what this does.
> After we fail to connect once and Thread.sleep is executed, Assert.fail will
> be executed without retrying create.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (ZOOKEEPER-1478) Small bug in QuorumTest.testFollowersStartAfterLeader( )
[
https://issues.apache.org/jira/browse/ZOOKEEPER-1478?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Patrick Hunt updated ZOOKEEPER-1478:
Fix Version/s: 3.5.0
3.4.4
> Small bug in QuorumTest.testFollowersStartAfterLeader( )
>
>
> Key: ZOOKEEPER-1478
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1478
> Project: ZooKeeper
> Issue Type: Bug
> Components: tests
>Reporter: Alexander Shraer
>Assignee: Alexander Shraer
>Priority: Minor
> Fix For: 3.4.4, 3.5.0
>
> Attachments: ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch,
> ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch
>
>
> The following code appears in QuorumTest.testFollowersStartAfterLeader( ):
> for (int i = 0; i < 30; i++) {
> try {
>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.fail("client could not connect to reestablished quorum: giving up
> after 30+ seconds.");
> }
> From the comment it looks like the intention was to try to reconnect 30 times
> and only then trigger the Assert, but that's not what this does.
> After we fail to connect once and Thread.sleep is executed, Assert.fail will
> be executed without retrying create.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (ZOOKEEPER-1478) Small bug in QuorumTest.testFollowersStartAfterLeader( )
[
https://issues.apache.org/jira/browse/ZOOKEEPER-1478?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Mahadev konar updated ZOOKEEPER-1478:
-
Affects Version/s: 3.4.3
> Small bug in QuorumTest.testFollowersStartAfterLeader( )
>
>
> Key: ZOOKEEPER-1478
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1478
> Project: ZooKeeper
> Issue Type: Bug
> Components: tests
>Affects Versions: 3.4.3
>Reporter: Alexander Shraer
>Assignee: Alexander Shraer
>Priority: Minor
> Fix For: 3.4.4, 3.5.0
>
> Attachments: ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch,
> ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch
>
>
> The following code appears in QuorumTest.testFollowersStartAfterLeader( ):
> for (int i = 0; i < 30; i++) {
> try {
>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.fail("client could not connect to reestablished quorum: giving up
> after 30+ seconds.");
> }
> From the comment it looks like the intention was to try to reconnect 30 times
> and only then trigger the Assert, but that's not what this does.
> After we fail to connect once and Thread.sleep is executed, Assert.fail will
> be executed without retrying create.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (ZOOKEEPER-1478) Small bug in QuorumTest.testFollowersStartAfterLeader( )
[
https://issues.apache.org/jira/browse/ZOOKEEPER-1478?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Mahadev konar updated ZOOKEEPER-1478:
-
Fix Version/s: (was: 3.4.4)
Moving it out to 3.5 since the bugfix isnt a really critical one.
> Small bug in QuorumTest.testFollowersStartAfterLeader( )
>
>
> Key: ZOOKEEPER-1478
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1478
> Project: ZooKeeper
> Issue Type: Bug
> Components: tests
>Affects Versions: 3.4.3
>Reporter: Alexander Shraer
>Assignee: Alexander Shraer
>Priority: Minor
> Fix For: 3.5.0
>
> Attachments: ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch,
> ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch
>
>
> The following code appears in QuorumTest.testFollowersStartAfterLeader( ):
> for (int i = 0; i < 30; i++) {
> try {
>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.fail("client could not connect to reestablished quorum: giving up
> after 30+ seconds.");
> }
> From the comment it looks like the intention was to try to reconnect 30 times
> and only then trigger the Assert, but that's not what this does.
> After we fail to connect once and Thread.sleep is executed, Assert.fail will
> be executed without retrying create.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (ZOOKEEPER-1478) Small bug in QuorumTest.testFollowersStartAfterLeader( )
[
https://issues.apache.org/jira/browse/ZOOKEEPER-1478?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Mahadev konar updated ZOOKEEPER-1478:
-
Attachment: ZOOKEEPER-1478.patch
Re uploading the patch for hudson.
> Small bug in QuorumTest.testFollowersStartAfterLeader( )
>
>
> Key: ZOOKEEPER-1478
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1478
> Project: ZooKeeper
> Issue Type: Bug
> Components: tests
>Affects Versions: 3.4.3
>Reporter: Alexander Shraer
>Assignee: Alexander Shraer
>Priority: Minor
> Fix For: 3.5.0
>
> Attachments: ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch,
> ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch
>
>
> The following code appears in QuorumTest.testFollowersStartAfterLeader( ):
> for (int i = 0; i < 30; i++) {
> try {
>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.fail("client could not connect to reestablished quorum: giving up
> after 30+ seconds.");
> }
> From the comment it looks like the intention was to try to reconnect 30 times
> and only then trigger the Assert, but that's not what this does.
> After we fail to connect once and Thread.sleep is executed, Assert.fail will
> be executed without retrying create.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (ZOOKEEPER-1478) Small bug in QuorumTest.testFollowersStartAfterLeader( )
[
https://issues.apache.org/jira/browse/ZOOKEEPER-1478?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13452351#comment-13452351
]
Hadoop QA commented on ZOOKEEPER-1478:
--
+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12544504/ZOOKEEPER-1478.patch
against trunk revision 1382661.
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 4 new or modified tests.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac
compiler warnings.
+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9)
warnings.
+1 release audit. The applied patch does not increase the total number of
release audit warnings.
+1 core tests. The patch passed core unit tests.
+1 contrib tests. The patch passed contrib unit tests.
Test results:
https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1181//testReport/
Findbugs warnings:
https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1181//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output:
https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1181//console
This message is automatically generated.
> Small bug in QuorumTest.testFollowersStartAfterLeader( )
>
>
> Key: ZOOKEEPER-1478
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1478
> Project: ZooKeeper
> Issue Type: Bug
> Components: tests
>Affects Versions: 3.4.3
>Reporter: Alexander Shraer
>Assignee: Alexander Shraer
>Priority: Minor
> Fix For: 3.5.0
>
> Attachments: ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch,
> ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch
>
>
> The following code appears in QuorumTest.testFollowersStartAfterLeader( ):
> for (int i = 0; i < 30; i++) {
> try {
>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.fail("client could not connect to reestablished quorum: giving up
> after 30+ seconds.");
> }
> From the comment it looks like the intention was to try to reconnect 30 times
> and only then trigger the Assert, but that's not what this does.
> After we fail to connect once and Thread.sleep is executed, Assert.fail will
> be executed without retrying create.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (ZOOKEEPER-1478) Small bug in QuorumTest.testFollowersStartAfterLeader( )
[
https://issues.apache.org/jira/browse/ZOOKEEPER-1478?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13462412#comment-13462412
]
Benjamin Reed commented on ZOOKEEPER-1478:
--
+1 looks good to me!
> Small bug in QuorumTest.testFollowersStartAfterLeader( )
>
>
> Key: ZOOKEEPER-1478
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1478
> Project: ZooKeeper
> Issue Type: Bug
> Components: tests
>Affects Versions: 3.4.3
>Reporter: Alexander Shraer
>Assignee: Alexander Shraer
>Priority: Minor
> Fix For: 3.5.0
>
> Attachments: ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch,
> ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch
>
>
> The following code appears in QuorumTest.testFollowersStartAfterLeader( ):
> for (int i = 0; i < 30; i++) {
> try {
>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.fail("client could not connect to reestablished quorum: giving up
> after 30+ seconds.");
> }
> From the comment it looks like the intention was to try to reconnect 30 times
> and only then trigger the Assert, but that's not what this does.
> After we fail to connect once and Thread.sleep is executed, Assert.fail will
> be executed without retrying create.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (ZOOKEEPER-1478) Small bug in QuorumTest.testFollowersStartAfterLeader( )
[
https://issues.apache.org/jira/browse/ZOOKEEPER-1478?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13530898#comment-13530898
]
Hudson commented on ZOOKEEPER-1478:
---
Integrated in ZooKeeper-trunk #1771 (See
[https://builds.apache.org/job/ZooKeeper-trunk/1771/])
ZOOKEEPER-1478. Small bug in QuorumTest.testFollowersStartAfterLeader( )
(Alexander Shraer via fpj, breed, phunt) (Revision 1421091)
Result = SUCCESS
phunt : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1421091
Files :
* /zookeeper/trunk/CHANGES.txt
* /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/QuorumTest.java
> Small bug in QuorumTest.testFollowersStartAfterLeader( )
>
>
> Key: ZOOKEEPER-1478
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1478
> Project: ZooKeeper
> Issue Type: Bug
> Components: tests
>Affects Versions: 3.4.3
>Reporter: Alexander Shraer
>Assignee: Alexander Shraer
>Priority: Minor
> Fix For: 3.5.0, 3.4.6
>
> Attachments: ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch,
> ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch, ZOOKEEPER-1478.patch
>
>
> The following code appears in QuorumTest.testFollowersStartAfterLeader( ):
> for (int i = 0; i < 30; i++) {
> try {
>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.fail("client could not connect to reestablished quorum: giving up
> after 30+ seconds.");
> }
> From the comment it looks like the intention was to try to reconnect 30 times
> and only then trigger the Assert, but that's not what this does.
> After we fail to connect once and Thread.sleep is executed, Assert.fail will
> be executed without retrying create.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
