ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-481

Add lastMessageSent to QuorumCnxManager

    Details

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

      Description

      Currently we rely on TCP for reliable delivery of FLE messages. However, as we concurrently drop and create new connections, it is possible that a message is sent but never received. With this patch, cnx manager keeps a list of last messages sent, and resends the last one sent. Receiving multiples copies is harmless.

      1. ZOOKEEPER-481.patch
        19 kB
        Flavio Junqueira
      2. ZOOKEEPER-481-branch3.2.patch
        19 kB
        Flavio Junqueira
      3. ZOOKEEPER-481.patch
        19 kB
        Flavio Junqueira
      4. ZOOKEEPER-481-branch3.2.patch
        19 kB
        Flavio Junqueira
      5. ZOOKEEPER-481.patch
        19 kB
        Flavio Junqueira
      6. ZOOKEEPER-481.patch
        18 kB
        Flavio Junqueira
      7. ZOOKEEPER-481.patch
        11 kB
        Flavio Junqueira

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in ZooKeeper-trunk #404 (See http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/404/)
          . Add lastMessageSent to QuorumCnxManager. (flavio via mahadev)

          Show
          Hudson added a comment - Integrated in ZooKeeper-trunk #404 (See http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/404/ ) . Add lastMessageSent to QuorumCnxManager. (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 -

          I have made a small modification to the port assignment in the test, so I'm resubmitting just to make sure that hudson likes it.

          Show
          Flavio Junqueira added a comment - I have made a small modification to the port assignment in the test, so I'm resubmitting just to make sure that hudson likes it.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12415199/ZOOKEEPER-481-branch3.2.patch
          against trunk revision 799741.

          +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/165/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/165/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/165/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/12415199/ZOOKEEPER-481-branch3.2.patch against trunk revision 799741. +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/165/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/165/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/165/console This message is automatically generated.
          Hide
          Flavio Junqueira added a comment -

          3.2 version of the same patch (only the test should be different).

          Show
          Flavio Junqueira added a comment - 3.2 version of the same patch (only the test should be different).
          Hide
          Flavio Junqueira added a comment -

          Fixed findbugs issues, will upload a 3.2 version once +1.

          Show
          Flavio Junqueira added a comment - Fixed findbugs issues, will upload a 3.2 version once +1.
          Hide
          Mahadev konar added a comment -

          flavio,
          I just ran ant test-patch since hudson wastn willing to run it for us.

          I fopt a -1 with

          3 new findbugs warnings generated. Can you please try running findbugs and see where the issues are?

          Show
          Mahadev konar added a comment - flavio, I just ran ant test-patch since hudson wastn willing to run it for us. I fopt a -1 with 3 new findbugs warnings generated. Can you please try running findbugs and see where the issues are?
          Hide
          Flavio Junqueira added a comment -

          Uploading a patch that applies to 3.2. Unit tests pass for me.

          Show
          Flavio Junqueira added a comment - Uploading a patch that applies to 3.2. Unit tests pass for me.
          Hide
          Todd Greenwood-Geer added a comment -

          ZOOKEEPER-481.patch fails to build when applied to branch-3.2 due to a missing file, PortAssignment.java. PortAssignment.java was added in ZOOKEEPER-473.patch. I have not investigated further dependencies, so I do not know if 481 depends on other files in 473. Patrick suggested that we may need separate patches for branch-3.2 and trunk.

          Show
          Todd Greenwood-Geer added a comment - ZOOKEEPER-481 .patch fails to build when applied to branch-3.2 due to a missing file, PortAssignment.java. PortAssignment.java was added in ZOOKEEPER-473 .patch. I have not investigated further dependencies, so I do not know if 481 depends on other files in 473. Patrick suggested that we may need separate patches for branch-3.2 and trunk.
          Hide
          Flavio Junqueira added a comment -

          Adding comment to the test as requested. Thanks, Ben.

          Show
          Flavio Junqueira added a comment - Adding comment to the test as requested. 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
          Flavio Junqueira added a comment -

          Adding unit test.

          Show
          Flavio Junqueira added a comment - Adding unit test.
          Hide
          Flavio Junqueira added a comment -

          No, ZOOKEEPER-22 is about client-to-server communication, and this one is affects server-to-server communication in particular during leade election.

          Show
          Flavio Junqueira added a comment - No, ZOOKEEPER-22 is about client-to-server communication, and this one is affects server-to-server communication in particular during leade election.
          Hide
          Henry Robinson added a comment -

          Does this change have any impact on ZOOKEEPER-22?

          Show
          Henry Robinson added a comment - Does this change have any impact on ZOOKEEPER-22 ?

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development