ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-480

FLE should perform leader check when node is not leading and add vote of follower

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.2.0
    • Fix Version/s: 3.2.1, 3.3.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      As a server may join leader election while others have already elected a leader, it is necessary that a server handles some special cases of leader election when notifications are from servers that are either LEADING or FOLLOWING. In such special cases, we check if we have received a message from the leader to declare a leader elected. This check does not consider the case that the process performing the check might be a recently elected leader, and consequently the check fails.

      This patch also adds a new case, which corresponds to adding a vote to recvset when the notification is from a process LEADING or FOLLOWING. This fixes the case raised in ZOOKEEPER-475.

      1. ZOOKEEPER-480.patch
        3 kB
        Flavio Junqueira
      2. ZOOKEEPER-480.patch
        10 kB
        Flavio Junqueira
      3. ZOOKEEPER-480.patch
        11 kB
        Flavio Junqueira
      4. ZOOKEEPER-480-3.2branch.patch
        11 kB
        Flavio Junqueira
      5. ZOOKEEPER-480.patch
        11 kB
        Flavio Junqueira
      6. ZOOKEEPER-480.patch
        10 kB
        Flavio Junqueira
      7. ZOOKEEPER-480-3.2branch.patch
        10 kB
        Flavio Junqueira

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in ZooKeeper-trunk #405 (See http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/405/)
          . FLE should perform leader check when node is not leading and add vote of follower (flavio via mahadev)

          Show
          Hudson added a comment - Integrated in ZooKeeper-trunk #405 (See http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/405/ ) . FLE should perform leader check when node is not leading and add vote of follower (flavio via mahadev)
          Hide
          Mahadev konar added a comment -

          I just committed this. thanks flavio.

          Show
          Mahadev konar added a comment - I just committed this. thanks flavio.
          Hide
          Flavio Junqueira added a comment -

          Patch for 3.2 branch.

          Show
          Flavio Junqueira added a comment - Patch for 3.2 branch.
          Hide
          Mahadev konar added a comment -

          flavio, canm you upload 3.2 patch as well?

          Show
          Mahadev konar added a comment - flavio, canm you upload 3.2 patch as well?
          Hide
          Flavio Junqueira added a comment -

          Regenerated patch to apply to trunk. Will be posting a new patch for 3.2 trunk shortly.

          Show
          Flavio Junqueira added a comment - Regenerated patch to apply to trunk. Will be posting a new patch for 3.2 trunk shortly.
          Hide
          Mahadev konar added a comment -

          flavio, the patch does not apply any more after ZOOKEEPER-481 commit. can you please update the patch?

          Show
          Mahadev konar added a comment - flavio, the patch does not apply any more after ZOOKEEPER-481 commit. can you please update the patch?
          Hide
          Flavio Junqueira added a comment -

          Small modification to port assignment in the test, re-running to make sure that still passes all tests.

          Show
          Flavio Junqueira added a comment - Small modification to port assignment in the test, re-running to make sure that still passes all tests.
          Hide
          Flavio Junqueira added a comment -

          Uploading the 3.2 version of the patch.

          Show
          Flavio Junqueira added a comment - Uploading the 3.2 version of the patch.
          Hide
          Flavio Junqueira added a comment -

          Added the comment Ben suggested. Thanks, Ben.

          Show
          Flavio Junqueira added a comment - Added the comment Ben suggested. Thanks, Ben.
          Hide
          Benjamin Reed added a comment -

          looks great flavio. i think i figured out how the test works. do you mind putting a comment into the test to state your strategy for posterity?

          Show
          Benjamin Reed added a comment - looks great flavio. i think i figured out how the test works. do you mind putting a comment into the test to state your strategy for posterity?
          Hide
          Hadoop QA added a comment -

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

          +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 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: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/159/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/159/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/159/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/12414159/ZOOKEEPER-480.patch against trunk revision 798038. +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 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: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/159/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/159/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/159/console This message is automatically generated.
          Hide
          Flavio Junqueira added a comment -
          • Adds a case to leader election to recover from a lost message when one single follower elects a leader, but the leader missed the new vote from the follower. This is not a really critical case because in practice the servers would just move to a new round of leader election, and the likelihood that this keeps happening forever is small;
          • It adds a unit test that basically mocks a server to force a particular sequence and interleaving of messages;
          • It makes some methods and variables public in QuorumCnxManager to enable the unit test to access them directly.
          Show
          Flavio Junqueira added a comment - Adds a case to leader election to recover from a lost message when one single follower elects a leader, but the leader missed the new vote from the follower. This is not a really critical case because in practice the servers would just move to a new round of leader election, and the likelihood that this keeps happening forever is small; It adds a unit test that basically mocks a server to force a particular sequence and interleaving of messages; It makes some methods and variables public in QuorumCnxManager to enable the unit test to access them directly.

            People

            • Assignee:
              Flavio Junqueira
              Reporter:
              Flavio Junqueira
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development