You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by Alexander Shraer <sh...@yahoo-inc.com> on 2012/11/08 07:34:43 UTC

Re: Review Request: Dynamic reconfiguration, see https://issues.apache.org/jira/browse/ZOOKEEPER-107

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6707/
-----------------------------------------------------------

(Updated Nov. 8, 2012, 6:34 a.m.)


Review request for zookeeper.


Changes
-------

This includes everything except for the C client tests, which will be available soon. Its the same as
ZOOKEEPER-107-6-NOV-2.patch on the Jira, for which all tests passed. 


Description
-------

see https://issues.apache.org/jira/browse/ZOOKEEPER-107


Diffs (updated)
-----

  /src/c/include/proto.h 1406719 
  /src/c/include/zookeeper.h 1406719 
  /src/c/src/cli.c 1406719 
  /src/c/src/zookeeper.c 1406719 
  /src/java/main/org/apache/zookeeper/KeeperException.java 1406719 
  /src/java/main/org/apache/zookeeper/ZooDefs.java 1406719 
  /src/java/main/org/apache/zookeeper/ZooKeeper.java 1406719 
  /src/java/main/org/apache/zookeeper/ZooKeeperMain.java 1406719 
  /src/java/main/org/apache/zookeeper/cli/GetConfigCommand.java PRE-CREATION 
  /src/java/main/org/apache/zookeeper/cli/ReconfigCommand.java PRE-CREATION 
  /src/java/main/org/apache/zookeeper/server/DataTree.java 1406719 
  /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1406719 
  /src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 1406719 
  /src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 1406719 
  /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1406719 
  /src/java/main/org/apache/zookeeper/server/Request.java 1406719 
  /src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 1406719 
  /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1406719 
  /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1406719 
  /src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 1406719 
  /src/java/main/org/apache/zookeeper/server/quorum/Follower.java 1406719 
  /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1406719 
  /src/java/main/org/apache/zookeeper/server/quorum/Leader.java 1406719 
  /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1406719 
  /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1406719 
  /src/java/main/org/apache/zookeeper/server/quorum/Observer.java 1406719 
  /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1406719 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumBean.java 1406719 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 1406719 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1406719 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1406719 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1406719 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1406719 
  /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1406719 
  /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumHierarchical.java 1406719 
  /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumMaj.java 1406719 
  /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumVerifier.java 1406719 
  /src/java/main/org/apache/zookeeper/server/util/ConfigUtils.java PRE-CREATION 
  /src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1406719 
  /src/java/test/org/apache/zookeeper/server/TruncateCorruptionTest.java 1406719 
  /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 1406719 
  /src/java/test/org/apache/zookeeper/server/quorum/ReconfigRecoveryTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 1406719 
  /src/java/test/org/apache/zookeeper/test/CnxManagerTest.java 1406719 
  /src/java/test/org/apache/zookeeper/test/QuorumMajorityTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1406719 
  /src/java/test/org/apache/zookeeper/test/ReconfigTest.java PRE-CREATION 
  /src/zookeeper.jute 1406719 

Diff: https://reviews.apache.org/r/6707/diff/


Testing
-------

New test files were added: ReconfigTest, ReconfigRecoveryTest and QuorumMajorityTest. Many other tests were modified. 


Thanks,

Alexander Shraer


Re: Review Request: Dynamic reconfiguration, see https://issues.apache.org/jira/browse/ZOOKEEPER-107

Posted by Edward Ribeiro <ed...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6707/#review15614
-----------------------------------------------------------



/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
<https://reviews.apache.org/r/6707/#comment33712>

    Lines 500-504 are a parsing logic that obfuscate the core logic of this part of the code. I suggest to extract these lines to a parse method that could return an array with two slots, for example, to be used at line 505.


- Edward Ribeiro


On Nov. 29, 2012, 7:12 a.m., Alexander Shraer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6707/
> -----------------------------------------------------------
> 
> (Updated Nov. 29, 2012, 7:12 a.m.)
> 
> 
> Review request for zookeeper.
> 
> 
> Description
> -------
> 
> see https://issues.apache.org/jira/browse/ZOOKEEPER-107
> 
> 
> Diffs
> -----
> 
>   /src/c/include/proto.h 1415037 
>   /src/c/include/zookeeper.h 1415037 
>   /src/c/src/cli.c 1415037 
>   /src/c/src/zookeeper.c 1415037 
>   /src/java/main/org/apache/zookeeper/KeeperException.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooDefs.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooKeeper.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooKeeperMain.java 1415037 
>   /src/java/main/org/apache/zookeeper/cli/GetConfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/cli/ReconfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/DataTree.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/Request.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Follower.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Leader.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Observer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumBean.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumHierarchical.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumMaj.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumVerifier.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/util/ConfigUtils.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/TruncateCorruptionTest.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/quorum/ReconfigRecoveryTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/CnxManagerTest.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/QuorumMajorityTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/ReconfigTest.java PRE-CREATION 
>   /src/zookeeper.jute 1415037 
> 
> Diff: https://reviews.apache.org/r/6707/diff/
> 
> 
> Testing
> -------
> 
> New test files were added: ReconfigTest, ReconfigRecoveryTest and QuorumMajorityTest. Many other tests were modified. 
> 
> 
> Thanks,
> 
> Alexander Shraer
> 
>


Re: Review Request: Dynamic reconfiguration, see https://issues.apache.org/jira/browse/ZOOKEEPER-107

Posted by Alexander Shraer <sh...@yahoo-inc.com>.

> On Dec. 16, 2012, 6:19 a.m., michim wrote:
> >

all comments are now addressed.


- Alexander


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6707/#review13756
-----------------------------------------------------------


On Nov. 29, 2012, 7:12 a.m., Alexander Shraer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6707/
> -----------------------------------------------------------
> 
> (Updated Nov. 29, 2012, 7:12 a.m.)
> 
> 
> Review request for zookeeper.
> 
> 
> Description
> -------
> 
> see https://issues.apache.org/jira/browse/ZOOKEEPER-107
> 
> 
> Diffs
> -----
> 
>   /src/c/include/proto.h 1415037 
>   /src/c/include/zookeeper.h 1415037 
>   /src/c/src/cli.c 1415037 
>   /src/c/src/zookeeper.c 1415037 
>   /src/java/main/org/apache/zookeeper/KeeperException.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooDefs.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooKeeper.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooKeeperMain.java 1415037 
>   /src/java/main/org/apache/zookeeper/cli/GetConfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/cli/ReconfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/DataTree.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/Request.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Follower.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Leader.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Observer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumBean.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumHierarchical.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumMaj.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumVerifier.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/util/ConfigUtils.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/TruncateCorruptionTest.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/quorum/ReconfigRecoveryTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/CnxManagerTest.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/QuorumMajorityTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/ReconfigTest.java PRE-CREATION 
>   /src/zookeeper.jute 1415037 
> 
> Diff: https://reviews.apache.org/r/6707/diff/
> 
> 
> Testing
> -------
> 
> New test files were added: ReconfigTest, ReconfigRecoveryTest and QuorumMajorityTest. Many other tests were modified. 
> 
> 
> Thanks,
> 
> Alexander Shraer
> 
>


