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



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

    (commented on the JIRA but commenting here as well to make it easier to 
fix/follow-up).
    
    This can just be:
    
    maxNotificationInterval = 
Integer.getInteger("zookeeper.maxFleNotificationInterval", 60000);



./src/java/test/org/apache/zookeeper/test/FLEMaxIntervalNotificationTest.java
<https://reviews.apache.org/r/15753/#comment57934>

    Nit: for all the LOG.info/LOG.debug statements, I think it's cleaner to use 
string extrapolation as opposed to concatenating strings. i.e.: instead of:
    
    LOG.info("Two equal notification intervals measured [" + counter + " of "
    +                                + numEqualIntervalsNeeded + "]");
    
    just:
    
    LOG.info("Two equal notification intervals measured [{} of {}]", counter, 
numEqualIntervalsNeeded);
    
    



./src/java/test/org/apache/zookeeper/test/FLEMaxIntervalNotificationTest.java
<https://reviews.apache.org/r/15753/#comment57935>

    maxFleNotificationInterval = 
Integer.getInteger("zookeeper.maxFleNotificationInterval");
    
    you probably want to make "zookeeper.maxFleNotificationInterval" a public 
constant somewhere. 



./src/java/test/org/apache/zookeeper/test/FLEMaxIntervalNotificationTest.java
<https://reviews.apache.org/r/15753/#comment57936>

    simpler: Assert.assertFalse("Should not be alive", thread.isAlive());


- Raul Gutierrez Segales


On Dec. 12, 2013, 5:12 p.m., Daniel Peon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15753/
> -----------------------------------------------------------
> 
> (Updated Dec. 12, 2013, 5:12 p.m.)
> 
> 
> Review request for zookeeper, German Blanco and fpj.
> 
> 
> Bugs: ZOOKEEPER-1814
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1814
> 
> 
> Repository: zookeeper
> 
> 
> Description
> -------
> 
> See Zookeeper-1814.
> 
> 
> Diffs
> -----
> 
>   ./src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 1550368 
>   ./src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 
> 1550368 
>   ./src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 
> 1550368 
>   
> ./src/java/test/org/apache/zookeeper/test/FLEMaxIntervalNotificationTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/15753/diff/
> 
> 
> Testing
> -------
> 
> New test case checking that the parameter maxNotificationInterval can be 
> configurable.
> 
> The test case overwrites the receiveConnection method in QuorumCnxManager 
> class in order to avoid any answer to Fast Leader Election. Thus, FLE is 
> forced to retry increasing exponentially the notifications interval. This 
> interval is measured and the test case stops when 3 consecutive measures are 
> equal. The test case will be OK if the measure is the configured in the cfg 
> file.
> 
> 
> File Attachments
> ----------------
> 
> ZOOKEEPER-1814.patch
>   
> https://reviews.apache.org/media/uploaded/files/2013/12/11/7e510364-6df3-486e-b338-3398960ba6b0__ZOOKEEPER-1814.patch
> 
> 
> Thanks,
> 
> Daniel Peon
> 
>

Reply via email to