ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-596

The last logged zxid calculated by zookeeper servers could cause problems in leader election if data gets corrupted.

    Details

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

      Description

      It is possible that the last loggged zxid as reported by all the servers during leader election is not the last zxid that the server can upload data to. It is very much possible that some transaction or snapshot gets corrupted and the servers actually do not have valid data till last logged zxid. We need to make sure that what the servers report as there last logged zxid, they are able to load data till that zxid.

      1. ZOOKEEPER-596.patch
        65 kB
        Mahadev konar
      2. ZOOKEEPER-596.patch
        65 kB
        Mahadev konar
      3. ZOOKEEPER-596.patch
        64 kB
        Mahadev konar
      4. ZOOKEEPER-596.patch
        65 kB
        Mahadev konar
      5. ZOOKEEPER-596.patch
        57 kB
        Mahadev konar
      6. ZOOKEEPER-596.patch
        53 kB
        Mahadev konar

        Issue Links

          Activity

          Mahadev konar created issue -
          Hide
          Mahadev konar added a comment -

          To elablorate on the problem:
          Currently this is what happens:

          • servers read the last logged zxid from the last log or snapshot and use that in the leader election
          • it is quite possible that something in the logs (some transaction lower than the one reported in leader election) is corrupt and the server does not have sane data till the last reported zxid in leader election
          • this could lead to leader election spinning in a loop if the one elected a leader cannot actually read the data till the reported transaction id.

          The solution is to let the servers upload all the data before they start the leader election and then send the last logged zxid. This way the server can be sure that it has valid data til the last zxid it actually reports in the leader election.

          Show
          Mahadev konar added a comment - To elablorate on the problem: Currently this is what happens: servers read the last logged zxid from the last log or snapshot and use that in the leader election it is quite possible that something in the logs (some transaction lower than the one reported in leader election) is corrupt and the server does not have sane data till the last reported zxid in leader election this could lead to leader election spinning in a loop if the one elected a leader cannot actually read the data till the reported transaction id. The solution is to let the servers upload all the data before they start the leader election and then send the last logged zxid. This way the server can be sure that it has valid data til the last zxid it actually reports in the leader election.
          Hide
          Mahadev konar added a comment -

          a preliminary patch. Does not include tests. Will still be adding tests and cleaning it up.

          this patch adds:

          • a new class ZKDatabase that becomes a top level member of quorumpeer and passed around to all the zookeeper server's created for the life of a quorumPeer
          • the zkdatabase includes all the api's needed to modify/use/load the zk database
            making ZKDatabase as the top level member of quorumpeer allows it to be shared across different instances of zookeeper servers (leader/learner/observer) of an instance of quorumpeer.

          I will be adding javadocs and cleaning up the patch shortly.

          Show
          Mahadev konar added a comment - a preliminary patch. Does not include tests. Will still be adding tests and cleaning it up. this patch adds: a new class ZKDatabase that becomes a top level member of quorumpeer and passed around to all the zookeeper server's created for the life of a quorumPeer the zkdatabase includes all the api's needed to modify/use/load the zk database making ZKDatabase as the top level member of quorumpeer allows it to be shared across different instances of zookeeper servers (leader/learner/observer) of an instance of quorumpeer. I will be adding javadocs and cleaning up the patch shortly.
          Mahadev konar made changes -
          Field Original Value New Value
          Attachment ZOOKEEPER-596.patch [ 12427881 ]
          Hide
          Mahadev konar added a comment -

          after 3 failed attempts, hopefully jira would upload this file.

          a cleaned up patch with comments/javadoc. I am still adding tests. Trying to fix some tests that are failing.

          Show
          Mahadev konar added a comment - after 3 failed attempts, hopefully jira would upload this file. a cleaned up patch with comments/javadoc. I am still adding tests. Trying to fix some tests that are failing.
          Mahadev konar made changes -
          Attachment ZOOKEEPER-596.patch [ 12428010 ]
          Flavio Junqueira made changes -
          Link This issue depends on ZOOKEEPER-629 [ ZOOKEEPER-629 ]
          Hide
          Flavio Junqueira added a comment -

          FLELostMessage test won't work without ZOOKEEPER-629.

          Show
          Flavio Junqueira added a comment - FLELostMessage test won't work without ZOOKEEPER-629 .
          Hide
          Mahadev konar added a comment -

          this patch adds a test case for using memory based zkdatabase most of the time. The test checks to see that a server who has corrupted database cannot join the cluster. I am still thinking if we should just start with a empty database in such a case or just shutdown and let the admin figure it out. This way if the disk is corrupt, an admin can take care of it. For now, I have left the quorumpeer to exit if it finds its database is corrupt on and upto the admin to sanitize the database (by just deleting the database and starting all new on that node).

          this patch includes :

          Show
          Mahadev konar added a comment - this patch adds a test case for using memory based zkdatabase most of the time. The test checks to see that a server who has corrupted database cannot join the cluster. I am still thinking if we should just start with a empty database in such a case or just shutdown and let the admin figure it out. This way if the disk is corrupt, an admin can take care of it. For now, I have left the quorumpeer to exit if it finds its database is corrupt on and upto the admin to sanitize the database (by just deleting the database and starting all new on that node). this patch includes : ZOOKEEPER-629
          Mahadev konar made changes -
          Attachment ZOOKEEPER-596.patch [ 12428210 ]
          Mahadev konar made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Hadoop QA added a comment -

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

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

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

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

          Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/28/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/12428210/ZOOKEEPER-596.patch against trunk revision 891368. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 12 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/28/console This message is automatically generated.
          Mahadev konar made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Hide
          Mahadev konar added a comment -

          looks like the patch got stale.... uploading a new patch that applies to the trunk.

          Show
          Mahadev konar added a comment - looks like the patch got stale.... uploading a new patch that applies to the trunk.
          Mahadev konar made changes -
          Attachment ZOOKEEPER-596.patch [ 12428212 ]
          Mahadev konar made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Hadoop QA added a comment -

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

          +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 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-h7.grid.sp2.yahoo.net/29/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/29/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/29/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/12428212/ZOOKEEPER-596.patch against trunk revision 891368. +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 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-h7.grid.sp2.yahoo.net/29/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/29/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/29/console This message is automatically generated.
          Mahadev konar made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Hide
          Mahadev konar added a comment -

          an updated patch with acess methods for accessing zkdatabase from zookeeperserver. It was accessing members directly which I think is a bad idea.

          Show
          Mahadev konar added a comment - an updated patch with acess methods for accessing zkdatabase from zookeeperserver. It was accessing members directly which I think is a bad idea.
          Mahadev konar made changes -
          Attachment ZOOKEEPER-596.patch [ 12428223 ]
          Mahadev konar made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Hadoop QA added a comment -

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

          +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 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-h7.grid.sp2.yahoo.net/30/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/30/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/30/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/12428223/ZOOKEEPER-596.patch against trunk revision 891368. +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 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-h7.grid.sp2.yahoo.net/30/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/30/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/30/console This message is automatically generated.
          Hide
          Benjamin Reed added a comment -

          looks really good. just a couple of comments:

          1) i think zkDb should be private
          2) you have get/setZkb() in QuorumPeer. you should make it consistent with ZooKeeper and use get/setZKDatabase

          Show
          Benjamin Reed added a comment - looks really good. just a couple of comments: 1) i think zkDb should be private 2) you have get/setZkb() in QuorumPeer. you should make it consistent with ZooKeeper and use get/setZKDatabase
          Hide
          Mahadev konar added a comment -

          patch that addresses ben's comments. Also removed some unnecessary logging from the tests.

          Show
          Mahadev konar added a comment - patch that addresses ben's comments. Also removed some unnecessary logging from the tests.
          Mahadev konar made changes -
          Attachment ZOOKEEPER-596.patch [ 12428346 ]
          Mahadev konar made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Mahadev konar made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Benjamin Reed added a comment -

          +1 (assuming it passes test)

          Show
          Benjamin Reed added a comment - +1 (assuming it passes test)
          Hide
          Hadoop QA added a comment -

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

          +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 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-h8.grid.sp2.yahoo.net/93/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/93/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/93/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/12428346/ZOOKEEPER-596.patch against trunk revision 892001. +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 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-h8.grid.sp2.yahoo.net/93/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/93/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h8.grid.sp2.yahoo.net/93/console This message is automatically generated.
          Hide
          Mahadev konar added a comment -

          I just committed this.

          Show
          Mahadev konar added a comment - I just committed this.
          Mahadev konar made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Hadoop Flags [Reviewed]
          Resolution Fixed [ 1 ]
          Hide
          Hudson added a comment -

          Integrated in ZooKeeper-trunk #634 (See http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/634/)
          . The last logged zxid calculated by zookeeper servers could cause problems in leader election if data gets corrupted. (mahadev)

          Show
          Hudson added a comment - Integrated in ZooKeeper-trunk #634 (See http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/634/ ) . The last logged zxid calculated by zookeeper servers could cause problems in leader election if data gets corrupted. (mahadev)
          Patrick Hunt made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Gavin made changes -
          Link This issue depends on ZOOKEEPER-629 [ ZOOKEEPER-629 ]
          Gavin made changes -
          Link This issue depends upon ZOOKEEPER-629 [ ZOOKEEPER-629 ]

            People

            • Assignee:
              Mahadev konar
              Reporter:
              Mahadev konar
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development