ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-1087

ForceSync VM arguement not working when set to "no"

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 3.3.3
    • Fix Version/s: 3.3.4, 3.4.0
    • Component/s: scripts
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Release Note:
      Respect the "zookeeper.forceSync" system property.

      Description

      Cannot use forceSync=no to asynchronously write transaction logs. This is a critical bug, please address it ASAP. More details:

      The class org.apache.zookeeper.server.persistence.FileTxnLog initializes forceSync property in a static block. However, the static variable is defined after the static block with a default value of true. Therefore, the value of the variable can never be false. Please move the declaration of the variable before the static block.

      1. ZOOKEEPER-1087.patch
        2 kB
        Nate Putnam
      2. ZOOKEEPER-1087.patch
        2 kB
        Nate Putnam
      3. ZOOKEEPER-1087.patch
        3 kB
        Nate Putnam

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open In Progress In Progress
        40m 22s 1 Nate Putnam 17/Jun/11 20:57
        In Progress In Progress Patch Available Patch Available
        2m 12s 1 Nate Putnam 17/Jun/11 20:59
        Patch Available Patch Available Open Open
        3h 5m 2 Nate Putnam 17/Jun/11 21:12
        Open Open Patch Available Patch Available
        10d 20h 14m 2 Nate Putnam 17/Jun/11 21:12
        Patch Available Patch Available Resolved Resolved
        3d 9h 21m 1 Benjamin Reed 21/Jun/11 06:34
        Resolved Resolved Closed Closed
        155d 13h 47m 1 Mahadev konar 23/Nov/11 19:22
        Mahadev konar made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Patrick Hunt made changes -
        Fix Version/s 3.4.0 [ 12314469 ]
        Fix Version/s 3.2.3 [ 12314847 ]
        Hide
        Hudson added a comment -

        Integrated in ZooKeeper-trunk #1218 (See https://builds.apache.org/job/ZooKeeper-trunk/1218/)
        ZOOKEEPER-1087. ForceSync VM arguement not working when set to "no"

        breed : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1137864
        Files :

        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java
        • /zookeeper/trunk/CHANGES.txt
        • /zookeeper/trunk/src/java/test/org/apache/zookeeper/server/ZooKeeperServerTest.java
        Show
        Hudson added a comment - Integrated in ZooKeeper-trunk #1218 (See https://builds.apache.org/job/ZooKeeper-trunk/1218/ ) ZOOKEEPER-1087 . ForceSync VM arguement not working when set to "no" breed : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1137864 Files : /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java /zookeeper/trunk/CHANGES.txt /zookeeper/trunk/src/java/test/org/apache/zookeeper/server/ZooKeeperServerTest.java
        Benjamin Reed made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Benjamin Reed added a comment -

        thanx nate!
        Committed revision 1137863. (3.3 branch)
        Committed revision 1137864. (trunk)

        Show
        Benjamin Reed added a comment - thanx nate! Committed revision 1137863. (3.3 branch) Committed revision 1137864. (trunk)
        Hide
        Mahadev konar added a comment -

        ben, can you commit this? should this go into 3.4 as well?

        Show
        Mahadev konar added a comment - ben, can you commit this? should this go into 3.4 as well?
        Benjamin Reed made changes -
        Fix Version/s 3.2.3 [ 12314847 ]
        Fix Version/s 3.4.0 [ 12314469 ]
        Affects Version/s 3.3.2 [ 12315108 ]
        Component/s scripts [ 12312384 ]
        Component/s server [ 12312382 ]
        Hide
        Benjamin Reed added a comment -

        +1 looks good

        Show
        Benjamin Reed added a comment - +1 looks good
        Benjamin Reed made changes -
        Hadoop Flags [Reviewed]
        Hide
        Benjamin Reed added a comment -

        +1 looks good

        Show
        Benjamin Reed added a comment - +1 looks good
        Hide
        Hadoop QA added a comment -

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

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

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

        Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/331//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/331//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/331//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/12483167/ZOOKEEPER-1087.patch against trunk revision 1136740. +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 passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/331//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/331//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/331//console This message is automatically generated.
        Nate Putnam made changes -
        Attachment ZOOKEEPER-1087.patch [ 12483167 ]
        Hide
        Nate Putnam added a comment -

        Fix import for Assert.assertTrue/False

        Show
        Nate Putnam added a comment - Fix import for Assert.assertTrue/False
        Hide
        Patrick Hunt added a comment -

        you want something like:

        import org.junit.Assert;
        ....
        Assert.assertTrue(tmpDir.mkdirs());

        Show
        Patrick Hunt added a comment - you want something like: import org.junit.Assert; .... Assert.assertTrue(tmpDir.mkdirs());
        Hide
        Hadoop QA added a comment -

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

        +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 patch appears to cause tar ant target to fail.

        +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/329//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/329//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/329//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/12482991/ZOOKEEPER-1087.patch against trunk revision 1136740. +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 patch appears to cause tar ant target to fail. +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/329//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/329//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/329//console This message is automatically generated.
        Nate Putnam made changes -
        Attachment ZOOKEEPER-1087.patch [ 12482991 ]
        Hide
        Nate Putnam added a comment -

        I'm not sure what is going on with the import of the assertion methods. The other tests in that class use assertEquals without any issue but Jenkins was complaining about assertTrue,assertFalse not being found. I don't see an explicit import for assertEquals so I'm not sure what's going on. Runs fine for me in IntelliJ and command line. Attaching the latest patch.

        Show
        Nate Putnam added a comment - I'm not sure what is going on with the import of the assertion methods. The other tests in that class use assertEquals without any issue but Jenkins was complaining about assertTrue,assertFalse not being found. I don't see an explicit import for assertEquals so I'm not sure what's going on. Runs fine for me in IntelliJ and command line. Attaching the latest patch.
        Hide
        Patrick Hunt added a comment -

        looks good, a couple suggestions:

        1) we should be able to make this variable final, no?
        2) in testForceSyncDefaultDisabled put a try/finally that resets the forceSync back to it's original value
        (you have no guarantee on the order of junit runs tests, if it runs this first your other test will fail)
        3) we don't use import * (+import static org.junit.Assert.* eclipse can do this for you automatically (organize imports)

        Show
        Patrick Hunt added a comment - looks good, a couple suggestions: 1) we should be able to make this variable final, no? 2) in testForceSyncDefaultDisabled put a try/finally that resets the forceSync back to it's original value (you have no guarantee on the order of junit runs tests, if it runs this first your other test will fail) 3) we don't use import * (+import static org.junit.Assert.* eclipse can do this for you automatically (organize imports)
        Hide
        Hadoop QA added a comment -

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

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

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

        Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/328//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/328//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/328//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/12482981/ZOOKEEPER-1087.patch against trunk revision 1136740. +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 passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/328//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/328//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/328//console This message is automatically generated.
        Hide
        Patrick Hunt added a comment -

        fyi no need to delete the old patch(es), just upload a new version with the same name and jira handles it properly (incl seeing history)

        Show
        Patrick Hunt added a comment - fyi no need to delete the old patch(es), just upload a new version with the same name and jira handles it properly (incl seeing history)
        Nate Putnam made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Nate Putnam added a comment -

        Add import to try and fix Jenkins.

        Show
        Nate Putnam added a comment - Add import to try and fix Jenkins.
        Nate Putnam made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Nate Putnam made changes -
        Attachment ZOOKEEPER-1087.patch [ 12482981 ]
        Nate Putnam made changes -
        Attachment ZOOKEEPER-1087.patch [ 12482979 ]
        Hide
        Hadoop QA added a comment -

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

        +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 patch appears to cause tar ant target to fail.

        +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/327//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/327//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/327//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/12482979/ZOOKEEPER-1087.patch against trunk revision 1136740. +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 patch appears to cause tar ant target to fail. +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/327//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/327//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/327//console This message is automatically generated.
        Nate Putnam made changes -
        Status In Progress [ 3 ] Patch Available [ 10002 ]
        Release Note Respect the "zookeeper.forceSync" system property.
        Nate Putnam made changes -
        Attachment ZOOKEEPER-1087.patch [ 12482979 ]
        Nate Putnam made changes -
        Attachment ZOOKEEPER-1087.patch [ 12482948 ]
        Nate Putnam made changes -
        Status Open [ 1 ] In Progress [ 3 ]
        Hide
        Nate Putnam added a comment -

        Totally. New patch attached.

        Show
        Nate Putnam added a comment - Totally. New patch attached.
        Patrick Hunt made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Hide
        Patrick Hunt added a comment -

        it's weird that this failed (not the author's fault afaict), probably some burp on jenkins, we can try it again.

        however, before we do that, can you see a way to add a test for this? In particular we don't want to regress in future.

        Show
        Patrick Hunt added a comment - it's weird that this failed (not the author's fault afaict), probably some burp on jenkins, we can try it again. however, before we do that, can you see a way to add a test for this? In particular we don't want to regress in future.
        Patrick Hunt made changes -
        Assignee Nate Putnam [ nputnam ]
        Patrick Hunt made changes -
        Fix Version/s 3.3.4 [ 12316276 ]
        Patrick Hunt made changes -
        Fix Version/s 3.3.2 [ 12315108 ]
        Fix Version/s 3.3.3 [ 12315482 ]
        Fix Version/s 3.4.0 [ 12314469 ]
        Hide
        Hadoop QA added a comment -

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

        +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/326//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/326//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/326//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/12482948/ZOOKEEPER-1087.patch against trunk revision 1136740. +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/326//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/326//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/326//console This message is automatically generated.
        Nate Putnam made changes -
        Attachment ZOOKEEPER-1087.patch [ 12482948 ]
        Nate Putnam made changes -
        Field Original Value New Value
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Nate Putnam added a comment -

        Moved initialization from static block to global scope.

        Show
        Nate Putnam added a comment - Moved initialization from static block to global scope.
        Ankit Patel created issue -

          People

          • Assignee:
            Nate Putnam
            Reporter:
            Ankit Patel
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 5m
              5m
              Remaining:
              Remaining Estimate - 5m
              5m
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development