ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-1096

Leader communication should listen on specified IP, not wildcard address

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 3.3.3, 3.4.0
    • Fix Version/s: 3.4.6, 3.5.0
    • Component/s: server
    • Labels:
      None

      Description

      Server should specify the local address that is used for leader communication and leader election (and not use the default of listening on all interfaces). This is similar to the clientPortAddress parameter that was added a year ago. After reviewing the code, we can't think of a reason why only the port would be used with the wildcard interface, when servers are already connecting specifically to that interface anyway.

      I have submitted a patch, but it does not account for all leader election algorithms.

      Probably should have an option to toggle this, for backwards compatibility, although it seems like it would be a bug if this change broke things.

      There is some more information about making it an option here:
      http://mail-archives.apache.org/mod_mbox/hadoop-zookeeper-dev/201008.mbox/%3CAANLkTikkT97Djqt3CU=H2+7Gnj_4p28hgCXjh345HiyN@mail.gmail.com%3E

      1. ZOOKEEPER-1096.patch
        10 kB
        Germán Blanco
      2. ZOOKEEPER-1096_branch3.4.patch
        10 kB
        Germán Blanco
      3. ZOOKEEPER-1096.patch
        11 kB
        Germán Blanco
      4. ZOOKEEPER-1096_branch3.4.patch
        11 kB
        Germán Blanco
      5. ZOOKEEPER-1096_branch3.4.patch
        4 kB
        Germán Blanco
      6. ZOOKEEPER-1096.patch
        2 kB
        Germán Blanco
      7. ZOOKEEPER-1096.patch
        0.9 kB
        Jared Cantwell
      8. ZOOKEEPER-1096.patch
        1.0 kB
        Jared Cantwell

        Issue Links

          Activity

          Hide
          Hadoop QA added a comment -

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

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

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

          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/317//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/12482702/ZOOKEEPER-1096.patch against trunk revision 1135515. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/317//console This message is automatically generated.
          Hide
          Patrick Hunt added a comment -

          with git use "--no-prefix" when creating the patch.

          Show
          Patrick Hunt added a comment - with git use "--no-prefix" when creating the patch.
          Hide
          Patrick Hunt added a comment -

          Probably should have an option to toggle this, for backwards compatibility, although it seems like it would be a bug if this change broke things.

          I agree on both counts.

          Show
          Patrick Hunt added a comment - Probably should have an option to toggle this, for backwards compatibility, although it seems like it would be a bug if this change broke things. I agree on both counts.
          Hide
          Jared Cantwell added a comment -

          Fixed some prefixes.

          Show
          Jared Cantwell added a comment - Fixed some prefixes.
          Hide
          Hadoop QA added a comment -

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

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +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/318//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/318//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/318//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/12482704/ZOOKEEPER-1096.patch against trunk revision 1135515. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +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/318//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/318//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/318//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/12482704/ZOOKEEPER-1096.patch
          against trunk revision 1135515.

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +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/319//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/319//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/319//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/12482704/ZOOKEEPER-1096.patch against trunk revision 1135515. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +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/319//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/319//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/319//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/12482704/ZOOKEEPER-1096.patch
          against trunk revision 1140017.

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          -1 javadoc. The javadoc tool appears to have generated 1 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/355//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/355//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/355//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/12482704/ZOOKEEPER-1096.patch against trunk revision 1140017. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. -1 javadoc. The javadoc tool appears to have generated 1 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/355//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/355//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/355//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/12482704/ZOOKEEPER-1096.patch
          against trunk revision 1164710.

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +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/492//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/492//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/492//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/12482704/ZOOKEEPER-1096.patch against trunk revision 1164710. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +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/492//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/492//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/492//console This message is automatically generated.
          Hide
          Thomas Koch added a comment -

          Could you please upload the patch to https://reviews.apache.org/r/new/ for review?

          Show
          Thomas Koch added a comment - Could you please upload the patch to https://reviews.apache.org/r/new/ for review?
          Hide
          Patrick Hunt added a comment -

          This is a good idea to fix, it should be enabled by default, but have a safety-valve option (java system property would be fine) to turn it off in the unlikely event user has a problem. I'd document it in the "unsafe" options section of the admin guide.

          Also, why not use getQuorumAddress.toString() rather than format it here?

          Show
          Patrick Hunt added a comment - This is a good idea to fix, it should be enabled by default, but have a safety-valve option (java system property would be fine) to turn it off in the unlikely event user has a problem. I'd document it in the "unsafe" options section of the admin guide. Also, why not use getQuorumAddress.toString() rather than format it here?
          Hide
          Germán Blanco added a comment -

          Wouldn't it make sense to apply this also for the leader election port (at least for Fast Leader Election)?
          And I also would like to have it in 3.4.6, if there is no problem.

          Show
          Germán Blanco added a comment - Wouldn't it make sense to apply this also for the leader election port (at least for Fast Leader Election)? And I also would like to have it in 3.4.6, if there is no problem.
          Hide
          Germán Blanco added a comment -

          I am going to prepare a patch updating also Fast Leader Election. Please let me know if there are is any reason to stop me.

          Show
          Germán Blanco added a comment - I am going to prepare a patch updating also Fast Leader Election. Please let me know if there are is any reason to stop me.
          Hide
          Jared Cantwell added a comment -

          3.5.0 solves some of this issue by correctly binding to the full address in QuorumCxnManager.java. Other places still bind to wildcard though, and that obviously doesn't apply to 3.4.6 I don't think.

          Show
          Jared Cantwell added a comment - 3.5.0 solves some of this issue by correctly binding to the full address in QuorumCxnManager.java. Other places still bind to wildcard though, and that obviously doesn't apply to 3.4.6 I don't think.
          Hide
          Flavio Junqueira added a comment -

          Jared Cantwell, I'm not sure I get your comment. Are you saying that we don't need to consider this issue for 3.4.6?

          Show
          Flavio Junqueira added a comment - Jared Cantwell , I'm not sure I get your comment. Are you saying that we don't need to consider this issue for 3.4.6?
          Hide
          Jared Cantwell added a comment -

          I am simply saying that QuorumCxnManager in 3.5.0 has this issue resolved.

          Show
          Jared Cantwell added a comment - I am simply saying that QuorumCxnManager in 3.5.0 has this issue resolved.
          Hide
          Flavio Junqueira added a comment -

          Jared Cantwell, do you remember by any chance the jira that fixed it?

          german, if it is fixed in QCM, then there is nothing else to fix for FLE, yes? We possibly need to port the QCM changes to 3.4.6, though.

          Show
          Flavio Junqueira added a comment - Jared Cantwell , do you remember by any chance the jira that fixed it? german, if it is fixed in QCM, then there is nothing else to fix for FLE, yes? We possibly need to port the QCM changes to 3.4.6, though.
          Show
          Jared Cantwell added a comment - It was changed as part of ZOOKEEPER-1411 : http://svn.apache.org/viewvc?view=revision&revision=1328991 http://svn.apache.org/viewvc/zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java?r1=1328991&r2=1328990&pathrev=1328991
          Hide
          Germán Blanco added a comment -

          yes, right, the fact that it was corrected for leader election in 3.5.0 was also commented in the description of ZOOKEEPER-1711. The previous patch for trunk didn't work any more, so I have updated it including also the java properties configuration proposed by Patrick Hunt.
          I will also prepare the patch for branch 3.4, including the change in QuorumCnxManager.

          Show
          Germán Blanco added a comment - yes, right, the fact that it was corrected for leader election in 3.5.0 was also commented in the description of ZOOKEEPER-1711 . The previous patch for trunk didn't work any more, so I have updated it including also the java properties configuration proposed by Patrick Hunt. I will also prepare the patch for branch 3.4, including the change in QuorumCnxManager.
          Hide
          Germán Blanco added a comment -

          Both patches are uploaded now.
          Would it make sense to have the java properties configuration value for leader election in 3.5.0?

          Show
          Germán Blanco added a comment - Both patches are uploaded now. Would it make sense to have the java properties configuration value for leader election in 3.5.0?
          Hide
          Flavio Junqueira added a comment -

          Thanks for the patch. Here are a few comments:

          1. Please add documentation.
          2. Why are we using system properties and not the config file to set this up?
          3. Please with parenthesis with if/else blocks, even if there is a single statement currently.
          4. I don't find it very elegant to catch an exception to determine that we need to return false. Also, I'd rather not catch a generic exception, but instead catch the precise exception we are expecting in the case the property is not there.
          Show
          Flavio Junqueira added a comment - Thanks for the patch. Here are a few comments: Please add documentation. Why are we using system properties and not the config file to set this up? Please with parenthesis with if/else blocks, even if there is a single statement currently. I don't find it very elegant to catch an exception to determine that we need to return false. Also, I'd rather not catch a generic exception, but instead catch the precise exception we are expecting in the case the property is not there.
          Hide
          Flavio Junqueira added a comment -

          Also, I don't understand this question:

          Would it make sense to have the java properties configuration value for leader election in 3.5.0?

          Could you clarify, German, please?

          Show
          Flavio Junqueira added a comment - Also, I don't understand this question: Would it make sense to have the java properties configuration value for leader election in 3.5.0? Could you clarify, German, please?
          Hide
          Germán Blanco added a comment -

          Thanks a lot for your comments Flavio!

          1. Please add documentation.

          I will as soon as I learn how to do it. If you could please point me to e.g. a paragraph that tells me how to do it or a JIRA that includes documentation that will make things faster, otherwise I will look for it.

          2. Why are we using system properties and not the config file to set this up?

          Because of my understanding of Patrick Hunt's comment above: comment-13137642. But I don't mind to change it to the config file if that is better.

          3. Please with parenthesis with if/else blocks, even if there is a single statement currently.

          OK

          4. I don't find it very elegant to catch an exception to determine that we need to return false. Also, I'd rather not catch a generic exception, but instead catch the precise exception we are expecting in the case the property is not there.

          That is just copied from getSnapCount and getGlobalOutstandingLimit already there in the same class. I don't mind changing it, but then I guess it would make sense to change it also in the other two methods, right?

          Would it make sense to have the java properties configuration value for leader election in 3.5.0?

          The feature of listening only on the configured IP address is present already for 3.5.0 in trunk, although only for the leader election port. However, in this case, there is no configuration option. That is, the server only listens on the IP that is in the configuration file and there is no way to set an option so that it listens on all IPs for that port which is what versions 3.4.5 and before used to do. Should there be a configuration option to "listenOnAllIPs" for 3.5.0 as the patch proposes for branch 3.4?

          Show
          Germán Blanco added a comment - Thanks a lot for your comments Flavio! 1. Please add documentation. I will as soon as I learn how to do it. If you could please point me to e.g. a paragraph that tells me how to do it or a JIRA that includes documentation that will make things faster, otherwise I will look for it. 2. Why are we using system properties and not the config file to set this up? Because of my understanding of Patrick Hunt's comment above: comment-13137642 . But I don't mind to change it to the config file if that is better. 3. Please with parenthesis with if/else blocks, even if there is a single statement currently. OK 4. I don't find it very elegant to catch an exception to determine that we need to return false. Also, I'd rather not catch a generic exception, but instead catch the precise exception we are expecting in the case the property is not there. That is just copied from getSnapCount and getGlobalOutstandingLimit already there in the same class. I don't mind changing it, but then I guess it would make sense to change it also in the other two methods, right? Would it make sense to have the java properties configuration value for leader election in 3.5.0? The feature of listening only on the configured IP address is present already for 3.5.0 in trunk, although only for the leader election port. However, in this case, there is no configuration option. That is, the server only listens on the IP that is in the configuration file and there is no way to set an option so that it listens on all IPs for that port which is what versions 3.4.5 and before used to do. Should there be a configuration option to "listenOnAllIPs" for 3.5.0 as the patch proposes for branch 3.4?
          Hide
          Germán Blanco added a comment -

          Ok, I know how to update the documentation now. By the way, the forrest task seems to be broken for Windows. I will ask in the developers mailing list.

          Show
          Germán Blanco added a comment - Ok, I know how to update the documentation now. By the way, the forrest task seems to be broken for Windows. I will ask in the developers mailing list.
          Hide
          Marshall McMullen added a comment -

          +1 for using the config file to configure the ports and any behavior thereof as this matches the way we configure client ports and is a lot easier to use and deploy on a mass scale IMO than java properties.

          Show
          Marshall McMullen added a comment - +1 for using the config file to configure the ports and any behavior thereof as this matches the way we configure client ports and is a lot easier to use and deploy on a mass scale IMO than java properties.
          Hide
          Germán Blanco added a comment -

          New patch with: Brackets added. Doc updated. Configuration in configuration file. Configuration parameter also in trunk for FLE.
          Please review.

          Show
          Germán Blanco added a comment - New patch with: Brackets added. Doc updated. Configuration in configuration file. Configuration parameter also in trunk for FLE. Please review.
          Hide
          Marshall McMullen added a comment -

          This latest version looks really good. I especially like that it's configured via the configuration file. Nicely done.

          +1 from me.

          Show
          Marshall McMullen added a comment - This latest version looks really good. I especially like that it's configured via the configuration file. Nicely done. +1 from me.
          Hide
          Germán Blanco added a comment -

          Manual steps to verify the patch:

          • Documentation generated with two new parameters in "Unsafe options" list.
          • Server started with fleListenOnAllIPs set to true, the server listens on all IPs.
          • Server started with fleListenOnAllIPs not set or set to false, the server listens only on one IP.
          • Leader started with zabListenOnAllIPs set to true, it listens on all IPs.
          • Leader started with zabListenOnAllIPs not set or set to false, it listens only on one IP.
            The log shows address "0.0.0.0:XXXX" when listening on all IPs and the specific IP address when it is listening just on one.
          Show
          Germán Blanco added a comment - Manual steps to verify the patch: Documentation generated with two new parameters in "Unsafe options" list. Server started with fleListenOnAllIPs set to true, the server listens on all IPs. Server started with fleListenOnAllIPs not set or set to false, the server listens only on one IP. Leader started with zabListenOnAllIPs set to true, it listens on all IPs. Leader started with zabListenOnAllIPs not set or set to false, it listens only on one IP. The log shows address "0.0.0.0:XXXX" when listening on all IPs and the specific IP address when it is listening just on one.
          Hide
          Flavio Junqueira added a comment -

          I'm sorry for coming up with another question, but I was wondering about the reason for having separate properties for fle and zab. Do you expect anyone to set the properties differently for fle and zab? I would say that it is ok to have a single property, but I'd like to hear your opinion.

          Show
          Flavio Junqueira added a comment - I'm sorry for coming up with another question, but I was wondering about the reason for having separate properties for fle and zab. Do you expect anyone to set the properties differently for fle and zab? I would say that it is ok to have a single property, but I'd like to hear your opinion.
          Hide
          Marshall McMullen added a comment -

          Flavio brings up a great point. It seems like most would want to change both fle and zab rather than one or the other separately. I like the idea of making this a single property.

          Show
          Marshall McMullen added a comment - Flavio brings up a great point. It seems like most would want to change both fle and zab rather than one or the other separately. I like the idea of making this a single property.
          Hide
          Germán Blanco added a comment -

          The patches now have only one configuration parameter for both leader election and broadcast protocol.

          Show
          Germán Blanco added a comment - The patches now have only one configuration parameter for both leader election and broadcast protocol.
          Hide
          Hadoop QA added a comment -

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

          +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/1586//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1586//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1586//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/12603793/ZOOKEEPER-1096.patch against trunk revision 1524275. +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/1586//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1586//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1586//console This message is automatically generated.
          Hide
          Flavio Junqueira added a comment -

          +1, thanks, Germán.

          Show
          Flavio Junqueira added a comment - +1, thanks, Germán.
          Hide
          Flavio Junqueira added a comment -

          Branch 3.4 Revision: 1526313
          Trunk Revision: 1526317

          Show
          Flavio Junqueira added a comment - Branch 3.4 Revision: 1526313 Trunk Revision: 1526317
          Hide
          Marshall McMullen added a comment -

          Thanks Germán and Flavio! Really nice job finishing this one up.

          Show
          Marshall McMullen added a comment - Thanks Germán and Flavio! Really nice job finishing this one up.
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in ZooKeeper-trunk #2068 (See https://builds.apache.org/job/ZooKeeper-trunk/2068/)
          ZOOKEEPER-1096. Leader communication should listen on specified IP, not wildcard address (Jared Cantwell, German Blanco via fpj) (fpj: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1526317)

          • /zookeeper/trunk/CHANGES.txt
          • /zookeeper/trunk/src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/Leader.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/LENonTerminateTest.java
          Show
          Hudson added a comment - SUCCESS: Integrated in ZooKeeper-trunk #2068 (See https://builds.apache.org/job/ZooKeeper-trunk/2068/ ) ZOOKEEPER-1096 . Leader communication should listen on specified IP, not wildcard address (Jared Cantwell, German Blanco via fpj) (fpj: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1526317 ) /zookeeper/trunk/CHANGES.txt /zookeeper/trunk/src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/Leader.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/LENonTerminateTest.java
          Hide
          Flavio Junqueira added a comment -

          Closing issues after releasing 3.4.6.

          Show
          Flavio Junqueira added a comment - Closing issues after releasing 3.4.6.

            People

            • Assignee:
              Germán Blanco
              Reporter:
              Jared Cantwell
            • Votes:
              4 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development