ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-1694

ZooKeeper Leader sends a repeated NEWLEADER quorum packet to followers

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Duplicate
    • Affects Version/s: 3.4.5, 3.5.0
    • Fix Version/s: 3.5.0
    • Component/s: quorum
    • Labels:
    • Environment:

      Windows, Linux, MacOSX

      Description

      This is at least what it seems in the logs. This also seems to cause a second snapshot in the follower.

      1. ZOOKEEPER-1694.patch
        0.9 kB
        Germán Blanco

        Issue Links

          Activity

          Hide
          Germán Blanco added a comment -

          This patch just removes the lines of code that send the second NEWLEADER quorum packet.
          The first packet is sent with:
          leaderLastZxid = leader.startForwarding(this, updates);
          a few lines above.

          Show
          Germán Blanco added a comment - This patch just removes the lines of code that send the second NEWLEADER quorum packet. The first packet is sent with: leaderLastZxid = leader.startForwarding(this, updates); a few lines above.
          Hide
          Flavio Junqueira added a comment -

          Hi Germán, I think this is a duplicate of ZOOKEEPER-1342. Could you have a look, please?

          Show
          Flavio Junqueira added a comment - Hi Germán, I think this is a duplicate of ZOOKEEPER-1342 . Could you have a look, please?
          Hide
          Flavio Junqueira added a comment -

          Sorry, I meant to say ZOOKEEPER-1324.

          Show
          Flavio Junqueira added a comment - Sorry, I meant to say ZOOKEEPER-1324 .
          Hide
          Germán Blanco added a comment -

          This second try removes the insertion of the quorum packet for NEWLEADER in Leader.startForwarding.
          The packet is sent in LearnerHandler@407, after a check on the version that I don't understand .

          Show
          Germán Blanco added a comment - This second try removes the insertion of the quorum packet for NEWLEADER in Leader.startForwarding. The packet is sent in LearnerHandler@407, after a check on the version that I don't understand .
          Hide
          Germán Blanco added a comment -

          This patch skips adding the NEWLEADER quorum packet in Leader.startForwarding, since it will be added in LearnerHandler a few lines below the call to Leader.startForwarding.
          I don't think that a test case (checking that the server doesn't send a duplicate message) is necessary in this case.
          Could somebody please review this?

          Show
          Germán Blanco added a comment - This patch skips adding the NEWLEADER quorum packet in Leader.startForwarding, since it will be added in LearnerHandler a few lines below the call to Leader.startForwarding. I don't think that a test case (checking that the server doesn't send a duplicate message) is necessary in this case. Could somebody please review this?
          Hide
          Germán Blanco added a comment -

          Sorry, I had the window with "Attach file" opened and I didn't see your comment.
          And sorry again, yes, it is a duplicate. I have been looking at JIRAs these days and I thought that I would have remembered that one, but no, I didn't.
          Should I then close this one?

          Show
          Germán Blanco added a comment - Sorry, I had the window with "Attach file" opened and I didn't see your comment. And sorry again, yes, it is a duplicate. I have been looking at JIRAs these days and I thought that I would have remembered that one, but no, I didn't. Should I then close this one?
          Hide
          Germán Blanco added a comment -

          Flavio, could you please tell me if the attached patch is worth anything?
          It is extremelly simple, but it seems to solve the issue and pass the tests.

          Show
          Germán Blanco added a comment - Flavio, could you please tell me if the attached patch is worth anything? It is extremelly simple, but it seems to solve the issue and pass the tests.
          Hide
          Flavio Junqueira added a comment - - edited

          The point is that we shouldn't treat the same issue in two separate jiras. We already have a patch in ZOOKEEPER-1324 that has been committed to branch 3.4. I had committed it to trunk, but we reverted because of a conflict with a much larger patch, which was about to get in (ZOOKEEPER-107). My suggestion is that you have a look at what has been done in ZOOKEEPER-1324 and confirm that it is the same issue. If it is, we should close this jira as duplicate. Also, please feel free to comment to ZOOKEEPER-1324, feedback is always appreciated.

          Show
          Flavio Junqueira added a comment - - edited The point is that we shouldn't treat the same issue in two separate jiras. We already have a patch in ZOOKEEPER-1324 that has been committed to branch 3.4. I had committed it to trunk, but we reverted because of a conflict with a much larger patch, which was about to get in ( ZOOKEEPER-107 ). My suggestion is that you have a look at what has been done in ZOOKEEPER-1324 and confirm that it is the same issue. If it is, we should close this jira as duplicate. Also, please feel free to comment to ZOOKEEPER-1324 , feedback is always appreciated.
          Hide
          Hadoop QA added a comment -

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

          +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 failed core unit tests.

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

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1463//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1463//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1463//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/12579823/ZOOKEEPER-1694.patch against trunk revision 1463329. +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 failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1463//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1463//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1463//console This message is automatically generated.
          Hide
          Germán Blanco added a comment -

          CAncelling the patch, this is a duplicate issue.

          Show
          Germán Blanco added a comment - CAncelling the patch, this is a duplicate issue.

            People

            • Assignee:
              Unassigned
              Reporter:
              Germán Blanco
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development