You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zookeeper.apache.org by ha...@apache.org on 2017/08/01 15:55:28 UTC

zookeeper git commit: ZOOKEEPER-2614 ZOOKEEPER-1576: Port to branch 3.4

Repository: zookeeper
Updated Branches:
  refs/heads/branch-3.4 ce67fdd46 -> be1409cc9


ZOOKEEPER-2614 ZOOKEEPER-1576: Port 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.

Author: Thomas Schüttel <th...@daimler.com>

Reviewers: Edward Ribeiro <ed...@gmail.com>, Michael Han <ha...@apache.org>

Closes #320 from tschuettel/ZOOKEEPER-2614


Project: http://git-wip-us.apache.org/repos/asf/zookeeper/repo
Commit: http://git-wip-us.apache.org/repos/asf/zookeeper/commit/be1409cc
Tree: http://git-wip-us.apache.org/repos/asf/zookeeper/tree/be1409cc
Diff: http://git-wip-us.apache.org/repos/asf/zookeeper/diff/be1409cc

Branch: refs/heads/branch-3.4
Commit: be1409cc9a14ac2e28693e0e02a0ba6d9713565e
Parents: ce67fdd
Author: Thomas Schüttel <th...@daimler.com>
Authored: Tue Aug 1 08:55:24 2017 -0700
Committer: Michael Han <ha...@apache.org>
Committed: Tue Aug 1 08:55:24 2017 -0700

----------------------------------------------------------------------
 .../zookeeper/client/StaticHostProvider.java    | 48 ++++++++++----------
 .../zookeeper/test/StaticHostProviderTest.java  | 33 ++++++++++----
 2 files changed, 50 insertions(+), 31 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zookeeper/blob/be1409cc/src/java/main/org/apache/zookeeper/client/StaticHostProvider.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/client/StaticHostProvider.java b/src/java/main/org/apache/zookeeper/client/StaticHostProvider.java
index 5754b27..58e3e6b 100644
--- a/src/java/main/org/apache/zookeeper/client/StaticHostProvider.java
+++ b/src/java/main/org/apache/zookeeper/client/StaticHostProvider.java
@@ -52,33 +52,35 @@ public final class StaticHostProvider implements HostProvider {
      * 
      * @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 {
+                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()));
+                    }
+                }
+            } catch (UnknownHostException e) {
+                LOG.error("Unable to connect to server: {}", address, e);
             }
         }
         

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/be1409cc/src/java/test/org/apache/zookeeper/test/StaticHostProviderTest.java
----------------------------------------------------------------------
diff --git a/src/java/test/org/apache/zookeeper/test/StaticHostProviderTest.java b/src/java/test/org/apache/zookeeper/test/StaticHostProviderTest.java
index aa78a4b..8414be9 100644
--- a/src/java/test/org/apache/zookeeper/test/StaticHostProviderTest.java
+++ b/src/java/test/org/apache/zookeeper/test/StaticHostProviderTest.java
@@ -43,7 +43,7 @@ public class StaticHostProviderTest extends ZKTestCase {
     private static final Logger LOG = LoggerFactory.getLogger(StaticHostProviderTest.class);
     
     @Test
-    public void testNextGoesRound() throws UnknownHostException {
+    public void testNextGoesRound() {
         HostProvider hostProvider = getHostProvider((byte) 2);
         InetSocketAddress first = hostProvider.next(0);
         assertTrue(first instanceof InetSocketAddress);
@@ -52,7 +52,7 @@ public class StaticHostProviderTest extends ZKTestCase {
     }
 
     @Test
-    public void testNextGoesRoundAndSleeps() throws UnknownHostException {
+    public void testNextGoesRoundAndSleeps() {
         byte size = 2;
         HostProvider hostProvider = getHostProvider(size);
         while (size > 0) {
@@ -66,7 +66,7 @@ public class StaticHostProviderTest extends ZKTestCase {
     }
 
     @Test
-    public void testNextDoesNotSleepForZero() throws UnknownHostException {
+    public void testNextDoesNotSleepForZero() {
         byte size = 2;
         HostProvider hostProvider = getHostProvider(size);
         while (size > 0) {
@@ -87,7 +87,7 @@ public class StaticHostProviderTest extends ZKTestCase {
     }
 
     @Test
-    public void testOnConnectDoesNotReset() throws UnknownHostException {
+    public void testOnConnectDoesNotReset() {
         HostProvider hostProvider = getHostProvider((byte) 2);
         InetSocketAddress first = hostProvider.next(0);
         hostProvider.onConnected();
@@ -111,8 +111,26 @@ public class StaticHostProviderTest extends ZKTestCase {
         }
     }
 
-    private StaticHostProvider getHostProviderUnresolved(byte size)
-            throws UnknownHostException {
+    @Test(expected = IllegalArgumentException.class)
+    public void testTwoInvalidHostAddresses() {
+        ArrayList<InetSocketAddress> list = new ArrayList<InetSocketAddress>();
+        list.add(new InetSocketAddress("a", 2181));
+        list.add(new InetSocketAddress("b", 2181));
+        new StaticHostProvider(list);
+    }
+
+    public void testOneInvalidHostAddresses() {
+        Collection<InetSocketAddress> addr = getUnresolvedServerAddresses((byte) 1);
+        addr.add(new InetSocketAddress("a", 2181));
+
+        StaticHostProvider sp = new StaticHostProvider(addr);
+        InetSocketAddress n1 = sp.next(0);
+        InetSocketAddress n2 = sp.next(0);
+
+        assertEquals(n2, n1);
+    }
+
+    private StaticHostProvider getHostProviderUnresolved(byte size) {
         return new StaticHostProvider(getUnresolvedServerAddresses(size));
     }
 
@@ -125,8 +143,7 @@ public class StaticHostProviderTest extends ZKTestCase {
         return list;
     }
     
-    private StaticHostProvider getHostProvider(byte size)
-            throws UnknownHostException {
+    private StaticHostProvider getHostProvider(byte size) {
         ArrayList<InetSocketAddress> list = new ArrayList<InetSocketAddress>(
                 size);
         while (size > 0) {