-----------------------------------------------------------
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
> 
>

Reply via email to