ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-1783

Distinguish initial configuration from first established configuration

    Details

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

      Description

      We need a way to distinguish an initial config of a server and an initial config of a running ensemble (before any reconfigs happen). Currently both have version 0.

      The version of a config increases with each reconfiguration, so the problem is just with the initial config.

      1. ZOOKEEPER-1783.patch
        2 kB
        Alexander Shraer
      2. ZOOKEEPER-1783-ver1.patch
        5 kB
        Alexander Shraer
      3. ZOOKEEPER-1783-ver2.patch
        7 kB
        Alexander Shraer
      4. ZOOKEEPER-1783-ver3.patch
        7 kB
        Alexander Shraer
      5. ZOOKEEPER-1783-ver4.patch
        9 kB
        Alexander Shraer
      6. ZOOKEEPER-1783-ver5.patch
        11 kB
        Alexander Shraer
      7. ZOOKEEPER-1783-ver6.patch
        13 kB
        Alexander Shraer
      8. ZOOKEEPER-1783-ver7.patch
        14 kB
        Alexander Shraer
      9. ZOOKEEPER-1783-ver8.patch
        14 kB
        Alexander Shraer
      10. ZOOKEEPER-1783-ver9.patch
        16 kB
        Alexander Shraer

        Issue Links

          Activity

          Hide
          Alexander Shraer added a comment -

          any suggestions welcome

          Show
          Alexander Shraer added a comment - any suggestions welcome
          Hide
          Alexander Shraer added a comment -

          Can be reproduced using the test in 1691

          Show
          Alexander Shraer added a comment - Can be reproduced using the test in 1691
          Hide
          Hadoop QA added a comment -

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

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

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

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

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

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

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

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

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

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1669//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1669//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1669//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/12607526/ZOOKEEPER-1783.patch against trunk revision 1530166. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1669//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1669//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1669//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/12607771/ZOOKEEPER-1783-ver1.patch
          against trunk revision 1530809.

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

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

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

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

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

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

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

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

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1674//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1674//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1674//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/12607771/ZOOKEEPER-1783-ver1.patch against trunk revision 1530809. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1674//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1674//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1674//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/12607949/ZOOKEEPER-1783-ver2.patch
          against trunk revision 1531061.

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

          +1 tests included. The patch appears to include 6 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/1678//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1678//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1678//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/12607949/ZOOKEEPER-1783-ver2.patch against trunk revision 1531061. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 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/1678//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1678//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1678//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/12608152/ZOOKEEPER-1783-ver3.patch
          against trunk revision 1531444.

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

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

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

          +1 tests included. The patch appears to include 9 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/1688//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1688//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1688//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/12608161/ZOOKEEPER-1783-ver4.patch against trunk revision 1531444. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 9 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/1688//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1688//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1688//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/12608164/ZOOKEEPER-1783-ver4.patch
          against trunk revision 1531444.

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

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

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

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

          can you describe the problem a bit more? is the scenario that you have an initial ensemble and then you are adding a new server and it also has a configuration version 0?

          Show
          Benjamin Reed added a comment - can you describe the problem a bit more? is the scenario that you have an initial ensemble and then you are adding a new server and it also has a configuration version 0?
          Hide
          Alexander Shraer added a comment -

          Exactly. When specifying config files the user doesn't specify the version and we treat this as version = 0. The version is then set to the zxid of the reconfig operations. So suppose that you have an ensemble that never reconfigured before. Its version is 0. And now you start a new server that wants to join, it also has version = 0. We need that new server to adopt the current config from the leader. But since both have version = 0 this doesn't happen.

          The main fix is just a few lines to Leader.java + a new test (to check that now the version of an established ensemble is > 0) and similar change to C tests that were checking for version = 0. Everything else in the patch was needed because now some of the new reconfig-related code (writing dynamic files, potentially restarting leader election) is activated even if we don't invoke reconfig. Also in context that was never tested before like with read-only mode being enabled.

          Show
          Alexander Shraer added a comment - Exactly. When specifying config files the user doesn't specify the version and we treat this as version = 0. The version is then set to the zxid of the reconfig operations. So suppose that you have an ensemble that never reconfigured before. Its version is 0. And now you start a new server that wants to join, it also has version = 0. We need that new server to adopt the current config from the leader. But since both have version = 0 this doesn't happen. The main fix is just a few lines to Leader.java + a new test (to check that now the version of an established ensemble is > 0) and similar change to C tests that were checking for version = 0. Everything else in the patch was needed because now some of the new reconfig-related code (writing dynamic files, potentially restarting leader election) is activated even if we don't invoke reconfig. Also in context that was never tested before like with read-only mode being enabled.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12608180/ZOOKEEPER-1783-ver5.patch
          against trunk revision 1531444.

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

          +1 tests included. The patch appears to include 9 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/1691//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1691//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1691//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/12608180/ZOOKEEPER-1783-ver5.patch against trunk revision 1531444. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 9 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/1691//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1691//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1691//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/12608184/ZOOKEEPER-1783-ver6.patch
          against trunk revision 1531444.

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

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

          Benjamin Reed can you please take a look ?

          Show
          Alexander Shraer added a comment - Benjamin Reed can you please take a look ?
          Hide
          Benjamin Reed added a comment -

          in Leader.java you create a new quorum verifier and update it with a the current zxid. couldn't you have just updated the current quorum verifier.

          also can you explain the changes in QuorumPeer. i'm not sure how they are related.

          Show
          Benjamin Reed added a comment - in Leader.java you create a new quorum verifier and update it with a the current zxid. couldn't you have just updated the current quorum verifier. also can you explain the changes in QuorumPeer. i'm not sure how they are related.
          Hide
          Alexander Shraer added a comment -

          Hi Ben,

          Our invariant is that the current quorum verifier has been committed / went through consensus, and I didn't want to break it by assigning it a version that didn't go through consensus. This restriction means that in the NEWLEADER message we'd have to send a config with version 0 and then have follower/observer/learner/leader make a check (when UPTODATE arrives) that detects version = 0 and replaces it with the version from the NEWLEADER version. It seemed that this would require more changes in more places, this is why I chose to do it like in the patch – basically use the fact that lastProposedQV is an uncommitted qv we're sending to everyone, and they already know how to handle it.

          Changes to QuorumPeer:

          1. Replicating the logic we have for restarting leader election to correctly work when read-only mode is enabled.

          roZkMgr.start();
          + reconfigFlagClear();
          + if (shuttingDownLE)

          { + shuttingDownLE = false; + startLeaderElection(); + }

          This is just duplicate of what we do for when read-only option is disabled. Without this, restarting leader election doesn't work correctly (it keeps spinning since shuttingDownLE = true. I noticed it because read-only test was failing with my changes to Leader.java. After this changed work, I changed FastLeaderElection not to restart in this case because the quorum verifier is identical besides the version. The change above is still needed though.

          The other two changes are there because I was hitting NullPointerException from different tests that now had to invoke processReconfig (to write dynamic config files with new version) but didn't initialize dynamic config related parameters correctly. These changes are just performing sanity checks before using variables that may be null.

          Show
          Alexander Shraer added a comment - Hi Ben, Our invariant is that the current quorum verifier has been committed / went through consensus, and I didn't want to break it by assigning it a version that didn't go through consensus. This restriction means that in the NEWLEADER message we'd have to send a config with version 0 and then have follower/observer/learner/leader make a check (when UPTODATE arrives) that detects version = 0 and replaces it with the version from the NEWLEADER version. It seemed that this would require more changes in more places, this is why I chose to do it like in the patch – basically use the fact that lastProposedQV is an uncommitted qv we're sending to everyone, and they already know how to handle it. Changes to QuorumPeer: 1. Replicating the logic we have for restarting leader election to correctly work when read-only mode is enabled. roZkMgr.start(); + reconfigFlagClear(); + if (shuttingDownLE) { + shuttingDownLE = false; + startLeaderElection(); + } This is just duplicate of what we do for when read-only option is disabled. Without this, restarting leader election doesn't work correctly (it keeps spinning since shuttingDownLE = true. I noticed it because read-only test was failing with my changes to Leader.java. After this changed work, I changed FastLeaderElection not to restart in this case because the quorum verifier is identical besides the version. The change above is still needed though. The other two changes are there because I was hitting NullPointerException from different tests that now had to invoke processReconfig (to write dynamic config files with new version) but didn't initialize dynamic config related parameters correctly. These changes are just performing sanity checks before using variables that may be null.
          Hide
          Alexander Shraer added a comment -

          Added a long comment in Leader.java about why we're doing what we're doing there, removed some tabs, and changed the check "version > 0" to "version == 0x100000000L" in ReconfigTest.

          Show
          Alexander Shraer added a comment - Added a long comment in Leader.java about why we're doing what we're doing there, removed some tabs, and changed the check "version > 0" to "version == 0x100000000L" in ReconfigTest.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12608431/ZOOKEEPER-1783-ver7.patch
          against trunk revision 1532152.

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

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

          looks great alex! sorry to nit pick but you have at least one stray tab, a couple of lines with an indentation of 2 spaces, and a couple with what looks like 8 spaces ( or it might be a tab)

          Show
          Benjamin Reed added a comment - looks great alex! sorry to nit pick but you have at least one stray tab, a couple of lines with an indentation of 2 spaces, and a couple with what looks like 8 spaces ( or it might be a tab)
          Hide
          Benjamin Reed added a comment -

          +1 looks great thanx for sticking with it.

          Show
          Benjamin Reed added a comment - +1 looks great thanx for sticking with it.
          Hide
          Alexander Shraer added a comment -

          thanks for reviewing!!

          Show
          Alexander Shraer added a comment - thanks for reviewing!!
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12608650/ZOOKEEPER-1783-ver8.patch
          against trunk revision 1532152.

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

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

          same as before, with a small change to ReconfigRecoveryTest. Since the version of the current config is now 100000000 instead of 0, the version of a new config should be something larger than 100000000.

          Show
          Alexander Shraer added a comment - same as before, with a small change to ReconfigRecoveryTest. Since the version of the current config is now 100000000 instead of 0, the version of a new config should be something larger than 100000000.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12609330/ZOOKEEPER-1783-ver9.patch
          against trunk revision 1533161.

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

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

          SUCCESS: Integrated in ZooKeeper-trunk #2113 (See https://builds.apache.org/job/ZooKeeper-trunk/2113/)
          ZOOKEEPER-1783. Distinguish initial configuration from first established configuration (shralex via breed) (breed: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1539529)

          • /zookeeper/trunk/CHANGES.txt
          • /zookeeper/trunk/src/c/tests/TestReconfigServer.cc
          • /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/QuorumPeer.java
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/server/quorum/ReconfigRecoveryTest.java
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/ReconfigTest.java
          Show
          Hudson added a comment - SUCCESS: Integrated in ZooKeeper-trunk #2113 (See https://builds.apache.org/job/ZooKeeper-trunk/2113/ ) ZOOKEEPER-1783 . Distinguish initial configuration from first established configuration (shralex via breed) (breed: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1539529 ) /zookeeper/trunk/CHANGES.txt /zookeeper/trunk/src/c/tests/TestReconfigServer.cc /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/QuorumPeer.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/server/quorum/ReconfigRecoveryTest.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/ReconfigTest.java

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development