Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/451#discussion_r165529085
  
    --- Diff: src/java/main/org/apache/zookeeper/client/StaticHostProvider.java 
---
    @@ -91,15 +79,106 @@ public 
StaticHostProvider(Collection<InetSocketAddress> serverAddresses) {
             Collections.shuffle(this.serverAddresses);
         }
     
    +    /**
    +     * Evaluate to a hostname if one is available and otherwise it returns 
the
    +     * string representation of the IP address.
    +     *
    +     * In Java 7, we have a method getHostString, but earlier versions do 
not support it.
    +     * This method is to provide a replacement for 
InetSocketAddress.getHostString().
    +     *
    +     * @param addr
    +     * @return Hostname string of address parameter
    +     */
    +    private String getHostString(InetSocketAddress addr) {
    +        String hostString = "";
    +
    +        if (addr == null) {
    +            return hostString;
    +        }
    +        if (!addr.isUnresolved()) {
    +            InetAddress ia = addr.getAddress();
    +
    +            // If the string starts with '/', then it has no hostname
    +            // and we want to avoid the reverse lookup, so we return
    +            // the string representation of the address.
    +            if (ia.toString().startsWith("/")) {
    +                hostString = ia.getHostAddress();
    +            } else {
    +                hostString = addr.getHostName();
    +            }
    +        } else {
    +            // According to the Java 6 documentation, if the hostname is
    +            // unresolved, then the string before the colon is the 
hostname.
    +            String addrString = addr.toString();
    +            hostString = addrString.substring(0, 
addrString.lastIndexOf(':'));
    +        }
    +
    +        return hostString;
    +    }
    +
         public int size() {
             return serverAddresses.size();
         }
     
    +    // Counts the number of addresses added and removed during
    +    // the last call to next. Used mainly for test purposes.
    +    // See StasticHostProviderTest.
    +    private int nextAdded = 0;
    +    private int nextRemoved = 0;
    +
    +    int getNextAdded() {
    +        return nextAdded;
    +    }
    +
    +    int getNextRemoved() {
    +        return nextRemoved;
    +    }
    +
         public InetSocketAddress next(long spinDelay) {
    -        ++currentIndex;
    -        if (currentIndex == serverAddresses.size()) {
    -            currentIndex = 0;
    +        // Handle possible connection error by re-resolving hostname if 
possible
    +        if (!connectedSinceNext) {
    --- End diff --
    
    would you mind explaining exactly under which conditions we reresolve the 
hostname and under which conditions we try the next one in the host list? My 
reading is that this reresolves everything if the client fails to connect to 
two hosts in a row. Is this the desired behavior?
    
    And do we always reresolve all serverAddresses?


---

Reply via email to