[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-1990?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14075231#comment-14075231
 ] 

Alexander Shraer commented on ZOOKEEPER-1990:
---------------------------------------------

Regarding StaticHostProvider.java - Randomness stuff was introduced in ZK-1355 
to make sure that each client has a different permutation of the servers, and 
that client-side rebalancing after reconfig ends up with equal load.

75:       sourceOfRandomness = new Random(System.currentTimeMillis() ^ 
this.hashCode());

If I remember correctly the goal here is to make sure that when multiple client 
threads start at the same time on the same machine (such as in our systest) and 
execute this line each one is likely to have a different random seed. 

src/java/test/org/apache/zookeeper/test/StaticHostProviderTest.java
40:    private Random r = new Random(1);
src/java/main/org/apache/zookeeper/client/StaticHostProvider.java
98:        sourceOfRandomness = new Random(randomnessSeed);

These two are only used in tests. I want repeatable results, hence the seed of 
1. But I still want each client to have a StaticHostProvider that creates a 
unique permutation, so I pass the StaticHostProvider a randomness seed like 
this:

    private StaticHostProvider getHostProvider(byte size) {
        return new StaticHostProvider(getServerAddresses(size), r.nextLong());
    }


> suspicious instantiation of java Random instances
> -------------------------------------------------
>
>                 Key: ZOOKEEPER-1990
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1990
>             Project: ZooKeeper
>          Issue Type: Bug
>    Affects Versions: 3.5.0
>            Reporter: Patrick Hunt
>            Priority: Critical
>             Fix For: 3.5.0
>
>
> It's not clear to me why we are doing this, but it looks very suspicious. Why 
> aren't we just calling "new Random()" in these cases? (even for the tests I 
> don't really see it - typically that would just be for repeatability)
> {noformat}
> ag "new Random[ \t]*\(" .
> src/java/main/org/apache/zookeeper/ClientCnxn.java
> 817:        private Random r = new Random(System.nanoTime());        
> src/java/main/org/apache/zookeeper/client/StaticHostProvider.java
> 75:       sourceOfRandomness = new Random(System.currentTimeMillis() ^ 
> this.hashCode());
> 98:        sourceOfRandomness = new Random(randomnessSeed);
> src/java/main/org/apache/zookeeper/server/quorum/AuthFastLeaderElection.java
> 420:                rand = new Random(java.lang.Thread.currentThread().getId()
> src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java
> 64:    private final Random r = new Random(System.nanoTime());
> src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java
> 537:        Random r = new Random(id ^ superSecret);
> 554:        Random r = new Random(sessionId ^ superSecret);
> src/java/test/org/apache/zookeeper/server/quorum/WatchLeakTest.java
> 271:        Random r = new Random(SESSION_ID ^ superSecret);
> src/java/test/org/apache/zookeeper/server/quorum/CommitProcessorTest.java
> 151:            Random rand = new Random(Thread.currentThread().getId());
> 258:            Random rand = new Random(Thread.currentThread().getId());
> 288:        Random rand = new Random(Thread.currentThread().getId());
> src/java/test/org/apache/zookeeper/test/StaticHostProviderTest.java
> 40:    private Random r = new Random(1);
> {noformat}



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to