ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-1576

Zookeeper cluster - failed to connect to cluster if one of the provided IPs causes java.net.UnknownHostException

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.5.0
    • Fix Version/s: 3.5.0
    • Component/s: server
    • Labels:
      None
    • Environment:

      Three 3.4.3 zookeeper servers in cluster, linux.

      Description

      Using a cluster of three 3.4.3 zookeeper servers.
      All the servers are up, but on the client machine, the firewall is blocking one of the servers.
      The following exception is happening, and the client is not connected to any of the other cluster members.

      The exception:Nov 02, 2012 9:54:32 PM com.netflix.curator.framework.imps.CuratorFrameworkImpl logError
      SEVERE: Background exception was not retry-able or retry gave up
      java.net.UnknownHostException: scnrmq003.myworkday.com
      at java.net.Inet4AddressImpl.lookupAllHostAddr(Native Method)
      at java.net.InetAddress$1.lookupAllHostAddr(Unknown Source)
      at java.net.InetAddress.getAddressesFromNameService(Unknown Source)
      at java.net.InetAddress.getAllByName0(Unknown Source)
      at java.net.InetAddress.getAllByName(Unknown Source)
      at java.net.InetAddress.getAllByName(Unknown Source)
      at org.apache.zookeeper.client.StaticHostProvider.<init>(StaticHostProvider.java:60)
      at org.apache.zookeeper.ZooKeeper.<init>(ZooKeeper.java:440)
      at org.apache.zookeeper.ZooKeeper.<init>(ZooKeeper.java:375)

      The code at the org.apache.zookeeper.client.StaticHostProvider.<init>(StaticHostProvider.java:60) is :
      public StaticHostProvider(Collection<InetSocketAddress> serverAddresses) throws UnknownHostException {
      for (InetSocketAddress address : serverAddresses) {
      InetAddress resolvedAddresses[] = InetAddress.getAllByName(address
      .getHostName());
      for (InetAddress resolvedAddress : resolvedAddresses)

      { this.serverAddresses.add(new InetSocketAddress(resolvedAddress .getHostAddress(), address.getPort())); }

      }
      ......

      The for-loop is not trying to resolve the rest of the servers on the list if there is an UnknownHostException at the InetAddress.getAllByName(address.getHostName());
      and it fails the client connection creation.

      I was expecting the connection will be created for the other members of the cluster.
      Also, InetAddress is a blocking command, and if it takes very long time, (longer than the defined timeout) - that also should allow us to continue to try and connect to the other servers on the list.
      Assuming this will be fixed, and we will get connection to the current available servers, I think the zookeeper should continue to retry to connect to the not-connected server of the cluster, so it will be able to use it later when it is back.
      If one of the servers on the list is not available during the connection creation, then it should be retried every x time despite the fact that we

      1. ZOOKEEPER-1576-3.4.patch
        7 kB
        Benjamin Jaton
      2. ZOOKEEPER-1576.patch
        8 kB
        Michi Mutsuzaki
      3. ZOOKEEPER-1576.5.patch
        9 kB
        Edward Ribeiro
      4. ZOOKEEPER-1576.4.patch
        9 kB
        Edward Ribeiro
      5. ZOOKEEPER-1576.3.patch
        8 kB
        Edward Ribeiro

        Issue Links

          Activity

          Hide
          Edward Ribeiro added a comment -

          I am attaching a patch to solve the issue, but I think we still think further if the interesting additional questions raised by Tally should be addressed.

          Show
          Edward Ribeiro added a comment - I am attaching a patch to solve the issue, but I think we still think further if the interesting additional questions raised by Tally should be addressed.
          Hide
          Edward Ribeiro added a comment -

          I think this patch would be a nice addition, but it is still subject to further discussion as it removes a previously thrown exception (UnknownHostException), effectively modifying the method signature. Plus, Tally comments are very insightful. Is it the best strategy? What do you think?

          Show
          Edward Ribeiro added a comment - I think this patch would be a nice addition, but it is still subject to further discussion as it removes a previously thrown exception (UnknownHostException), effectively modifying the method signature. Plus, Tally comments are very insightful. Is it the best strategy? What do you think?
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12600987/ZOOKEEPER-1576.patch
          against trunk revision 1516126.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1551//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12600987/ZOOKEEPER-1576.patch against trunk revision 1516126. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1551//console This message is automatically generated.
          Hide
          Dmitry added a comment -

          It seems that second version of patch was forgotten. Could you guys check if second attached patch can be applied correctly?

          This issue is important for everyone who uses fqdn addresses instead of ips.

          Show
          Dmitry added a comment - It seems that second version of patch was forgotten. Could you guys check if second attached patch can be applied correctly? This issue is important for everyone who uses fqdn addresses instead of ips.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12600996/ZOOKEEPER-1576.2.patch
          against trunk revision 1546227.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1811//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12600996/ZOOKEEPER-1576.2.patch against trunk revision 1546227. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1811//console This message is automatically generated.
          Hide
          Camille Fournier added a comment -

          I'm looking at this, but is this for 3.5 or 3.4?

          Show
          Camille Fournier added a comment - I'm looking at this, but is this for 3.5 or 3.4?
          Hide
          Edward Ribeiro added a comment -

          I am really sorry for the mess up, Camille, but it's for 3.4. I'll fix this now.

          PS: I can't figure it now why it's not applying to trunk anymore because I am not able to checkout and work on zookeeper now, so I'll only be able to see the problem at home.

          Show
          Edward Ribeiro added a comment - I am really sorry for the mess up, Camille, but it's for 3.4. I'll fix this now. PS: I can't figure it now why it's not applying to trunk anymore because I am not able to checkout and work on zookeeper now, so I'll only be able to see the problem at home.
          Hide
          Edward Ribeiro added a comment -

          The previous patches 1 and 2 don't apply to trunk anymore. A new version is attached.

          Show
          Edward Ribeiro added a comment - The previous patches 1 and 2 don't apply to trunk anymore. A new version is attached.
          Hide
          Camille Fournier added a comment -

          This looks to apply to trunk, but not 3.4. Do you want this patch on both branches?

          Show
          Camille Fournier added a comment - This looks to apply to trunk, but not 3.4. Do you want this patch on both branches?
          Hide
          Edward Ribeiro added a comment -

          Um... I was planning only on latest release (3.4.6) or even later (3.5) if you decide so. I'll take a look at 3.4 branch tough. Previous patches failed due to a merge conflict. OTOH, branch 3.4 should be very different from the trunk, right? I can take a look at it, tough.

          Show
          Edward Ribeiro added a comment - Um... I was planning only on latest release (3.4.6) or even later (3.5) if you decide so. I'll take a look at 3.4 branch tough. Previous patches failed due to a merge conflict. OTOH, branch 3.4 should be very different from the trunk, right? I can take a look at it, tough.
          Hide
          Camille Fournier added a comment -

          Trunk only is fine, but then we need to change the tags to 3.5 instead of 3.4.6. Up to you.

          Show
          Camille Fournier added a comment - Trunk only is fine, but then we need to change the tags to 3.5 instead of 3.4.6. Up to you.
          Hide
          Edward Ribeiro added a comment -

          Okay. I've just changed to 3.5.0. Thanks.

          Show
          Edward Ribeiro added a comment - Okay. I've just changed to 3.5.0. Thanks.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12616449/ZOOKEEPER-1576.3.patch
          against trunk revision 1546227.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1812//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1812//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1812//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12616449/ZOOKEEPER-1576.3.patch against trunk revision 1546227. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1812//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1812//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1812//console This message is automatically generated.
          Hide
          Raul Gutierrez Segales added a comment -

          Some nits:

          +            String addr = (ia!=null) ? ia.getHostAddress(): address.getHostName();
          

          spaces between != and (possibly) you can drop the parentheses around `ia != null`.

          +                   if (resolvedAddress.toString().startsWith("/") && resolvedAddress.getAddress() != null) {
          +                        tmpList.add(new InetSocketAddress(InetAddress.getByAddress(address.getHostName(),
          +                                    resolvedAddress.getAddress()), address.getPort()));
          +                   } else {
          +                       tmpList.add(new InetSocketAddress(resolvedAddress.getHostAddress(), address.getPort()));
          +                   }
          

          long lines — make 'em < 80 maybe?

          +            LOG.error("Unable to connect to server (UnknowHostException): " + address);
          

          {} is preferred to string concatenation, so:

          +            LOG.error("Unable to connect to server (UnknowHostException): {}", address);
          
          +    public boolean updateServerList(Collection<InetSocketAddress> serverAddresses, InetSocketAddress currentHost) { 
          

          line too long.

          +	boolean updateServerList(Collection<InetSocketAddress> serverAddresses, InetSocketAddress currentHost);
          

          ditto.

          Show
          Raul Gutierrez Segales added a comment - Some nits: + String addr = (ia!=null) ? ia.getHostAddress(): address.getHostName(); spaces between != and (possibly) you can drop the parentheses around `ia != null`. + if (resolvedAddress.toString().startsWith("/") && resolvedAddress.getAddress() != null) { + tmpList.add(new InetSocketAddress(InetAddress.getByAddress(address.getHostName(), + resolvedAddress.getAddress()), address.getPort())); + } else { + tmpList.add(new InetSocketAddress(resolvedAddress.getHostAddress(), address.getPort())); + } long lines — make 'em < 80 maybe? + LOG.error("Unable to connect to server (UnknowHostException): " + address); {} is preferred to string concatenation, so: + LOG.error("Unable to connect to server (UnknowHostException): {}", address); + public boolean updateServerList(Collection<InetSocketAddress> serverAddresses, InetSocketAddress currentHost) { line too long. + boolean updateServerList(Collection<InetSocketAddress> serverAddresses, InetSocketAddress currentHost); ditto.
          Hide
          Edward Ribeiro added a comment -

          tl;dr: new version of the patch, addressing the Raul Gutierrez Segales suggestions

          Hi Raul Gutierrez Segales, thank you very much for your suggestions! Sorry for the delay, I have been reading Flavio Junqueira and Benjamin Reed's ZooKeeper book during the last two days.

          Well, I've addressed them as far as I could. Unfortunately, I couldn't remove the visibility operator (public) from updateListServer() because it breaks code elsewhere, but I tried to reduce the line sizes of the parts affected by the patch as much as I could while minimizing change. They still overflow the 80 lines, but by a small range this time. If you have further ideas, please let me know.

          Best regards,
          Edward

          Show
          Edward Ribeiro added a comment - tl;dr: new version of the patch, addressing the Raul Gutierrez Segales suggestions Hi Raul Gutierrez Segales , thank you very much for your suggestions! Sorry for the delay, I have been reading Flavio Junqueira and Benjamin Reed 's ZooKeeper book during the last two days. Well, I've addressed them as far as I could. Unfortunately, I couldn't remove the visibility operator (public) from updateListServer() because it breaks code elsewhere, but I tried to reduce the line sizes of the parts affected by the patch as much as I could while minimizing change. They still overflow the 80 lines, but by a small range this time. If you have further ideas, please let me know. Best regards, Edward
          Hide
          Raul Gutierrez Segales added a comment -

          Hi Edward Ribeiro,

          thanks for updating the patch. For things like:

          +    public StaticHostProvider(Collection<InetSocketAddress> serverAddresses, long randomnessSeed) {
          

          you could move some params to the next line (and indent w/ 8 spaces from the start of the upper line - note that JIRA might indent it funnily), i.e.:

          +    public StaticHostProvider(Collection<InetSocketAddress> serverAddresses,
          +                long randomnessSeed) {
          

          Some more nits:

          +               // the hostName is an literal IP address. Then we can set the host string as the hostname
          

          *is a literal IP address.

          +             // Both the two implementations of InetAddress are final class, so we can trust the return value of
          

          *Both implementations of InetAddress are final classes, ...

          In:

          +            LOG.error("(UnknowHostException) Unable to connect to server: {}", address);
          

          I wonder if instead of pre-pending `(UnknowHostException)` we should just append the exception, i.e.:

          +            LOG.error("Unable to connect to server: {}: {}", address, e);
          

          maybe this is more informative?

          In:

          +    public boolean updateServerList(Collection<InetSocketAddress> serverAddresses, 
          +                                    InetSocketAddress currentHost) { 
          

          it seems the indentation for the wrapped parameter is more than it should be. Maybe (8 spaces after the identifier from above):

          +    public boolean updateServerList(Collection<InetSocketAddress> serverAddresses, 
          +                InetSocketAddress currentHost) { 
          

          ?

          In:

          +	boolean updateServerList(Collection<InetSocketAddress> serverAddresses, InetSocketAddress currentHost);
          

          maybe:

          +	boolean updateServerList(Collection<InetSocketAddress> serverAddresses,
          +                    InetSocketAddress currentHost);
          
          Show
          Raul Gutierrez Segales added a comment - Hi Edward Ribeiro , thanks for updating the patch. For things like: + public StaticHostProvider(Collection<InetSocketAddress> serverAddresses, long randomnessSeed) { you could move some params to the next line (and indent w/ 8 spaces from the start of the upper line - note that JIRA might indent it funnily), i.e.: + public StaticHostProvider(Collection<InetSocketAddress> serverAddresses, + long randomnessSeed) { Some more nits: + // the hostName is an literal IP address. Then we can set the host string as the hostname *is a literal IP address. + // Both the two implementations of InetAddress are final class, so we can trust the return value of *Both implementations of InetAddress are final classes, ... In: + LOG.error("(UnknowHostException) Unable to connect to server: {}", address); I wonder if instead of pre-pending `(UnknowHostException)` we should just append the exception, i.e.: + LOG.error("Unable to connect to server: {}: {}", address, e); maybe this is more informative? In: + public boolean updateServerList(Collection<InetSocketAddress> serverAddresses, + InetSocketAddress currentHost) { it seems the indentation for the wrapped parameter is more than it should be. Maybe (8 spaces after the identifier from above): + public boolean updateServerList(Collection<InetSocketAddress> serverAddresses, + InetSocketAddress currentHost) { ? In: + boolean updateServerList(Collection<InetSocketAddress> serverAddresses, InetSocketAddress currentHost); maybe: + boolean updateServerList(Collection<InetSocketAddress> serverAddresses, + InetSocketAddress currentHost);
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12616881/ZOOKEEPER-1576.4.patch
          against trunk revision 1546227.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1813//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1813//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1813//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12616881/ZOOKEEPER-1576.4.patch against trunk revision 1546227. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1813//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1813//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1813//console This message is automatically generated.
          Hide
          Edward Ribeiro added a comment -

          Addressing Raul Gutierrez Segales latest suggestions.

          Show
          Edward Ribeiro added a comment - Addressing Raul Gutierrez Segales latest suggestions.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12616900/ZOOKEEPER-1576.5.patch
          against trunk revision 1546227.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1814//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1814//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1814//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12616900/ZOOKEEPER-1576.5.patch against trunk revision 1546227. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1814//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1814//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1814//console This message is automatically generated.
          Hide
          Edward Ribeiro added a comment -

          I suspending this patch for now because it failed some core tests. Below I've attached the most important parts of the log:

          [exec] [exec] /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/tests/TestReconfig.cc:570: Assertion: assertion failed [Expression: numClientsPerHost.at(i) <= upperboundClientsPerServer(numClients, numServers)]
          [exec] [exec] Failures !!!
          [exec] [exec] Run: 41 Failure total: 1 Failures: 1 Errors: 0
          [exec] [exec] FAIL: zktest-st

          (...)

          [exec] [exec] 1 of 2 tests failed
          [exec] [exec] Please report to user@zookeeper.apache.org

          Any idea what has gone wrong?

          Show
          Edward Ribeiro added a comment - I suspending this patch for now because it failed some core tests. Below I've attached the most important parts of the log: [exec] [exec] /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/tests/TestReconfig.cc:570: Assertion: assertion failed [Expression: numClientsPerHost.at(i) <= upperboundClientsPerServer(numClients, numServers)] [exec] [exec] Failures !!! [exec] [exec] Run: 41 Failure total: 1 Failures: 1 Errors: 0 [exec] [exec] FAIL: zktest-st (...) [exec] [exec] 1 of 2 tests failed [exec] [exec] Please report to user@zookeeper.apache.org Any idea what has gone wrong?
          Hide
          Camille Fournier added a comment -

          Probably nothing to do with your patch.
          Can we keep the trivial formatting code reviews to a minimum please, Raul Gutierrez Segales? I really don't think reformatting everything this patch touches just because it happens to touch it is a good practice in general.

          Show
          Camille Fournier added a comment - Probably nothing to do with your patch. Can we keep the trivial formatting code reviews to a minimum please, Raul Gutierrez Segales ? I really don't think reformatting everything this patch touches just because it happens to touch it is a good practice in general.
          Hide
          Camille Fournier added a comment - - edited

          Unfortunately Edward Ribeiro, this fails when I run it on my mac on the two tests you added... are you sure this works on all platforms?

          Show
          Camille Fournier added a comment - - edited Unfortunately Edward Ribeiro , this fails when I run it on my mac on the two tests you added... are you sure this works on all platforms?
          Hide
          Raul Gutierrez Segales added a comment -

          Sure Camille Fournier - but tbh requesting minimum formatting diligence (i.e.: spelling, indentation and consistency) on every + line (not the context ones) isn't trivial formatting but just basic sanity. Sadly, many patches don't have it which makes it very hard for people testing patches that haven't been committed yet, testing trunk, etc. Being able to read through those — and related — patches fast and without losing focus because of inconsistent style really helps when testing stuff.

          It would certainly have helped me when finding bugs like ZOOKEEPER-1805 and ZOOKEEPER-1807 for which I had to read through a lot of code with mixed styles or no style at all

          Show
          Raul Gutierrez Segales added a comment - Sure Camille Fournier - but tbh requesting minimum formatting diligence (i.e.: spelling, indentation and consistency) on every + line (not the context ones) isn't trivial formatting but just basic sanity. Sadly, many patches don't have it which makes it very hard for people testing patches that haven't been committed yet, testing trunk, etc. Being able to read through those — and related — patches fast and without losing focus because of inconsistent style really helps when testing stuff. It would certainly have helped me when finding bugs like ZOOKEEPER-1805 and ZOOKEEPER-1807 for which I had to read through a lot of code with mixed styles or no style at all
          Hide
          Edward Ribeiro added a comment -

          Camille Fournier Really?! What a pity. I didn't check on mac, but will do as soon as possible (I am not a mac user tbh). Thanks for pointing out. I will reevaluate the tests.

          Show
          Edward Ribeiro added a comment - Camille Fournier Really?! What a pity. I didn't check on mac, but will do as soon as possible (I am not a mac user tbh). Thanks for pointing out. I will reevaluate the tests.
          Hide
          Benjamin Jaton added a comment -

          Patch for 3.4

          Show
          Benjamin Jaton added a comment - Patch for 3.4
          Hide
          Benjamin Jaton added a comment -

          Hello, I adapted the existing patch for 3.4.
          Camille: I'd be interested to know if this works on your machine, there shouldn't be anything OS specific.

          Show
          Benjamin Jaton added a comment - Hello, I adapted the existing patch for 3.4. Camille: I'd be interested to know if this works on your machine, there shouldn't be anything OS specific.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12645651/ZOOKEEPER-1576.patch
          against trunk revision 1595561.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2104//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2104//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2104//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12645651/ZOOKEEPER-1576.patch against trunk revision 1595561. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2104//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2104//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2104//console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12645651/ZOOKEEPER-1576.patch
          against trunk revision 1605517.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2159//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2159//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2159//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12645651/ZOOKEEPER-1576.patch against trunk revision 1605517. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2159//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2159//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2159//console This message is automatically generated.
          Hide
          Camille Fournier added a comment -

          This is failing in the cppunit tests, but since it only changes the java client I'm going to +1 and check it in. Thanks Edward Ribeiro

          Show
          Camille Fournier added a comment - This is failing in the cppunit tests, but since it only changes the java client I'm going to +1 and check it in. Thanks Edward Ribeiro
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in ZooKeeper-trunk #2349 (See https://builds.apache.org/job/ZooKeeper-trunk/2349/)
          ZOOKEEPER-1576. Zookeeper cluster - failed to connect to cluster if one of the
          provided IPs causes java.net.UnknownHostException (Edward Ribeiro via camille) (camille: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1606254)

          • /zookeeper/trunk/CHANGES.txt
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/client/HostProvider.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/client/StaticHostProvider.java
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/StaticHostProviderTest.java
          Show
          Hudson added a comment - SUCCESS: Integrated in ZooKeeper-trunk #2349 (See https://builds.apache.org/job/ZooKeeper-trunk/2349/ ) ZOOKEEPER-1576 . Zookeeper cluster - failed to connect to cluster if one of the provided IPs causes java.net.UnknownHostException (Edward Ribeiro via camille) (camille: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1606254 ) /zookeeper/trunk/CHANGES.txt /zookeeper/trunk/src/java/main/org/apache/zookeeper/client/HostProvider.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/client/StaticHostProvider.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/StaticHostProviderTest.java
          Hide
          Edward Ribeiro added a comment -

          Thanks Camille Fournier! It's very nice to see this patch applied.

          Cheers,
          Eddie

          Show
          Edward Ribeiro added a comment - Thanks Camille Fournier ! It's very nice to see this patch applied. Cheers, Eddie

            People

            • Assignee:
              Edward Ribeiro
              Reporter:
              Tally Tsabary
            • Votes:
              2 Vote for this issue
              Watchers:
              16 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development