ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-107 Allow dynamic changes to server cluster membership
  3. ZOOKEEPER-1411

Consolidate membership management, distinguish between static and dynamic configuration parameters

    Details

    • Type: Sub-task Sub-task
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.5.0
    • Component/s: server
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Currently every server has a different static configuration file. This patch distinguishes between dynamic parameters, which are now in a separate "dynamic configuration file", and static parameters which are in the usual file. The config file points to the dynamic config file by specifying "dynamicConfigFile=...". In the first stage (this patch), all cluster membership definitions are in the dynamic config file, but in the future additional parameters may be moved to the dynamic file.

      Backward compatibility makes sure that you can still use a single config file if you'd like. Only when the config is changed (once ZK-107 is in) a dynamic file is automatically created and the necessary parameters are moved to it.

      This patch also moves all membership parsing and management into the QuorumVerifier classes, and removes QuorumPeer.quorumPeers.
      The cluster membership is contained in QuorumPeer.quorumVerifier. QuorumVerifier was expanded and now has methods such as getAllMembers(), getVotingMembers(), getObservingMembers().

      1. ZOOKEEPER-1411-small-fix.patch
        1 kB
        Alexander Shraer
      2. ZOOKEEPER-1411-ver11.patch
        107 kB
        Alexander Shraer
      3. ZOOKEEPER-1411-ver10.patch
        107 kB
        Alexander Shraer
      4. ZOOKEEPER-1411-ver9.patch
        108 kB
        Alexander Shraer
      5. ZOOKEEPER-1411-ver8.patch
        107 kB
        Alexander Shraer
      6. ZOOKEEPER-1411-ver7.patch
        108 kB
        Alexander Shraer
      7. ZOOKEEPER-1411-ver6.patch
        99 kB
        Alexander Shraer
      8. ZOOKEEPER-1411-ver5.patch
        106 kB
        Alexander Shraer
      9. ZOOKEEPER-1411-ver4.patch
        107 kB
        Alexander Shraer
      10. ZOOKEEPER-1411-ver3.patch
        107 kB
        Alexander Shraer
      11. ZOOKEEPER-1411-ver2.patch
        107 kB
        Alexander Shraer
      12. ZOOKEEPER-1411-ver1.patch
        106 kB
        Alexander Shraer

        Issue Links

          Activity

          Hide
          Alexander Shraer added a comment -

          The patch creates the option to have a config file and a separate membership file.

          -----------
          Example of config file:

          tickTime=2000
          dataDir=/home/shralex/zookeeper-3.3.2/zookeeper1
          initLimit=5
          syncLimit=2
          membershipFile=/home/shralex/zookeeper-3.3.2/conf/zoo_replicated1.members

          Example of membership file:

          server.1=localhost:2780:2783:participant;localhost:2791
          server.2=localhost:2781:2784;2792
          server.3=localhost:2782:2785:participant;2793

          -------------

          the cfg file points to the membership file by specifying "membershipFile". The membership file has all information about membership and ports. You must specify all client ports in that file - this info is separated by ";" from the usual server info. The cfg file can still include client ports (or hostname and port, but if given they must be the same as in the membership file.

          Backward compatibility: you can still use one configuration file and that would work fine. But, if the configuration changes (will only happen once ZK-107 is in), the config file is automatically split to two files, and the files are updated accordingly.

          Show
          Alexander Shraer added a comment - The patch creates the option to have a config file and a separate membership file. ----------- Example of config file: tickTime=2000 dataDir=/home/shralex/zookeeper-3.3.2/zookeeper1 initLimit=5 syncLimit=2 membershipFile=/home/shralex/zookeeper-3.3.2/conf/zoo_replicated1.members Example of membership file: server.1=localhost:2780:2783:participant;localhost:2791 server.2=localhost:2781:2784;2792 server.3=localhost:2782:2785:participant;2793 ------------- the cfg file points to the membership file by specifying "membershipFile". The membership file has all information about membership and ports. You must specify all client ports in that file - this info is separated by ";" from the usual server info. The cfg file can still include client ports (or hostname and port, but if given they must be the same as in the membership file. Backward compatibility: you can still use one configuration file and that would work fine. But, if the configuration changes (will only happen once ZK-107 is in), the config file is automatically split to two files, and the files are updated accordingly.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12517700/ZOOKEEPER-1411-ver1.patch
          against trunk revision 1297740.

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

          +1 tests included. The patch appears to include 39 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 appears to introduce 3 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/981//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/981//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/981//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/12517700/ZOOKEEPER-1411-ver1.patch against trunk revision 1297740. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 39 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 appears to introduce 3 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/981//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/981//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/981//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/12517771/ZOOKEEPER-1411-ver2.patch
          against trunk revision 1297740.

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

          +1 tests included. The patch appears to include 39 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 appears to introduce 1 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/983//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/983//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/983//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/12517771/ZOOKEEPER-1411-ver2.patch against trunk revision 1297740. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 39 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 appears to introduce 1 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/983//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/983//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/983//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/12517798/ZOOKEEPER-1411-ver3.patch
          against trunk revision 1297740.

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

          +1 tests included. The patch appears to include 39 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/984//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/984//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/984//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/12517798/ZOOKEEPER-1411-ver3.patch against trunk revision 1297740. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 39 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/984//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/984//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/984//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/12517802/ZOOKEEPER-1411-ver4.patch
          against trunk revision 1297740.

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

          +1 tests included. The patch appears to include 39 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/985//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/985//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/985//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/12517802/ZOOKEEPER-1411-ver4.patch against trunk revision 1297740. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 39 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/985//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/985//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/985//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/12517810/ZOOKEEPER-1411-ver5.patch
          against trunk revision 1297740.

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

          +1 tests included. The patch appears to include 39 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/986//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/986//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/986//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/12517810/ZOOKEEPER-1411-ver5.patch against trunk revision 1297740. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 39 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/986//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/986//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/986//console This message is automatically generated.
          Hide
          Alexander Shraer added a comment -

          Your comments and review of this patch are very appreciated (ZOOKEEPER-1411-ver5.patch).

          One thing we may consider is making QuorumServer a separate class instead of a subclass of QuorumPeer as this subclass has now grown.

          Show
          Alexander Shraer added a comment - Your comments and review of this patch are very appreciated ( ZOOKEEPER-1411 -ver5.patch). One thing we may consider is making QuorumServer a separate class instead of a subclass of QuorumPeer as this subclass has now grown.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12517810/ZOOKEEPER-1411-ver5.patch
          against trunk revision 1302281.

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

          +1 tests included. The patch appears to include 39 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/1000//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1000//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1000//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/12517810/ZOOKEEPER-1411-ver5.patch against trunk revision 1302281. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 39 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/1000//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1000//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1000//console This message is automatically generated.
          Hide
          Benjamin Reed added a comment -

          +1. since this is a rather non-trivial patch can i get another +1?

          Show
          Benjamin Reed added a comment - +1. since this is a rather non-trivial patch can i get another +1?
          Hide
          Rakesh R added a comment -

          Hi Alex, the patch looks good to me. I have couple of suggestions/imprv points. Please go through the following:

          Comment1
          IMO, in QPC.writeMembership(), before going to upgrade the old config file, good to check the status of the membershipFilename operations. If any exception occurred during the creation/writing of the membershipFilename, make more sense to discontinue with the upgradation rather than modifying the cfg file, this would cause inconsistent config and affect ZK startup/restart.

          Comment2

          QuorumPeer.java
          try {
          electionAddr = makeAddr(serverParts[0], Integer.parseInt(serverParts[2]));
          } catch (NumberFormatException e) {
          setType(serverParts[2]);
          }
          if (serverParts.length == 3) return;
          if (electionAddr == null) {
          throw new ConfigException(addressStr + wrongFormat);
          }
          

          Say, occurred NumberFormatException and 'serverParts.length == 3' is satisfied, in this case it will continue to next level with electionAddr as null. IMO, would throw ConfigException for NumberFormatException.

          Hope the combination 'host:port:type' is newly adding to the code base as part of this patch.
          As I know, UDP electionAlg is not in a consistent state and also deprecated this feature. Also in the QPC, there is a check for validating the electionAddr - "Missing election port for server: ". So I feel, its not necessary to consider new combination 'host:port:type'

          Comment3
          QuorumPeer.java
          public String membershipFilename = null;
          public String configFilename = null;
          public boolean membershipBackwardCompatibility = false;

          Is it OK to have private scope?

          Comment4
          Improve the code format, please adjust it a bit by removing the 'tab spaces'

          Show
          Rakesh R added a comment - Hi Alex, the patch looks good to me. I have couple of suggestions/imprv points. Please go through the following: Comment1 IMO, in QPC.writeMembership(), before going to upgrade the old config file, good to check the status of the membershipFilename operations. If any exception occurred during the creation/writing of the membershipFilename, make more sense to discontinue with the upgradation rather than modifying the cfg file, this would cause inconsistent config and affect ZK startup/restart. Comment2 QuorumPeer.java try { electionAddr = makeAddr(serverParts[0], Integer.parseInt(serverParts[2])); } catch (NumberFormatException e) { setType(serverParts[2]); } if (serverParts.length == 3) return; if (electionAddr == null) { throw new ConfigException(addressStr + wrongFormat); } Say, occurred NumberFormatException and 'serverParts.length == 3' is satisfied, in this case it will continue to next level with electionAddr as null. IMO, would throw ConfigException for NumberFormatException. Hope the combination 'host:port:type' is newly adding to the code base as part of this patch. As I know, UDP electionAlg is not in a consistent state and also deprecated this feature. Also in the QPC, there is a check for validating the electionAddr - "Missing election port for server: ". So I feel, its not necessary to consider new combination 'host:port:type' Comment3 QuorumPeer.java public String membershipFilename = null; public String configFilename = null; public boolean membershipBackwardCompatibility = false; Is it OK to have private scope? Comment4 Improve the code format, please adjust it a bit by removing the 'tab spaces'
          Hide
          Alexander Shraer added a comment -

          Hi Rakesh, thanks again for the comments!

          I agree with 1, 3, 4. Regarding comment 2 - this host:port:type is not something I'm adding - AFAIK it is currently permitted by QPC. The check says that if we're using FLE then electionAddr should be defined. I'm not sure whether disallowing host:port:type is something I should do here.

          Maybe someone with more experience with LE can comment on this ?

          Thanks,
          Alex

          Show
          Alexander Shraer added a comment - Hi Rakesh, thanks again for the comments! I agree with 1, 3, 4. Regarding comment 2 - this host:port:type is not something I'm adding - AFAIK it is currently permitted by QPC. The check says that if we're using FLE then electionAddr should be defined. I'm not sure whether disallowing host:port:type is something I should do here. Maybe someone with more experience with LE can comment on this ? Thanks, Alex
          Hide
          Flavio Junqueira added a comment -

          In the way I understand the problem Rakesh is pointing out, say we have NumberFormatException (numeric string is broken) and "serverParts.length == 3". In this situation, this patch will allow the server to continue, and it shouldn't. We should throw and exception in such a run instead of allowing it to continue.

          Show
          Flavio Junqueira added a comment - In the way I understand the problem Rakesh is pointing out, say we have NumberFormatException (numeric string is broken) and "serverParts.length == 3". In this situation, this patch will allow the server to continue, and it shouldn't. We should throw and exception in such a run instead of allowing it to continue.
          Hide
          Alexander Shraer added a comment -

          In the case you describe, when NumberFormatException is thrown, setType is called, and if the parameter is neither "participant" nor "observer", an exception is thrown by setType. If its one of these types, then the function returns and we continue normally knowing that we got host:port:type (which would be legal for the old LE algorithm)

          Show
          Alexander Shraer added a comment - In the case you describe, when NumberFormatException is thrown, setType is called, and if the parameter is neither "participant" nor "observer", an exception is thrown by setType. If its one of these types, then the function returns and we continue normally knowing that we got host:port:type (which would be legal for the old LE algorithm)
          Hide
          Rakesh R added a comment -

          Yeah, I understood setType() is throwing 'ConfigException' otherthan observer/participant and is sufficent to handle the comment#2.

          But still I feel, the existing code base is not considering the 'host:port:type' pattern. If you know, please point me to that code. I am thinking to avoid unwanted conditions.
          The following snippet is from existing QPC and preparing the 'host:port:port' pattern, here its not separately handling the NFE of Integer.parseInt(parts[2]). This will be just propagating to the upper layer and throwing ConfigException.

          } else if (parts.length == 3) {
               InetSocketAddress electionAddr = new InetSocketAddress(parts[0], Integer.parseInt(parts[2]));
               servers.put(Long.valueOf(sid), new QuorumServer(sid, addr, electionAddr));
          
          Show
          Rakesh R added a comment - Yeah, I understood setType() is throwing 'ConfigException' otherthan observer/participant and is sufficent to handle the comment#2. But still I feel, the existing code base is not considering the 'host:port:type' pattern. If you know, please point me to that code. I am thinking to avoid unwanted conditions. The following snippet is from existing QPC and preparing the 'host:port:port' pattern, here its not separately handling the NFE of Integer.parseInt(parts [2] ). This will be just propagating to the upper layer and throwing ConfigException. } else if (parts.length == 3) { InetSocketAddress electionAddr = new InetSocketAddress(parts[0], Integer.parseInt(parts[2])); servers.put(Long.valueOf(sid), new QuorumServer(sid, addr, electionAddr));
          Hide
          Flavio Junqueira added a comment -

          @Alex You're right, I hadn't looked into setType.

          @Rakesh If you check the first comment of this jira, in the sample config, it sounds like Alex is proposing this new pattern: hostname:port:port:type. We currently don't use such a pattern host:port:type, at least according to the documentation and to my knowledge. I'm also not sure I understand the comment about the NFE propagating back in the call chain. Why is that an issue exactly?

          Show
          Flavio Junqueira added a comment - @Alex You're right, I hadn't looked into setType. @Rakesh If you check the first comment of this jira, in the sample config, it sounds like Alex is proposing this new pattern: hostname:port:port:type. We currently don't use such a pattern host:port:type, at least according to the documentation and to my knowledge. I'm also not sure I understand the comment about the NFE propagating back in the call chain. Why is that an issue exactly?
          Hide
          Rakesh R added a comment -

          @Flavio
          Yeah, certainly no serious functional issues and I agree to throw exception back to the call chain. I just shared a thought, to remove the logic(also in the logs and java comments as shown below) of supporting 'host:port:type' pattern from the uploaded patch to make it fairer.
          (FYI, Please see the Alex's comments regarding 'host:port:type')

          QPC.java
          private static final String wrongFormat = " does not have the form server_cofig or server_config;client_config"+
          " where server_config is one of: host:port or host:port:port " +
          " or host:port:port:type or host:port:type and client_config is port or host:port";
          .
          .
          // length 3: host:port:port or host:port:type
          .
          .
          
          Show
          Rakesh R added a comment - @Flavio Yeah, certainly no serious functional issues and I agree to throw exception back to the call chain. I just shared a thought, to remove the logic(also in the logs and java comments as shown below) of supporting 'host:port:type' pattern from the uploaded patch to make it fairer. (FYI, Please see the Alex's comments regarding 'host:port:type') QPC.java private static final String wrongFormat = " does not have the form server_cofig or server_config;client_config"+ " where server_config is one of: host:port or host:port:port " + " or host:port:port:type or host:port:type and client_config is port or host:port"; . . // length 3: host:port:port or host:port:type . .
          Hide
          Flavio Junqueira added a comment -

          Got it, Rakesh. Thanks for the clarification. You're right, we need to remember to remove it when we get rid of UDP leader election.

          Show
          Flavio Junqueira added a comment - Got it, Rakesh. Thanks for the clarification. You're right, we need to remember to remove it when we get rid of UDP leader election.
          Hide
          Alexander Shraer added a comment -

          Thanks for the comments! So if I understand the conclusion correctly we're leaving this for now and will simplify the logic when the old LE code is removed.

          Alex

          Show
          Alexander Shraer added a comment - Thanks for the comments! So if I understand the conclusion correctly we're leaving this for now and will simplify the logic when the old LE code is removed. Alex
          Hide
          Flavio Junqueira added a comment -

          Given that the old LE code is already deprecated (ZOOKEEPER-1153), I would say that it is fine to not take it into account as Rakesh suggests. I don't feel too strongly either way. In the case we keep it, it would be a good idea to create a jira or sub-task for removing it.

          I can't find a jira for removing the UDP-based implementations. If there isn't one, then we need to create one for it too.

          Show
          Flavio Junqueira added a comment - Given that the old LE code is already deprecated ( ZOOKEEPER-1153 ), I would say that it is fine to not take it into account as Rakesh suggests. I don't feel too strongly either way. In the case we keep it, it would be a good idea to create a jira or sub-task for removing it. I can't find a jira for removing the UDP-based implementations. If there isn't one, then we need to create one for it too.
          Hide
          Alexander Shraer added a comment -

          ok, in this case I'll make the change Rakesh suggests and make sure that server_config is either host:port:port or host:port:port:type

          Show
          Alexander Shraer added a comment - ok, in this case I'll make the change Rakesh suggests and make sure that server_config is either host:port:port or host:port:port:type
          Hide
          Alexander Shraer added a comment -

          Rakesh's comments are addressed ZOOKEEPER-1411-ver6.patch

          Show
          Alexander Shraer added a comment - Rakesh's comments are addressed ZOOKEEPER-1411 -ver6.patch
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12519750/ZOOKEEPER-1411-ver6.patch
          against trunk revision 1302736.

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

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

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

          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1009//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/12519750/ZOOKEEPER-1411-ver6.patch against trunk revision 1302736. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 36 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1009//console This message is automatically generated.
          Hide
          Rakesh R added a comment -

          Hi Alex, latest patch looks fine to me.

          Its good to correct the log message(String wrongFormat: as it still contains the info related to 'host:port:type' and would mislead).

          Also, ZK tab policy is 'Spaces only', apply the same to the patch.

          Show
          Rakesh R added a comment - Hi Alex, latest patch looks fine to me. Its good to correct the log message(String wrongFormat: as it still contains the info related to 'host:port:type' and would mislead). Also, ZK tab policy is 'Spaces only', apply the same to the patch.
          Hide
          Alexander Shraer added a comment -

          this should be without tabs and with an updated comment. thanks Rakesh!

          Show
          Alexander Shraer added a comment - this should be without tabs and with an updated comment. thanks Rakesh!
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12519911/ZOOKEEPER-1411-ver7.patch
          against trunk revision 1302736.

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

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

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

          -1 javac. The patch appears to cause tar ant target to fail.

          -1 findbugs. The patch appears to introduce 3 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/1013//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1013//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1013//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/12519911/ZOOKEEPER-1411-ver7.patch against trunk revision 1302736. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 39 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. -1 javac. The patch appears to cause tar ant target to fail. -1 findbugs. The patch appears to introduce 3 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/1013//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1013//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1013//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/12519912/ZOOKEEPER-1411-ver8.patch
          against trunk revision 1302736.

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

          +1 tests included. The patch appears to include 39 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 appears to introduce 3 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/1014//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1014//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1014//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/12519912/ZOOKEEPER-1411-ver8.patch against trunk revision 1302736. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 39 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 appears to introduce 3 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/1014//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1014//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1014//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/12519919/ZOOKEEPER-1411-ver9.patch
          against trunk revision 1302736.

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

          +1 tests included. The patch appears to include 39 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/1015//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1015//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1015//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/12519919/ZOOKEEPER-1411-ver9.patch against trunk revision 1302736. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 39 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/1015//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1015//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1015//console This message is automatically generated.
          Hide
          Rakesh R added a comment -

          +1 Looks good!

          Show
          Rakesh R added a comment - +1 Looks good!
          Hide
          Alexander Shraer added a comment -

          I opened a review board for this:

          https://reviews.apache.org/r/4729/

          Show
          Alexander Shraer added a comment - I opened a review board for this: https://reviews.apache.org/r/4729/
          Hide
          Benjamin Reed added a comment -

          this has been reviewed and out there for a while. if there aren't any objections, i'm committing tonight.

          Show
          Benjamin Reed added a comment - this has been reviewed and out there for a while. if there aren't any objections, i'm committing tonight.
          Hide
          Flavio Junqueira added a comment -

          I have added some comments to the review board. The review is not associated to this jira, so it doesn't show up here. It looks mostly good to me, though.

          Show
          Flavio Junqueira added a comment - I have added some comments to the review board. The review is not associated to this jira, so it doesn't show up here. It looks mostly good to me, though.
          Hide
          Flavio Junqueira added a comment -

          Canceling to sort out comments.

          Show
          Flavio Junqueira added a comment - Canceling to sort out comments.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12523360/ZOOKEEPER-1411-ver10.patch
          against trunk revision 1326029.

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

          +1 tests included. The patch appears to include 39 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/1042//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1042//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1042//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/12523360/ZOOKEEPER-1411-ver10.patch against trunk revision 1326029. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 39 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/1042//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1042//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1042//console This message is automatically generated.
          Hide
          Alexander Shraer added a comment -

          Flavio, thanks a lot for the comments. I uploaded a new patch here and on review board.

          Show
          Alexander Shraer added a comment - Flavio, thanks a lot for the comments. I uploaded a new patch here and on review board.
          Hide
          Flavio Junqueira added a comment -

          Sorry, I didn't do it right. Fixing the assignee.

          Show
          Flavio Junqueira added a comment - Sorry, I didn't do it right. Fixing the assignee.
          Hide
          Flavio Junqueira added a comment -

          I just have one small comment about public methods. Would you mind fixing it, Alex? Other than that, looks good to me.

          Show
          Flavio Junqueira added a comment - I just have one small comment about public methods. Would you mind fixing it, Alex? Other than that, looks good to me.
          Hide
          Alexander Shraer added a comment -

          changed the methods to protected

          Show
          Alexander Shraer added a comment - changed the methods to protected
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12523636/ZOOKEEPER-1411-ver11.patch
          against trunk revision 1326029.

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

          +1 tests included. The patch appears to include 39 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/1047//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1047//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1047//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/12523636/ZOOKEEPER-1411-ver11.patch against trunk revision 1326029. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 39 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/1047//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1047//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1047//console This message is automatically generated.
          Hide
          Benjamin Reed added a comment -

          +1 looks like we are good to go?

          Show
          Benjamin Reed added a comment - +1 looks like we are good to go?
          Hide
          Flavio Junqueira added a comment -

          +1, lgmt. thanks for all changes to the patch, alex.

          Show
          Flavio Junqueira added a comment - +1, lgmt. thanks for all changes to the patch, alex.
          Hide
          Benjamin Reed added a comment -

          Committed revision 1328991.
          thanx alex!

          Show
          Benjamin Reed added a comment - Committed revision 1328991. thanx alex!
          Hide
          Hudson added a comment -

          Integrated in ZooKeeper-trunk #1535 (See https://builds.apache.org/job/ZooKeeper-trunk/1535/)
          ZOOKEEPER-1411. Consolidate membership management, distinguish between static and dynamic configuration parameters (Revision 1328991)

          Result = SUCCESS
          breed : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1328991
          Files :

          • /zookeeper/trunk/CHANGES.txt
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/Leader.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.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/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumHierarchical.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumMaj.java
          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumVerifier.java
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/server/util/DynamicConfigBCTest.java
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/CnxManagerTest.java
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/FLETest.java
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/FLEZeroWeightTest.java
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/HierarchicalQuorumTest.java
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/LENonTerminateTest.java
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/ObserverTest.java
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/QuorumBase.java
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/QuorumUtil.java
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/StandaloneTest.java
          Show
          Hudson added a comment - Integrated in ZooKeeper-trunk #1535 (See https://builds.apache.org/job/ZooKeeper-trunk/1535/ ) ZOOKEEPER-1411 . Consolidate membership management, distinguish between static and dynamic configuration parameters (Revision 1328991) Result = SUCCESS breed : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1328991 Files : /zookeeper/trunk/CHANGES.txt /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/Leader.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.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/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumHierarchical.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumMaj.java /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumVerifier.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/server/util/DynamicConfigBCTest.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/CnxManagerTest.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/FLETest.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/FLEZeroWeightTest.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/HierarchicalQuorumTest.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/LENonTerminateTest.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/ObserverTest.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/QuorumBase.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/QuorumUtil.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/StandaloneTest.java
          Hide
          Hudson added a comment -

          Integrated in ZooKeeper-trunk #1690 (See https://builds.apache.org/job/ZooKeeper-trunk/1690/)
          ZOOKEEPER-1540. ZOOKEEPER-1411 breaks backwards compatibility (Andrew Ferguson via breed) (Revision 1389711)

          Result = SUCCESS
          breed : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1389711
          Files :

          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/server/util/DynamicConfigBCTest.java
          Show
          Hudson added a comment - Integrated in ZooKeeper-trunk #1690 (See https://builds.apache.org/job/ZooKeeper-trunk/1690/ ) ZOOKEEPER-1540 . ZOOKEEPER-1411 breaks backwards compatibility (Andrew Ferguson via breed) (Revision 1389711) Result = SUCCESS breed : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1389711 Files : /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/server/util/DynamicConfigBCTest.java
          Hide
          Michi Mutsuzaki added a comment -

          It looks like this patch broke ZooKeeper-trunk-ibm6.

          https://builds.apache.org/job/ZooKeeper-trunk-ibm6/122/testReport/org.apache.zookeeper.server.util/DynamicConfigBCTest/dynamicConfigBackwardCompatibilityTest/

          This line is throwing an exception:

          zkProp.entrySet().contains("dynamicConfigFile")
          

          To avoid dealing with hashtable, we can do something like this:

          zkProp.getProperty("dynamicConfigFile") != null
          

          (I'm assuming the original patch meant to call containsKey() instead of contains())

          Show
          Michi Mutsuzaki added a comment - It looks like this patch broke ZooKeeper-trunk-ibm6. https://builds.apache.org/job/ZooKeeper-trunk-ibm6/122/testReport/org.apache.zookeeper.server.util/DynamicConfigBCTest/dynamicConfigBackwardCompatibilityTest/ This line is throwing an exception: zkProp.entrySet().contains("dynamicConfigFile") To avoid dealing with hashtable, we can do something like this: zkProp.getProperty("dynamicConfigFile") != null (I'm assuming the original patch meant to call containsKey() instead of contains())
          Hide
          Alexander Shraer added a comment -

          Thanks Michi. I'm attaching a fix.

          Show
          Alexander Shraer added a comment - Thanks Michi. I'm attaching a fix.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12576430/ZOOKEEPER-1411-small-fix.patch
          against trunk revision 1462141.

          +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/1442//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1442//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1442//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/12576430/ZOOKEEPER-1411-small-fix.patch against trunk revision 1462141. +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/1442//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1442//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1442//console This message is automatically generated.
          Hide
          Michi Mutsuzaki added a comment -

          +1. Thanks Alex!

          Show
          Michi Mutsuzaki added a comment - +1. Thanks Alex!
          Hide
          Hudson added a comment -

          Integrated in ZooKeeper-trunk #1889 (See https://builds.apache.org/job/ZooKeeper-trunk/1889/)
          ZOOKEEPER-1411. Consolidate membership management, distinguish between static and dynamic configuration parameters. A small bug fix. (Revision 1463329)

          Result = SUCCESS
          michim : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1463329
          Files :

          • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
          Show
          Hudson added a comment - Integrated in ZooKeeper-trunk #1889 (See https://builds.apache.org/job/ZooKeeper-trunk/1889/ ) ZOOKEEPER-1411 . Consolidate membership management, distinguish between static and dynamic configuration parameters. A small bug fix. (Revision 1463329) Result = SUCCESS michim : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1463329 Files : /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java

            People

            • Assignee:
              Alexander Shraer
              Reporter:
              Alexander Shraer
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development