Re: Review Request: Dynamic reconfiguration, see https://issues.apache.org/jira/browse/ZOOKEEPER-107

Posted by mi...@cs.stanford.edu.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6707/#review13756
-----------------------------------------------------------



/src/c/include/zookeeper.h
<https://reviews.apache.org/r/6707/#comment29430>

    



/src/zookeeper.jute
<https://reviews.apache.org/r/6707/#comment29428>

    



/src/c/include/zookeeper.h
<https://reviews.apache.org/r/6707/#comment30960>

    Could you document the format of the returned data?



/src/c/src/cli.c
<https://reviews.apache.org/r/6707/#comment30955>

    bad indentation?



/src/c/src/cli.c
<https://reviews.apache.org/r/6707/#comment30956>

    Shouldn't it be "serverId=host:port1:port2:port3"? It might be a good idea to give more descriptive names for the ports.



/src/c/src/zookeeper.c
<https://reviews.apache.org/r/6707/#comment30957>

    Don't use STRUCT_INITIALIZER macro.



/src/c/src/zookeeper.c
<https://reviews.apache.org/r/6707/#comment30958>

    bad indentation?



/src/c/src/zookeeper.c
<https://reviews.apache.org/r/6707/#comment30959>

    Don't use STRUCT_INITIALIZER.


- michim


On Nov. 29, 2012, 7:12 a.m., Alexander Shraer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6707/
> -----------------------------------------------------------
> 
> (Updated Nov. 29, 2012, 7:12 a.m.)
> 
> 
> Review request for zookeeper.
> 
> 
> Description
> -------
> 
> see https://issues.apache.org/jira/browse/ZOOKEEPER-107
> 
> 
> Diffs
> -----
> 
>   /src/c/include/proto.h 1415037 
>   /src/c/include/zookeeper.h 1415037 
>   /src/c/src/cli.c 1415037 
>   /src/c/src/zookeeper.c 1415037 
>   /src/java/main/org/apache/zookeeper/KeeperException.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooDefs.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooKeeper.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooKeeperMain.java 1415037 
>   /src/java/main/org/apache/zookeeper/cli/GetConfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/cli/ReconfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/DataTree.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/Request.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Follower.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Leader.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Observer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumBean.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumHierarchical.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumMaj.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumVerifier.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/util/ConfigUtils.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/TruncateCorruptionTest.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/quorum/ReconfigRecoveryTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/CnxManagerTest.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/QuorumMajorityTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/ReconfigTest.java PRE-CREATION 
>   /src/zookeeper.jute 1415037 
> 
> Diff: https://reviews.apache.org/r/6707/diff/
> 
> 
> Testing
> -------
> 
> New test files were added: ReconfigTest, ReconfigRecoveryTest and QuorumMajorityTest. Many other tests were modified. 
> 
> 
> Thanks,
> 
> Alexander Shraer
> 
>


Re: Review Request: Dynamic reconfiguration, see https://issues.apache.org/jira/browse/ZOOKEEPER-107

Posted by Alexander Shraer <sh...@yahoo-inc.com>.

