zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hanm <...@git.apache.org>
Subject [GitHub] zookeeper pull request #534: ZOOKEEPER-2184 Zookeeper Client should re-resol...
Date Tue, 10 Jul 2018 18:55:37 GMT
Github user hanm commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/534#discussion_r201456281
  
    --- Diff: src/java/main/org/apache/zookeeper/client/StaticHostProvider.java ---
    @@ -149,15 +185,12 @@ public StaticHostProvider(Collection<InetSocketAddress> serverAddresses,
          * @param currentHost the host to which this client is currently connected
          * @return true if changing connections is necessary for load-balancing, false otherwise
 
          */
    -
    -
         @Override
         public synchronized boolean updateServerList(
                 Collection<InetSocketAddress> serverAddresses,
                 InetSocketAddress currentHost) {
    -        // Resolve server addresses and shuffle them
    -        List<InetSocketAddress> resolvedList = resolveAndShuffle(serverAddresses);
    -        if (resolvedList.isEmpty()) {
    +        List<InetSocketAddress> shuffledList = shuffle(serverAddresses);
    --- End diff --
    
    >> However, exactly the same functionality has already been implemented in the InetSocketAddress.equals()
method
    
    I am wondering if this is the case or not. I just did a random peek at https://github.com/openjdk-mirror/jdk7u-jdk/blob/master/src/share/classes/java/net/InetSocketAddress.java#L300,
looks like if we compare an unresolved address with a resolved address, the equals will return
false - but the code you pasted will return true if getHostString works for both resolved
and unresolved address... could you double check this behavior?
    
    Also, is it possible to add a test case to cover the case where the second parameter of
updateServerList is a resolved address? The existing test cases only cover the case where
the second parameter (myServer) is unresolved. In practice I think the method updateServerList
is called by ZooKeeper's updateServerList method with second parameter as a resolved address
(the remote server where current client connected to.). 
    
    



---

Mime
View raw message