zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (ZOOKEEPER-2184) Zookeeper Client should re-resolve hosts when connection attempts fail
Date Wed, 07 Feb 2018 16:19:00 GMT

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

ASF GitHub Bot commented on ZOOKEEPER-2184:
-------------------------------------------

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

    https://github.com/apache/zookeeper/pull/451#discussion_r166670125
  
    --- Diff: src/java/main/org/apache/zookeeper/client/StaticHostProvider.java ---
    @@ -58,48 +61,122 @@
         public StaticHostProvider(Collection<InetSocketAddress> serverAddresses) {
             for (InetSocketAddress address : serverAddresses) {
                 try {
    -                InetAddress ia = address.getAddress();
    -                InetAddress resolvedAddresses[] = InetAddress.getAllByName((ia != null)
? ia.getHostAddress() :
    -                        address.getHostName());
    +                InetAddress resolvedAddresses[] = InetAddress.getAllByName(getHostString(address));
                     for (InetAddress resolvedAddress : resolvedAddresses) {
    -                    // If hostName is null but the address is not, we can tell that
    -                    // the hostName is an literal IP address. Then we can set the host
string as the hostname
    -                    // safely to avoid reverse DNS lookup.
    -                    // As far as i know, the only way to check if the hostName is null
is use toString().
    -                    // Both the two implementations of InetAddress are final class, so
we can trust the return value of
    -                    // the toString() method.
    -                    if (resolvedAddress.toString().startsWith("/")
    -                            && resolvedAddress.getAddress() != null) {
    -                        this.serverAddresses.add(
    -                                new InetSocketAddress(InetAddress.getByAddress(
    -                                        address.getHostName(),
    -                                        resolvedAddress.getAddress()),
    -                                        address.getPort()));
    -                    } else {
    -                        this.serverAddresses.add(new InetSocketAddress(resolvedAddress.getHostAddress(),
address.getPort()));
    -                    }
    +                    this.serverAddresses.add(new InetSocketAddress(resolvedAddress, address.getPort()));
                     }
                 } catch (UnknownHostException e) {
                     LOG.error("Unable to connect to server: {}", address, e);
                 }
             }
    -        
    +
             if (this.serverAddresses.isEmpty()) {
                 throw new IllegalArgumentException(
                         "A HostProvider may not be empty!");
             }
             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;
    +
    +    public int getNextAdded() {
    +        return nextAdded;
    +    }
    +
    +    public 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) {
    +            InetSocketAddress curAddr = serverAddresses.get(currentIndex);
    +            String curHostString = getHostString(curAddr);
    +            if (!curHostString.equals(curAddr.getAddress().getHostAddress())) {
    +                LOG.info("Resolving again hostname: {}", getHostString(curAddr));
    +                try {
    +                    int thePort = curAddr.getPort();
    +                    InetAddress resolvedAddresses[] = InetAddress.getAllByName(curHostString);
    +                    nextAdded = 0;
    +                    nextRemoved = 0;
    +                    if (resolvedAddresses.length == 1) {
    +                        serverAddresses.set(currentIndex, new InetSocketAddress(resolvedAddresses[0],
thePort));
    +                        nextAdded = nextRemoved = 1;
    +                        LOG.debug("Newly resolved address: {}", resolvedAddresses[0]);
    +                    } else {
    +                        int i = 0;
    +                        while (i < serverAddresses.size()) {
    +                            if (getHostString(serverAddresses.get(i)).equals(curHostString)
&&
    --- End diff --
    
    How to shuffle the elements?


> Zookeeper Client should re-resolve hosts when connection attempts fail
> ----------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-2184
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2184
>             Project: ZooKeeper
>          Issue Type: Bug
>          Components: java client
>    Affects Versions: 3.4.6, 3.4.7, 3.4.8, 3.4.9, 3.4.10, 3.5.0, 3.5.1, 3.5.2, 3.5.3,
3.4.11
>         Environment: Ubuntu 14.04 host, Docker containers for Zookeeper & Kafka
>            Reporter: Robert P. Thille
>            Assignee: Flavio Junqueira
>            Priority: Blocker
>              Labels: easyfix, patch
>             Fix For: 3.5.4, 3.4.12
>
>         Attachments: ZOOKEEPER-2184.patch
>
>
> Testing in a Docker environment with a single Kafka instance using a single Zookeeper
instance. Restarting the Zookeeper container will cause it to receive a new IP address. Kafka
will never be able to reconnect to Zookeeper and will hang indefinitely. Updating DNS or /etc/hosts
with the new IP address will not help the client to reconnect as the zookeeper/client/StaticHostProvider
resolves the connection string hosts at creation time and never re-resolves.
> A solution would be for the client to notice that connection attempts fail and attempt
to re-resolve the hostnames in the connectString.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Mime
View raw message