You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by tschuettel <gi...@git.apache.org> on 2017/07/30 11:11:35 UTC

[GitHub] zookeeper pull request #320: ZOOKEEPER-2614 Port ZOOKEEPER-1576 to branch 3....

GitHub user tschuettel opened a pull request:

    https://github.com/apache/zookeeper/pull/320

    ZOOKEEPER-2614 Port ZOOKEEPER-1576 to branch 3.4

    This is a backport of ZOOKEEPER-1576 to the 3.4-line.
    When running Zookeeper as an ensemble in a dynamic environment such as
    Kubernetes, the DNS entry of a Zookeeper pod is apparently instantly
    purged as one of the nodes goes down. This leads to an UnknownHostException
    when interacting with the cluster, even though a healthy majority of nodes
    is still working.
    This behavior is also observed in a firewall situation as described in
    ZOOOKEEPER-1576.
    This fix catches and logs the UnkownHostException and continues trying the
    next node.
    Thanks to Vishal Khandelwal for providing the patch.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/DaimlerTSS/zookeeper ZOOKEEPER-2614

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zookeeper/pull/320.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #320
    
----
commit 67eff1c9984c0081cd36d4f978fcce642362b4b3
Author: Thomas Schüttel <th...@daimler.com>
Date:   2017-07-30T10:50:23Z

    ZOOKEEPER-2614 Port ZOOKEEPER-1576 to branch 3.4
    
    This is a backport of ZOOKEEPER-1576 to the 3.4-line.
    When running Zookeeper as an ensemble in a dynamic environment such as
    Kubernetes, the DNS entry of a Zookeeper pod is apparently instantly
    purged as one of the nodes goes down. This leads to an UnknownHostException
    when interacting with the cluster, even though a healthy majority of nodes
    is still working.
    This behavior is also observed in a firewall situation as described in
    ZOOOKEEPER-1576.
    This fix catches and logs the UnkownHostException and continues trying the
    next node.
    Thanks to Vishal Khandelwal for providing the patch.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper issue #320: ZOOKEEPER-2614 Port ZOOKEEPER-1576 to branch 3.4

Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on the issue:

    https://github.com/apache/zookeeper/pull/320
  
    Merged to 3.4: https://github.com/apache/zookeeper/commit/be1409cc9a14ac2e28693e0e02a0ba6d9713565e
    
    Thanks for the work @tschuettel, please close this pull request.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #320: ZOOKEEPER-2614 Port ZOOKEEPER-1576 to branch 3....

Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/320#discussion_r130246293
  
    --- Diff: src/java/main/org/apache/zookeeper/client/StaticHostProvider.java ---
    @@ -52,34 +52,36 @@
          * 
          * @param serverAddresses
          *            possibly unresolved ZooKeeper server addresses
    -     * @throws UnknownHostException
          * @throws IllegalArgumentException
          *             if serverAddresses is empty or resolves to an empty list
          */
    -    public StaticHostProvider(Collection<InetSocketAddress> serverAddresses)
    -            throws UnknownHostException {
    +    public StaticHostProvider(Collection<InetSocketAddress> serverAddresses) {
             for (InetSocketAddress address : serverAddresses) {
    -            InetAddress ia = address.getAddress();
    -            InetAddress resolvedAddresses[] = InetAddress.getAllByName((ia!=null) ? ia.getHostAddress():
    -                address.getHostName());
    -            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()));
    -                }  
    -            }
    +        	try {
    --- End diff --
    
    Indentation here is a little bit off. 4 spaces instead of 8. Please update. Other than this patch looks good.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper issue #320: ZOOKEEPER-2614 Port ZOOKEEPER-1576 to branch 3.4

Posted by eribeiro <gi...@git.apache.org>.
Github user eribeiro commented on the issue:

    https://github.com/apache/zookeeper/pull/320
  
    LGTM, that is, on par with the version on `master/branch-3.5`. Wdyt @hanm ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #320: ZOOKEEPER-2614 Port ZOOKEEPER-1576 to branch 3....

Posted by tschuettel <gi...@git.apache.org>.
Github user tschuettel closed the pull request at:

    https://github.com/apache/zookeeper/pull/320


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper issue #320: ZOOKEEPER-2614 Port ZOOKEEPER-1576 to branch 3.4

Posted by tschuettel <gi...@git.apache.org>.
Github user tschuettel commented on the issue:

    https://github.com/apache/zookeeper/pull/320
  
    I fixed the indentation problem. Please check again.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---