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




src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml (line 1134)
<https://reviews.apache.org/r/51546/#comment221657>

    is there a way for us to sanity check this (make sure that their individual 
reconfigEnabled matches that of the leader) when servers join the quorum?



src/java/main/org/apache/zookeeper/KeeperException.java 
<https://reviews.apache.org/r/51546/#comment221640>

    whitespace change?



src/java/main/org/apache/zookeeper/cli/CliCommand.java (line 23)
<https://reviews.apache.org/r/51546/#comment221636>

    unused import?



src/java/main/org/apache/zookeeper/cli/CliCommand.java (line 65)
<https://reviews.apache.org/r/51546/#comment221637>

    can this file be reverted instead of being part of this patch?



src/java/main/org/apache/zookeeper/cli/ReconfigCommand.java (line 148)
<https://reviews.apache.org/r/51546/#comment221651>

    This would need to be replicated on future commands that live in 
ZooKeeperAdmin.
    
    Perhaps it would be useful to subclass CliCommand with CliAdminCommand for 
commands that need ZooKeeperAdmin.



src/java/main/org/apache/zookeeper/server/DataTree.java (line 259)
<https://reviews.apache.org/r/51546/#comment221639>

    if setting the acl throws an exception would the server continue starting 
with incorrect permissioning on the config znode?



src/java/test/org/apache/zookeeper/server/DataTreeTest.java (line 201)
<https://reviews.apache.org/r/51546/#comment221643>

    not sure how this change relates to the purpose of the patch



src/java/test/org/apache/zookeeper/server/quorum/ReconfigBackupTest.java (line 
66)
<https://reviews.apache.org/r/51546/#comment221650>

    is there some sort of method we could call to generate the digest instead 
of inserting these as magic strings? it may make it easier to tell what is 
going on?



src/java/test/org/apache/zookeeper/test/ReconfigExceptionTest.java (line 201)
<https://reviews.apache.org/r/51546/#comment221652>

    this method is shared with ReconfigMisconfigTest so perhaps it could be 
moved to ZkTestCase?


- Abraham Fine


On Oct. 11, 2016, 10:33 p.m., Michael Han wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51546/
> -----------------------------------------------------------
> 
> (Updated Oct. 11, 2016, 10:33 p.m.)
> 
> 
> Review request for zookeeper, fpj, Patrick Hunt, Raul Gutierrez Segales, and 
> Alexander Shraer.
> 
> 
> Bugs: ZOOKEEPER-2014
>     https://issues.apache.org/jira/browse/ZOOKEEPER-2014
> 
> 
> Repository: zookeeper-git
> 
> 
> Description
> -------
> 
> Address various security concerns around reconfig feature (ZOOKEEPER-2014) to 
> unblock 3.5.3 release.
> 
> 
> Diffs
> -----
> 
>   build.xml 5328035 
>   src/c/include/zookeeper.h 18a203d 
>   src/c/tests/TestReconfigServer.cc 6a429ac 
>   src/c/tests/ZooKeeperQuorumServer.h aa8b7cc 
>   src/c/tests/ZooKeeperQuorumServer.cc 23392cd 
>   src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 5a30da8 
>   src/docs/src/documentation/content/xdocs/zookeeperReconfig.xml 7168a01 
>   src/java/main/org/apache/zookeeper/ClientCnxn.java 12dd51c 
>   src/java/main/org/apache/zookeeper/KeeperException.java a05f1ab 
>   src/java/main/org/apache/zookeeper/ZooKeeper.java 1c9ed4e 
>   src/java/main/org/apache/zookeeper/ZooKeeperMain.java 25d61a4 
>   src/java/main/org/apache/zookeeper/admin/ZooKeeperAdmin.java PRE-CREATION 
>   src/java/main/org/apache/zookeeper/cli/CliCommand.java 3d0a90b 
>   src/java/main/org/apache/zookeeper/cli/ReconfigCommand.java deb7914 
>   src/java/main/org/apache/zookeeper/server/DataTree.java 4666578 
>   src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java e275f9c 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 
> e772fa8 
>   src/java/test/org/apache/zookeeper/TestableZooKeeper.java 4d46fdf 
>   src/java/test/org/apache/zookeeper/server/DataTreeTest.java d726643 
>   src/java/test/org/apache/zookeeper/server/quorum/ReconfigBackupTest.java 
> 241af52 
>   
> src/java/test/org/apache/zookeeper/server/quorum/ReconfigDuringLeaderSyncTest.java
>  301837d 
>   
> src/java/test/org/apache/zookeeper/server/quorum/ReconfigFailureCasesTest.java
>  e7147b3 
>   src/java/test/org/apache/zookeeper/server/quorum/ReconfigLegacyTest.java 
> ee9f2e2 
>   
> src/java/test/org/apache/zookeeper/server/quorum/StandaloneDisabledTest.java 
> 1f6ce1f 
>   src/java/test/org/apache/zookeeper/test/ACLTest.java 9920fc4 
>   src/java/test/org/apache/zookeeper/test/ReconfigExceptionTest.java 
> PRE-CREATION 
>   src/java/test/org/apache/zookeeper/test/ReconfigMisconfigTest.java 
> PRE-CREATION 
>   src/java/test/org/apache/zookeeper/test/ReconfigTest.java 248a754 
>   src/java/test/org/apache/zookeeper/test/StandaloneTest.java 5c95280 
> 
> Diff: https://reviews.apache.org/r/51546/diff/
> 
> 
> Testing
> -------
> 
> Added new Java unit tests that cover various exception cases of using 
> reconfig API without proper set up. 
> All existing tests (Java and C) passed under stress tests (minors those known 
> flaky tests.).
> Manuelly tested reconfig commands using ZooKeeper command line tool.
> 
> 
> Thanks,
> 
> Michael Han
> 
>

Reply via email to