> On Jan. 23, 2013, 7:30 a.m., Edward Ribeiro wrote:
> > /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java, line 492
> > <https://reviews.apache.org/r/6707/diff/4/?file=231478#file231478line492>
> >
> >     I didn't get why you do this test here. If you try to remove a non-existent entry from a HashMap then HashMap's remove() method returns null, otherwise it returns the object you just removed. Therefore, there's no need to bypass the remove() call if the entry doesn't exists (i.e. remove() doesn't throw an exception).
> >     
> >     You may remove lines 492-494 or, if you really need them, you can rewrite it as:
> >     
> >     if (nextServers.containsKey(sid)) {
> >       nextServers.remove(sid);
> >     }
> >     
> >     IMO, the latter is more readable, but it's just my opinion.

I think the reason for the if was a warning message that once was inside the if... I removed the if.


- Alexander


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6707/#review15612
-----------------------------------------------------------


On Nov. 29, 2012, 7:12 a.m., Alexander Shraer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6707/
> -----------------------------------------------------------
> 
> (Updated Nov. 29, 2012, 7:12 a.m.)
> 
> 
> Review request for zookeeper.
> 
> 
> Description
> -------
> 
> see https://issues.apache.org/jira/browse/ZOOKEEPER-107
> 
> 
> Diffs
> -----
> 
>   /src/c/include/proto.h 1415037 
>   /src/c/include/zookeeper.h 1415037 
>   /src/c/src/cli.c 1415037 
>   /src/c/src/zookeeper.c 1415037 
>   /src/java/main/org/apache/zookeeper/KeeperException.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooDefs.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooKeeper.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooKeeperMain.java 1415037 
>   /src/java/main/org/apache/zookeeper/cli/GetConfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/cli/ReconfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/DataTree.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/Request.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Follower.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Leader.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Observer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumBean.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumHierarchical.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumMaj.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumVerifier.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/util/ConfigUtils.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/TruncateCorruptionTest.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/quorum/ReconfigRecoveryTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/CnxManagerTest.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/QuorumMajorityTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/ReconfigTest.java PRE-CREATION 
>   /src/zookeeper.jute 1415037 
> 
> Diff: https://reviews.apache.org/r/6707/diff/
> 
> 
> Testing
> -------
> 
> New test files were added: ReconfigTest, ReconfigRecoveryTest and QuorumMajorityTest. Many other tests were modified. 
> 
> 
> Thanks,
> 
> Alexander Shraer
> 
>


Re: Review Request: Dynamic reconfiguration, see https://issues.apache.org/jira/browse/ZOOKEEPER-107

Posted by Edward Ribeiro <ed...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6707/#review15612
-----------------------------------------------------------



/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
<https://reviews.apache.org/r/6707/#comment33710>

    I didn't get why you do this test here. If you try to remove a non-existent entry from a HashMap then HashMap's remove() method returns null, otherwise it returns the object you just removed. Therefore, there's no need to bypass the remove() call if the entry doesn't exists (i.e. remove() doesn't throw an exception).
    
    You may remove lines 492-494 or, if you really need them, you can rewrite it as:
    
    if (nextServers.containsKey(sid)) {
      nextServers.remove(sid);
    }
    
    IMO, the latter is more readable, but it's just my opinion.


- Edward Ribeiro


On Nov. 29, 2012, 7:12 a.m., Alexander Shraer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6707/
> -----------------------------------------------------------
> 
> (Updated Nov. 29, 2012, 7:12 a.m.)
> 
> 
> Review request for zookeeper.
> 
> 
> Description
> -------
> 
> see https://issues.apache.org/jira/browse/ZOOKEEPER-107
> 
> 
> Diffs
> -----
> 
>   /src/c/include/proto.h 1415037 
>   /src/c/include/zookeeper.h 1415037 
>   /src/c/src/cli.c 1415037 
>   /src/c/src/zookeeper.c 1415037 
>   /src/java/main/org/apache/zookeeper/KeeperException.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooDefs.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooKeeper.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooKeeperMain.java 1415037 
>   /src/java/main/org/apache/zookeeper/cli/GetConfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/cli/ReconfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/DataTree.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/Request.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Follower.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Leader.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Observer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumBean.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumHierarchical.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumMaj.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumVerifier.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/util/ConfigUtils.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/TruncateCorruptionTest.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/quorum/ReconfigRecoveryTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/CnxManagerTest.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/QuorumMajorityTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/ReconfigTest.java PRE-CREATION 
>   /src/zookeeper.jute 1415037 
> 
> Diff: https://reviews.apache.org/r/6707/diff/
> 
> 
> Testing
> -------
> 
> New test files were added: ReconfigTest, ReconfigRecoveryTest and QuorumMajorityTest. Many other tests were modified. 
> 
> 
> Thanks,
> 
> Alexander Shraer
> 
>


Re: Review Request: Dynamic reconfiguration, see https://issues.apache.org/jira/browse/ZOOKEEPER-107

Posted by Edward Ribeiro <ed...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6707/#review15608
-----------------------------------------------------------



/src/java/main/org/apache/zookeeper/ZooKeeper.java
<https://reviews.apache.org/r/6707/#comment33706>

    It would be cool if we could extract this method to a helper class called  StringUtils or something like that. This helper class could also have a method to handle a more judiciously parsing of the comma separated host (trimming and removing empty values).


- Edward Ribeiro


On Nov. 29, 2012, 7:12 a.m., Alexander Shraer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6707/
> -----------------------------------------------------------
> 
> (Updated Nov. 29, 2012, 7:12 a.m.)
> 
> 
> Review request for zookeeper.
> 
> 
> Description
> -------
> 
> see https://issues.apache.org/jira/browse/ZOOKEEPER-107
> 
> 
> Diffs
> -----
> 
>   /src/c/include/proto.h 1415037 
>   /src/c/include/zookeeper.h 1415037 
>   /src/c/src/cli.c 1415037 
>   /src/c/src/zookeeper.c 1415037 
>   /src/java/main/org/apache/zookeeper/KeeperException.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooDefs.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooKeeper.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooKeeperMain.java 1415037 
>   /src/java/main/org/apache/zookeeper/cli/GetConfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/cli/ReconfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/DataTree.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/Request.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Follower.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Leader.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Observer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumBean.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumHierarchical.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumMaj.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumVerifier.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/util/ConfigUtils.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/TruncateCorruptionTest.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/quorum/ReconfigRecoveryTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/CnxManagerTest.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/QuorumMajorityTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/ReconfigTest.java PRE-CREATION 
>   /src/zookeeper.jute 1415037 
> 
> Diff: https://reviews.apache.org/r/6707/diff/
> 
> 
> Testing
> -------
> 
> New test files were added: ReconfigTest, ReconfigRecoveryTest and QuorumMajorityTest. Many other tests were modified. 
> 
> 
> Thanks,
> 
> Alexander Shraer
> 
>


Re: Review Request: Dynamic reconfiguration, see https://issues.apache.org/jira/browse/ZOOKEEPER-107

Posted by Alexander Shraer <sh...@yahoo-inc.com>.

> On Jan. 23, 2013, 7:08 a.m., Edward Ribeiro wrote:
> > /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java, line 450
> > <https://reviews.apache.org/r/6707/diff/4/?file=231478#file231478line450>
> >
> >     What if the newMembers is "host1, host2, host3" (i.e., with spaces after comma) or "host1, ,host2" ('empty' entry). I think it would be cool if we could trim and remove spurious entries through a helper class (StringUtils). Please look at ZOOKEEPER-1619 for my take on this issue and say what you think.
> >     
> >     I have found that many places in the code base on both client and server side that use the default String.split(), but it lacks trimming and removing of empty values. I propose that we start addressing this issue in this patch and eventually open a JIRA to address the remaining cases. A user has opened a issue recently to address client-side (ZOOKEEPER-1619), but as far as I looked for, it affects the server side too. :(

in this case, the commas are replaced by newlines, then Properties.load should take care of empty lines and spaces.


- Alexander


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6707/#review15610
-----------------------------------------------------------


On Nov. 29, 2012, 7:12 a.m., Alexander Shraer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6707/
> -----------------------------------------------------------
> 
> (Updated Nov. 29, 2012, 7:12 a.m.)
> 
> 
> Review request for zookeeper.
> 
> 
> Description
> -------
> 
> see https://issues.apache.org/jira/browse/ZOOKEEPER-107
> 
> 
> Diffs
> -----
> 
>   /src/c/include/proto.h 1415037 
>   /src/c/include/zookeeper.h 1415037 
>   /src/c/src/cli.c 1415037 
>   /src/c/src/zookeeper.c 1415037 
>   /src/java/main/org/apache/zookeeper/KeeperException.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooDefs.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooKeeper.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooKeeperMain.java 1415037 
>   /src/java/main/org/apache/zookeeper/cli/GetConfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/cli/ReconfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/DataTree.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/Request.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Follower.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Leader.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Observer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumBean.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumHierarchical.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumMaj.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumVerifier.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/util/ConfigUtils.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/TruncateCorruptionTest.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/quorum/ReconfigRecoveryTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/CnxManagerTest.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/QuorumMajorityTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/ReconfigTest.java PRE-CREATION 
>   /src/zookeeper.jute 1415037 
> 
> Diff: https://reviews.apache.org/r/6707/diff/
> 
> 
> Testing
> -------
> 
> New test files were added: ReconfigTest, ReconfigRecoveryTest and QuorumMajorityTest. Many other tests were modified. 
> 
> 
> Thanks,
> 
> Alexander Shraer
> 
>


Re: Review Request: Dynamic reconfiguration, see https://issues.apache.org/jira/browse/ZOOKEEPER-107

Posted by Edward Ribeiro <ed...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6707/#review15610
-----------------------------------------------------------



/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
<https://reviews.apache.org/r/6707/#comment33708>

    What if the newMembers is "host1, host2, host3" (i.e., with spaces after comma) or "host1, ,host2" ('empty' entry). I think it would be cool if we could trim and remove spurious entries through a helper class (StringUtils). Please look at ZOOKEEPER-1619 for my take on this issue and say what you think.
    
    I have found that many places in the code base on both client and server side that use the default String.split(), but it lacks trimming and removing of empty values. I propose that we start addressing this issue in this patch and eventually open a JIRA to address the remaining cases. A user has opened a issue recently to address client-side (ZOOKEEPER-1619), but as far as I looked for, it affects the server side too. :(


- Edward Ribeiro


On Nov. 29, 2012, 7:12 a.m., Alexander Shraer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6707/
> -----------------------------------------------------------
> 
> (Updated Nov. 29, 2012, 7:12 a.m.)
> 
> 
> Review request for zookeeper.
> 
> 
> Description
> -------
> 
> see https://issues.apache.org/jira/browse/ZOOKEEPER-107
> 
> 
> Diffs
> -----
> 
>   /src/c/include/proto.h 1415037 
>   /src/c/include/zookeeper.h 1415037 
>   /src/c/src/cli.c 1415037 
>   /src/c/src/zookeeper.c 1415037 
>   /src/java/main/org/apache/zookeeper/KeeperException.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooDefs.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooKeeper.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooKeeperMain.java 1415037 
>   /src/java/main/org/apache/zookeeper/cli/GetConfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/cli/ReconfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/DataTree.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/Request.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Follower.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Leader.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Observer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumBean.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumHierarchical.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumMaj.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumVerifier.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/util/ConfigUtils.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/TruncateCorruptionTest.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/quorum/ReconfigRecoveryTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/CnxManagerTest.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/QuorumMajorityTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/ReconfigTest.java PRE-CREATION 
>   /src/zookeeper.jute 1415037 
> 
> Diff: https://reviews.apache.org/r/6707/diff/
> 
> 
> Testing
> -------
> 
> New test files were added: ReconfigTest, ReconfigRecoveryTest and QuorumMajorityTest. Many other tests were modified. 
> 
> 
> Thanks,
> 
> Alexander Shraer
> 
>


Re: Review Request: Dynamic reconfiguration, see https://issues.apache.org/jira/browse/ZOOKEEPER-107

Posted by Alexander Shraer <sh...@yahoo-inc.com>.

> On Jan. 23, 2013, 7:20 a.m., Edward Ribeiro wrote:
> > /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java, line 467
> > <https://reviews.apache.org/r/6707/diff/4/?file=231478#file231478line467>
> >
> >     Lines 467-479 contains duplicate code. I suggest to create a method split (or parse) method like:
> >     
> >     List<String> split(String input) {
> >     
> >        if (input != null) {
> >     
> >          List<String> list = Arrays.asList(input.split(","));
> >     
> >          return Collections.unmodifiableList(list);
> >        }
> >     
> >        return Collections.emptyList();
> >     }
> >     
> >     Then lines 467 and 474 can be simplified as:
> >     
> >     List<String> joiningServers = split(reconfigRequest.getJoiningServers());
> >     
> >     List<String> leavingServers = split(reconfigRequest.getLeavingServers());
> >     
> >     PS: I think this split method should go in a helper class as stated before.

need to additionally check that reconfigRequest.getJoiningServers() or  reconfigRequest.getLeavingServers() return a non-null value.


- Alexander


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6707/#review15611
-----------------------------------------------------------


On Nov. 29, 2012, 7:12 a.m., Alexander Shraer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6707/
> -----------------------------------------------------------
> 
> (Updated Nov. 29, 2012, 7:12 a.m.)
> 
> 
> Review request for zookeeper.
> 
> 
> Description
> -------
> 
> see https://issues.apache.org/jira/browse/ZOOKEEPER-107
> 
> 
> Diffs
> -----
> 
>   /src/c/include/proto.h 1415037 
>   /src/c/include/zookeeper.h 1415037 
>   /src/c/src/cli.c 1415037 
>   /src/c/src/zookeeper.c 1415037 
>   /src/java/main/org/apache/zookeeper/KeeperException.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooDefs.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooKeeper.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooKeeperMain.java 1415037 
>   /src/java/main/org/apache/zookeeper/cli/GetConfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/cli/ReconfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/DataTree.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/Request.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Follower.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Leader.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Observer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumBean.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumHierarchical.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumMaj.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumVerifier.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/util/ConfigUtils.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/TruncateCorruptionTest.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/quorum/ReconfigRecoveryTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/CnxManagerTest.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/QuorumMajorityTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/ReconfigTest.java PRE-CREATION 
>   /src/zookeeper.jute 1415037 
> 
> Diff: https://reviews.apache.org/r/6707/diff/
> 
> 
> Testing
> -------
> 
> New test files were added: ReconfigTest, ReconfigRecoveryTest and QuorumMajorityTest. Many other tests were modified. 
> 
> 
> Thanks,
> 
> Alexander Shraer
> 
>


Re: Review Request: Dynamic reconfiguration, see https://issues.apache.org/jira/browse/ZOOKEEPER-107

Posted by Edward Ribeiro <ed...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6707/#review15611
-----------------------------------------------------------



/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
<https://reviews.apache.org/r/6707/#comment33709>

    Lines 467-479 contains duplicate code. I suggest to create a method split (or parse) method like:
    
    List<String> split(String input) {
    
       if (input != null) {
    
         List<String> list = Arrays.asList(input.split(","));
    
         return Collections.unmodifiableList(list);
       }
    
       return Collections.emptyList();
    }
    
    Then lines 467 and 474 can be simplified as:
    
    List<String> joiningServers = split(reconfigRequest.getJoiningServers());
    
    List<String> leavingServers = split(reconfigRequest.getLeavingServers());
    
    PS: I think this split method should go in a helper class as stated before.


- Edward Ribeiro


On Nov. 29, 2012, 7:12 a.m., Alexander Shraer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6707/
> -----------------------------------------------------------
> 
> (Updated Nov. 29, 2012, 7:12 a.m.)
> 
> 
> Review request for zookeeper.
> 
> 
> Description
> -------
> 
> see https://issues.apache.org/jira/browse/ZOOKEEPER-107
> 
> 
> Diffs
> -----
> 
>   /src/c/include/proto.h 1415037 
>   /src/c/include/zookeeper.h 1415037 
>   /src/c/src/cli.c 1415037 
>   /src/c/src/zookeeper.c 1415037 
>   /src/java/main/org/apache/zookeeper/KeeperException.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooDefs.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooKeeper.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooKeeperMain.java 1415037 
>   /src/java/main/org/apache/zookeeper/cli/GetConfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/cli/ReconfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/DataTree.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/Request.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Follower.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Leader.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Observer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumBean.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumHierarchical.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumMaj.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumVerifier.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/util/ConfigUtils.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/TruncateCorruptionTest.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/quorum/ReconfigRecoveryTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/CnxManagerTest.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/QuorumMajorityTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/ReconfigTest.java PRE-CREATION 
>   /src/zookeeper.jute 1415037 
> 
> Diff: https://reviews.apache.org/r/6707/diff/
> 
> 
> Testing
> -------
> 
> New test files were added: ReconfigTest, ReconfigRecoveryTest and QuorumMajorityTest. Many other tests were modified. 
> 
> 
> Thanks,
> 
> Alexander Shraer
> 
>


Re: Review Request: Dynamic reconfiguration, see https://issues.apache.org/jira/browse/ZOOKEEPER-107

Posted by Edward Ribeiro <ed...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6707/#review18298
-----------------------------------------------------------



/src/java/main/org/apache/zookeeper/server/util/ConfigUtils.java
<https://reviews.apache.org/r/6707/#comment38498>

    I'd be cool if you remove the white spaces (in red) at this line, as well as, l#46 and l#50


- Edward Ribeiro


On Feb. 6, 2013, 1 a.m., Alexander Shraer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6707/
> -----------------------------------------------------------
> 
> (Updated Feb. 6, 2013, 1 a.m.)
> 
> 
> Review request for zookeeper.
> 
> 
> Description
> -------
> 
> see https://issues.apache.org/jira/browse/ZOOKEEPER-107
> 
> 
> Diffs
> -----
> 
>   /src/c/include/proto.h 1442038 
>   /src/c/include/zookeeper.h 1442038 
>   /src/c/src/cli.c 1442038 
>   /src/c/src/zookeeper.c 1442038 
>   /src/c/tests/test-reconfig.py PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/KeeperException.java 1442038 
>   /src/java/main/org/apache/zookeeper/ZooDefs.java 1442038 
>   /src/java/main/org/apache/zookeeper/ZooKeeper.java 1442038 
>   /src/java/main/org/apache/zookeeper/ZooKeeperMain.java 1442038 
>   /src/java/main/org/apache/zookeeper/cli/GetConfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/cli/ReconfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/common/StringUtils.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/DataTree.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/Request.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/ZKDatabase.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/quorum/Follower.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/quorum/Leader.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/quorum/Observer.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumBean.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumHierarchical.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumMaj.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumVerifier.java 1442038 
>   /src/java/main/org/apache/zookeeper/server/util/ConfigUtils.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1442038 
>   /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 1442038 
>   /src/java/test/org/apache/zookeeper/server/quorum/ReconfigRecoveryTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 1442038 
>   /src/java/test/org/apache/zookeeper/test/CnxManagerTest.java 1442038 
>   /src/java/test/org/apache/zookeeper/test/NIOConnectionFactoryFdLeakTest.java 1442038 
>   /src/java/test/org/apache/zookeeper/test/QuorumMajorityTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1442038 
>   /src/java/test/org/apache/zookeeper/test/ReconfigTest.java PRE-CREATION 
>   /src/zookeeper.jute 1442038 
> 
> Diff: https://reviews.apache.org/r/6707/diff/
> 
> 
> Testing
> -------
> 
> New test files were added: ReconfigTest, ReconfigRecoveryTest and QuorumMajorityTest. Many other tests were modified. 
> 
> 
> Thanks,
> 
> Alexander Shraer
> 
>


Re: Review Request: Dynamic reconfiguration, see https://issues.apache.org/jira/browse/ZOOKEEPER-107

Posted by Alexander Shraer <sh...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6707/
-----------------------------------------------------------

(Updated Feb. 6, 2013, 1 a.m.)


Review request for zookeeper.


Changes
-------

there were some files missing in last patch.


Description
-------

see https://issues.apache.org/jira/browse/ZOOKEEPER-107


Diffs (updated)
-----

  /src/c/include/proto.h 1442038 
  /src/c/include/zookeeper.h 1442038 
  /src/c/src/cli.c 1442038 
  /src/c/src/zookeeper.c 1442038 
  /src/c/tests/test-reconfig.py PRE-CREATION 
  /src/java/main/org/apache/zookeeper/KeeperException.java 1442038 
  /src/java/main/org/apache/zookeeper/ZooDefs.java 1442038 
  /src/java/main/org/apache/zookeeper/ZooKeeper.java 1442038 
  /src/java/main/org/apache/zookeeper/ZooKeeperMain.java 1442038 
  /src/java/main/org/apache/zookeeper/cli/GetConfigCommand.java PRE-CREATION 
  /src/java/main/org/apache/zookeeper/cli/ReconfigCommand.java PRE-CREATION 
  /src/java/main/org/apache/zookeeper/common/StringUtils.java 1442038 
  /src/java/main/org/apache/zookeeper/server/DataTree.java 1442038 
  /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1442038 
  /src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 1442038 
  /src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 1442038 
  /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1442038 
  /src/java/main/org/apache/zookeeper/server/Request.java 1442038 
  /src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 1442038 
  /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1442038 
  /src/java/main/org/apache/zookeeper/server/ZKDatabase.java 1442038 
  /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1442038 
  /src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 1442038 
  /src/java/main/org/apache/zookeeper/server/quorum/Follower.java 1442038 
  /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1442038 
  /src/java/main/org/apache/zookeeper/server/quorum/Leader.java 1442038 
  /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1442038 
  /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1442038 
  /src/java/main/org/apache/zookeeper/server/quorum/Observer.java 1442038 
  /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1442038 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumBean.java 1442038 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 1442038 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1442038 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1442038 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1442038 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1442038 
  /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1442038 
  /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumHierarchical.java 1442038 
  /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumMaj.java 1442038 
  /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumVerifier.java 1442038 
  /src/java/main/org/apache/zookeeper/server/util/ConfigUtils.java PRE-CREATION 
  /src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1442038 
  /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 1442038 
  /src/java/test/org/apache/zookeeper/server/quorum/ReconfigRecoveryTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 1442038 
  /src/java/test/org/apache/zookeeper/test/CnxManagerTest.java 1442038 
  /src/java/test/org/apache/zookeeper/test/NIOConnectionFactoryFdLeakTest.java 1442038 
  /src/java/test/org/apache/zookeeper/test/QuorumMajorityTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1442038 
  /src/java/test/org/apache/zookeeper/test/ReconfigTest.java PRE-CREATION 
  /src/zookeeper.jute 1442038 

Diff: https://reviews.apache.org/r/6707/diff/


Testing
-------

New test files were added: ReconfigTest, ReconfigRecoveryTest and QuorumMajorityTest. Many other tests were modified. 


Thanks,

Alexander Shraer


Re: Review Request: Dynamic reconfiguration, see https://issues.apache.org/jira/browse/ZOOKEEPER-107

Posted by Alexander Shraer <sh...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6707/
-----------------------------------------------------------

(Updated Feb. 5, 2013, 5:07 a.m.)


Review request for zookeeper.


Changes
-------

Includes Michi's C client tests, fixed backward compatibility issues (backward compatibility will work from 3.4.6 assuming ZOOKEEPER-1633 goes in).
Addressed Flavio's comments. 


Description
-------

see https://issues.apache.org/jira/browse/ZOOKEEPER-107


Diffs (updated)
-----

  /src/c/include/proto.h 1442472 
  /src/c/include/zookeeper.h 1442472 
  /src/c/src/cli.c 1442472 
  /src/c/src/zookeeper.c 1442472 
  /src/java/main/org/apache/zookeeper/KeeperException.java 1442472 
  /src/java/main/org/apache/zookeeper/ZooDefs.java 1442472 
  /src/java/main/org/apache/zookeeper/ZooKeeper.java 1442472 
  /src/java/main/org/apache/zookeeper/ZooKeeperMain.java 1442472 
  /src/java/main/org/apache/zookeeper/common/StringUtils.java 1442472 
  /src/java/main/org/apache/zookeeper/server/DataTree.java 1442472 
  /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1442472 
  /src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 1442472 
  /src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 1442472 
  /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1442472 
  /src/java/main/org/apache/zookeeper/server/Request.java 1442472 
  /src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 1442472 
  /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1442472 
  /src/java/main/org/apache/zookeeper/server/ZKDatabase.java 1442472 
  /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1442472 
  /src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 1442472 
  /src/java/main/org/apache/zookeeper/server/quorum/Follower.java 1442472 
  /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1442472 
  /src/java/main/org/apache/zookeeper/server/quorum/Leader.java 1442472 
  /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1442472 
  /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1442472 
  /src/java/main/org/apache/zookeeper/server/quorum/Observer.java 1442472 
  /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1442472 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumBean.java 1442472 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 1442472 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1442472 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1442472 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1442472 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1442472 
  /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1442472 
  /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumHierarchical.java 1442472 
  /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumMaj.java 1442472 
  /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumVerifier.java 1442472 
  /src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1442472 
  /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 1442472 
  /src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 1442472 
  /src/java/test/org/apache/zookeeper/test/CnxManagerTest.java 1442472 
  /src/java/test/org/apache/zookeeper/test/NIOConnectionFactoryFdLeakTest.java 1442472 
  /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1442472 
  /src/zookeeper.jute 1442472 

Diff: https://reviews.apache.org/r/6707/diff/


Testing
-------

New test files were added: ReconfigTest, ReconfigRecoveryTest and QuorumMajorityTest. Many other tests were modified. 


Thanks,

Alexander Shraer


Re: Review Request: Dynamic reconfiguration, see https://issues.apache.org/jira/browse/ZOOKEEPER-107

Posted by Alexander Shraer <sh...@yahoo-inc.com>.

> On Jan. 31, 2013, 12:58 a.m., fpj wrote:
> > /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java, line 243
> > <https://reviews.apache.org/r/6707/diff/5/?file=251937#file251937line243>
> >
> >     I guess that the problem of having the quorum verifier info into stat is that we would have to ship it with every response? I was really looking for a way of removing the reference to QuorumZooKeeperServer.

Just in responses for getData and reconfig, where you probably want to know anyway what happened. For example in incremental reconfig you just say the delta but don't know the final state.


> On Jan. 31, 2013, 12:58 a.m., fpj wrote:
> > /src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java, line 267
> > <https://reviews.apache.org/r/6707/diff/5/?file=251945#file251945line267>
> >
> >     Could you explain me why you're shutting down leader election here? How can we get into this scenario?

This is when during leader election you discover a server from a future configuration. For example the new configuration were activated while you were down. In this case you may be not talking to the right set of servers. We update the set of servers and restart leader election so that you connect to the right peers. The configuration info is now sent in all FLE notification messages for this purpose.

In general, in several occasions (upon every "serious" change in configuration) I restart leader election. In many places this can be avoided, for example by connecting to new peers, or modifying existing connections. So restarting is often done for simplicity to reuse existing code.


- Alexander


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6707/#review15907
-----------------------------------------------------------


On Jan. 25, 2013, 7:59 a.m., Alexander Shraer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6707/
> -----------------------------------------------------------
> 
> (Updated Jan. 25, 2013, 7:59 a.m.)
> 
> 
> Review request for zookeeper.
> 
> 
> Description
> -------
> 
> see https://issues.apache.org/jira/browse/ZOOKEEPER-107
> 
> 
> Diffs
> -----
> 
>   /src/c/include/proto.h 1438352 
>   /src/c/include/zookeeper.h 1438352 
>   /src/c/src/cli.c 1438352 
>   /src/c/src/zookeeper.c 1438352 
>   /src/java/main/org/apache/zookeeper/KeeperException.java 1438352 
>   /src/java/main/org/apache/zookeeper/ZooDefs.java 1438352 
>   /src/java/main/org/apache/zookeeper/ZooKeeper.java 1438352 
>   /src/java/main/org/apache/zookeeper/ZooKeeperMain.java 1438352 
>   /src/java/main/org/apache/zookeeper/cli/GetConfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/cli/ReconfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/common/StringUtils.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/DataTree.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 1438348 
>   /src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/Request.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/Follower.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/Leader.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/Observer.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumBean.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumHierarchical.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumMaj.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumVerifier.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/util/ConfigUtils.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1438352 
>   /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 1438352 
>   /src/java/test/org/apache/zookeeper/server/quorum/ReconfigRecoveryTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 1438352 
>   /src/java/test/org/apache/zookeeper/test/CnxManagerTest.java 1438352 
>   /src/java/test/org/apache/zookeeper/test/NIOConnectionFactoryFdLeakTest.java 1438352 
>   /src/java/test/org/apache/zookeeper/test/QuorumMajorityTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1438352 
>   /src/java/test/org/apache/zookeeper/test/ReconfigTest.java PRE-CREATION 
>   /src/zookeeper.jute 1438352 
> 
> Diff: https://reviews.apache.org/r/6707/diff/
> 
> 
> Testing
> -------
> 
> New test files were added: ReconfigTest, ReconfigRecoveryTest and QuorumMajorityTest. Many other tests were modified. 
> 
> 
> Thanks,
> 
> Alexander Shraer
> 
>


Re: Review Request: Dynamic reconfiguration, see https://issues.apache.org/jira/browse/ZOOKEEPER-107

Posted by fp...@apache.org.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6707/#review15907
-----------------------------------------------------------


Hey Alex, I have a few minor points that would be cool if you could fix for the next version of the patch. They are really minor. One extra comment is that there is a lot of red in your patch (trailing and leading spaces, perhaps tabs too). The trunk code already has some, so it would be great not to introduce more. Please try to remove as much as possible. Otherwise, great job! 


/src/java/main/org/apache/zookeeper/cli/ReconfigCommand.java
<https://reviews.apache.org/r/6707/#comment34182>

    you may want to break this line up, it seems too long.



/src/java/main/org/apache/zookeeper/cli/ReconfigCommand.java
<https://reviews.apache.org/r/6707/#comment34183>

    another long one.



/src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java
<https://reviews.apache.org/r/6707/#comment34184>

    I guess that the problem of having the quorum verifier info into stat is that we would have to ship it with every response? I was really looking for a way of removing the reference to QuorumZooKeeperServer.



/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
<https://reviews.apache.org/r/6707/#comment34186>

    Is the comment supposed to be an example? Can we have it on top of the statement and a more precise statement of what it is about?



/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
<https://reviews.apache.org/r/6707/#comment34187>

    Same here.



/src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java
<https://reviews.apache.org/r/6707/#comment34194>

    Could you explain me why you're shutting down leader election here? How can we get into this scenario?



/src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java
<https://reviews.apache.org/r/6707/#comment34192>

    If we are supposer to stop the peer, then this should be a fatal log message, don't simply dump the stack trace, use the log facility.



/src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java
<https://reviews.apache.org/r/6707/#comment34189>

    Can we fix the format of this comment?



/src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java
<https://reviews.apache.org/r/6707/#comment34190>

    Thanks for fixing this!



/src/java/main/org/apache/zookeeper/server/util/ConfigUtils.java
<https://reviews.apache.org/r/6707/#comment34180>

    The formatting for this method looks broken.


- fpj


On Jan. 25, 2013, 7:59 a.m., Alexander Shraer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6707/
> -----------------------------------------------------------
> 
> (Updated Jan. 25, 2013, 7:59 a.m.)
> 
> 
> Review request for zookeeper.
> 
> 
> Description
> -------
> 
> see https://issues.apache.org/jira/browse/ZOOKEEPER-107
> 
> 
> Diffs
> -----
> 
>   /src/c/include/proto.h 1438352 
>   /src/c/include/zookeeper.h 1438352 
>   /src/c/src/cli.c 1438352 
>   /src/c/src/zookeeper.c 1438352 
>   /src/java/main/org/apache/zookeeper/KeeperException.java 1438352 
>   /src/java/main/org/apache/zookeeper/ZooDefs.java 1438352 
>   /src/java/main/org/apache/zookeeper/ZooKeeper.java 1438352 
>   /src/java/main/org/apache/zookeeper/ZooKeeperMain.java 1438352 
>   /src/java/main/org/apache/zookeeper/cli/GetConfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/cli/ReconfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/common/StringUtils.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/DataTree.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 1438348 
>   /src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/Request.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/Follower.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/Leader.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/Observer.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumBean.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumHierarchical.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumMaj.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumVerifier.java 1438352 
>   /src/java/main/org/apache/zookeeper/server/util/ConfigUtils.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1438352 
>   /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 1438352 
>   /src/java/test/org/apache/zookeeper/server/quorum/ReconfigRecoveryTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 1438352 
>   /src/java/test/org/apache/zookeeper/test/CnxManagerTest.java 1438352 
>   /src/java/test/org/apache/zookeeper/test/NIOConnectionFactoryFdLeakTest.java 1438352 
>   /src/java/test/org/apache/zookeeper/test/QuorumMajorityTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1438352 
>   /src/java/test/org/apache/zookeeper/test/ReconfigTest.java PRE-CREATION 
>   /src/zookeeper.jute 1438352 
> 
> Diff: https://reviews.apache.org/r/6707/diff/
> 
> 
> Testing
> -------
> 
> New test files were added: ReconfigTest, ReconfigRecoveryTest and QuorumMajorityTest. Many other tests were modified. 
> 
> 
> Thanks,
> 
> Alexander Shraer
> 
>


Re: Review Request: Dynamic reconfiguration, see https://issues.apache.org/jira/browse/ZOOKEEPER-107

Posted by Alexander Shraer <sh...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6707/
-----------------------------------------------------------

(Updated Jan. 25, 2013, 7:59 a.m.)


Review request for zookeeper.


Description
-------

see https://issues.apache.org/jira/browse/ZOOKEEPER-107


Diffs (updated)
-----

  /src/c/include/proto.h 1438352 
  /src/c/include/zookeeper.h 1438352 
  /src/c/src/cli.c 1438352 
  /src/c/src/zookeeper.c 1438352 
  /src/java/main/org/apache/zookeeper/KeeperException.java 1438352 
  /src/java/main/org/apache/zookeeper/ZooDefs.java 1438352 
  /src/java/main/org/apache/zookeeper/ZooKeeper.java 1438352 
  /src/java/main/org/apache/zookeeper/ZooKeeperMain.java 1438352 
  /src/java/main/org/apache/zookeeper/cli/GetConfigCommand.java PRE-CREATION 
  /src/java/main/org/apache/zookeeper/cli/ReconfigCommand.java PRE-CREATION 
  /src/java/main/org/apache/zookeeper/common/StringUtils.java 1438352 
  /src/java/main/org/apache/zookeeper/server/DataTree.java 1438352 
  /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1438352 
  /src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 1438348 
  /src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 1438352 
  /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1438352 
  /src/java/main/org/apache/zookeeper/server/Request.java 1438352 
  /src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 1438352 
  /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1438352 
  /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1438352 
  /src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 1438352 
  /src/java/main/org/apache/zookeeper/server/quorum/Follower.java 1438352 
  /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1438352 
  /src/java/main/org/apache/zookeeper/server/quorum/Leader.java 1438352 
  /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1438352 
  /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1438352 
  /src/java/main/org/apache/zookeeper/server/quorum/Observer.java 1438352 
  /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1438352 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumBean.java 1438352 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 1438352 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1438352 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1438352 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1438352 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1438352 
  /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1438352 
  /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumHierarchical.java 1438352 
  /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumMaj.java 1438352 
  /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumVerifier.java 1438352 
  /src/java/main/org/apache/zookeeper/server/util/ConfigUtils.java PRE-CREATION 
  /src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1438352 
  /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 1438352 
  /src/java/test/org/apache/zookeeper/server/quorum/ReconfigRecoveryTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 1438352 
  /src/java/test/org/apache/zookeeper/test/CnxManagerTest.java 1438352 
  /src/java/test/org/apache/zookeeper/test/NIOConnectionFactoryFdLeakTest.java 1438352 
  /src/java/test/org/apache/zookeeper/test/QuorumMajorityTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1438352 
  /src/java/test/org/apache/zookeeper/test/ReconfigTest.java PRE-CREATION 
  /src/zookeeper.jute 1438352 

Diff: https://reviews.apache.org/r/6707/diff/


Testing
-------

New test files were added: ReconfigTest, ReconfigRecoveryTest and QuorumMajorityTest. Many other tests were modified. 


Thanks,

Alexander Shraer


Re: Review Request: Dynamic reconfiguration, see https://issues.apache.org/jira/browse/ZOOKEEPER-107

Posted by Edward Ribeiro <ed...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6707/#review15613
-----------------------------------------------------------



/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
<https://reviews.apache.org/r/6707/#comment33711>

    Read comment on line 492.


- Edward Ribeiro


On Nov. 29, 2012, 7:12 a.m., Alexander Shraer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6707/
> -----------------------------------------------------------
> 
> (Updated Nov. 29, 2012, 7:12 a.m.)
> 
> 
> Review request for zookeeper.
> 
> 
> Description
> -------
> 
> see https://issues.apache.org/jira/browse/ZOOKEEPER-107
> 
> 
> Diffs
> -----
> 
>   /src/c/include/proto.h 1415037 
>   /src/c/include/zookeeper.h 1415037 
>   /src/c/src/cli.c 1415037 
>   /src/c/src/zookeeper.c 1415037 
>   /src/java/main/org/apache/zookeeper/KeeperException.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooDefs.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooKeeper.java 1415037 
>   /src/java/main/org/apache/zookeeper/ZooKeeperMain.java 1415037 
>   /src/java/main/org/apache/zookeeper/cli/GetConfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/cli/ReconfigCommand.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/DataTree.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/Request.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Follower.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Leader.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/Observer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumBean.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumHierarchical.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumMaj.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumVerifier.java 1415037 
>   /src/java/main/org/apache/zookeeper/server/util/ConfigUtils.java PRE-CREATION 
>   /src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/TruncateCorruptionTest.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 1415037 
>   /src/java/test/org/apache/zookeeper/server/quorum/ReconfigRecoveryTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/CnxManagerTest.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/QuorumMajorityTest.java PRE-CREATION 
>   /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1415037 
>   /src/java/test/org/apache/zookeeper/test/ReconfigTest.java PRE-CREATION 
>   /src/zookeeper.jute 1415037 
> 
> Diff: https://reviews.apache.org/r/6707/diff/
> 
> 
> Testing
> -------
> 
> New test files were added: ReconfigTest, ReconfigRecoveryTest and QuorumMajorityTest. Many other tests were modified. 
> 
> 
> Thanks,
> 
> Alexander Shraer
> 
>


Re: Review Request: Dynamic reconfiguration, see https://issues.apache.org/jira/browse/ZOOKEEPER-107

Posted by Alexander Shraer <sh...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6707/
-----------------------------------------------------------

(Updated Nov. 29, 2012, 7:12 a.m.)


Review request for zookeeper.


Changes
-------

Latest patch


Description
-------

see https://issues.apache.org/jira/browse/ZOOKEEPER-107


Diffs (updated)
-----

  /src/c/include/proto.h 1415037 
  /src/c/include/zookeeper.h 1415037 
  /src/c/src/cli.c 1415037 
  /src/c/src/zookeeper.c 1415037 
  /src/java/main/org/apache/zookeeper/KeeperException.java 1415037 
  /src/java/main/org/apache/zookeeper/ZooDefs.java 1415037 
  /src/java/main/org/apache/zookeeper/ZooKeeper.java 1415037 
  /src/java/main/org/apache/zookeeper/ZooKeeperMain.java 1415037 
  /src/java/main/org/apache/zookeeper/cli/GetConfigCommand.java PRE-CREATION 
  /src/java/main/org/apache/zookeeper/cli/ReconfigCommand.java PRE-CREATION 
  /src/java/main/org/apache/zookeeper/server/DataTree.java 1415037 
  /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 1415037 
  /src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java 1415037 
  /src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 1415037 
  /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 1415037 
  /src/java/main/org/apache/zookeeper/server/Request.java 1415037 
  /src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 1415037 
  /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1415037 
  /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java 1415037 
  /src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 1415037 
  /src/java/main/org/apache/zookeeper/server/quorum/Follower.java 1415037 
  /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java 1415037 
  /src/java/main/org/apache/zookeeper/server/quorum/Leader.java 1415037 
  /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1415037 
  /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 1415037 
  /src/java/main/org/apache/zookeeper/server/quorum/Observer.java 1415037 
  /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java 1415037 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumBean.java 1415037 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 1415037 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1415037 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 1415037 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 1415037 
  /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java 1415037 
  /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java 1415037 
  /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumHierarchical.java 1415037 
  /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumMaj.java 1415037 
  /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumVerifier.java 1415037 
  /src/java/main/org/apache/zookeeper/server/util/ConfigUtils.java PRE-CREATION 
  /src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1415037 
  /src/java/test/org/apache/zookeeper/server/TruncateCorruptionTest.java 1415037 
  /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 1415037 
  /src/java/test/org/apache/zookeeper/server/quorum/ReconfigRecoveryTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 1415037 
  /src/java/test/org/apache/zookeeper/test/CnxManagerTest.java 1415037 
  /src/java/test/org/apache/zookeeper/test/QuorumMajorityTest.java PRE-CREATION 
  /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1415037 
  /src/java/test/org/apache/zookeeper/test/ReconfigTest.java PRE-CREATION 
  /src/zookeeper.jute 1415037 

Diff: https://reviews.apache.org/r/6707/diff/


Testing
-------

New test files were added: ReconfigTest, ReconfigRecoveryTest and QuorumMajorityTest. Many other tests were modified. 


Thanks,

Alexander Shraer