Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Plain View
Zookeeper >> mail # dev >> Review Request 15568: See ZOOKEEPER-1810


+
German Blanco 2013-11-15, 07:00
Copy link to this message
-
Re: Review Request 15568: See ZOOKEEPER-1810

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

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

    space: synchronized (self) {

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

    Use {} instead of concatenating?

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

    Even though this is a critical path the params for LOG.debug aren't computed so could probably drop the if(LOG.isDebugEnabled) - your call.
    
    Also - could you change it to use {} instead of concatenating since you are already fixing the line :-).

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

    Even though this is a critical path the params for LOG.debug aren't computed so could probably drop the if(LOG.isDebugEnabled) - your call.
    
    Also - could you change it to use {} instead of concatenating since you are already fixing the line :-).

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

    Maybe convert to {} instead of concat while we are at it?

./src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java
<https://reviews.apache.org/r/15568/#comment56025>

    extra newline

./src/java/test/org/apache/zookeeper/server/quorum/FLETestUtils.java
<https://reviews.apache.org/r/15568/#comment56026>

    Lets start pushing towards {} instead of +.

./src/java/test/org/apache/zookeeper/server/quorum/FLETestUtils.java
<https://reviews.apache.org/r/15568/#comment56027>

    ditto
- Raul Gutierrez Segales
On Nov. 15, 2013, 7 a.m., German Blanco wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15568/
> -----------------------------------------------------------
>
> (Updated Nov. 15, 2013, 7 a.m.)
>
>
> Review request for zookeeper, fpj and Raul Gutierrez Segales.
>
>
> Bugs: ZOOKEEPER-1810
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1810
>
>
> Repository: zookeeper
>
>
> Description
> -------
>
> See ZOOKEEPER-1810
>
>
> Diffs
> -----
>
>   ./src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java 1542171
>   ./src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 1542171
>   ./src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1542171
>   ./src/java/main/org/apache/zookeeper/server/quorum/Vote.java 1542171
>   ./src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java PRE-CREATION
>   ./src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java PRE-CREATION
>   ./src/java/test/org/apache/zookeeper/server/quorum/FLETestUtils.java PRE-CREATION
>   ./src/java/test/org/apache/zookeeper/test/FLEBackwardElectionRoundTest.java 1542171
>   ./src/java/test/org/apache/zookeeper/test/FLELostMessageTest.java 1542171
>   ./src/java/test/org/apache/zookeeper/test/FLENewEpochTest.java 1542171
>   ./src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 1542171
>   ./src/java/test/org/apache/zookeeper/test/FLETest.java 1542171
>   ./src/java/test/org/apache/zookeeper/test/FLETestUtils.java 1542171
>   ./src/java/test/org/apache/zookeeper/test/FLEZeroWeightTest.java 1542171
>   ./src/java/test/org/apache/zookeeper/test/LENonTerminateTest.java 1542171
>
> Diff: https://reviews.apache.org/r/15568/diff/
>
>
> Testing
> -------
>
> Test included.
>
>
> Thanks,
>
> German Blanco
>
>

+
German Blanco 2013-11-18, 19:56
+
Raul Gutierrez Segales 2013-11-20, 00:17
+
German Blanco 2014-03-05, 14:23