+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12567954/zookeeper-1606.patch
against trunk revision 1441922.
+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/1385//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1385//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1385//console
This message is automatically generated.
There seems to be a contradiction between the comment and the code.
{ throw new Exception("the last server is not the leader"); }//find out who is the leader and kill it
if ( qb.s5.getPeerState() != ServerState.LEADING)
I'm new here so there may be a very good reason that I'm not aware of that the last server will be the leader. But the comment clearly says differently, stating the code below intend to find out who is the leader. Isn't it supposed to be s5 all the time? According to the code.