zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From anmolnar <...@git.apache.org>
Subject [GitHub] zookeeper pull request #451: ZOOKEEPER-2184: Zookeeper Client should re-reso...
Date Fri, 25 May 2018 09:55:04 GMT
Github user anmolnar commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/451#discussion_r190846274
  
    --- Diff: src/java/main/org/apache/zookeeper/client/HostProvider.java ---
    @@ -53,7 +54,7 @@
          * @param spinDelay
          *            Milliseconds to wait if all hosts have been tried once.
          */
    -    public InetSocketAddress next(long spinDelay);
    +    public InetSocketAddress next(long spinDelay) throws UnknownHostException;
    --- End diff --
    
    @hanm Answering @fpj comment below:
    
    > > We need a way to break the loop in the case the client closes, though.
    
    > That's actually a good reason for _not_ dealing with the error here. Because the
caller - ClientCnxn - is be able to detect client closes, but StatisHostProvider is not.
    
    In a nutshell the problem of unresolvable DNS name must be handled somewhere and here're
the considerations:
    1. Client should endlessly try to resolve DNS names instead of giving up at some point.
The re-try logic is already implemented in the caller methods properly.
    2. In order to avoid API change we have to replicate the retry logic in the next() method
which would add more complexity to the method and more extensive testing
    
    With these considerations I strongly believe that the API change is reasonable.
    That's basically what we discussed in the e-mail thread too.
    
    I'll update the comment as requested. Thanks.


---

Mime
View raw message