ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-1473

Committed proposal log retains triple the memory it needs to

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: 3.5.2, 3.6.0
    • Component/s: server
    • Labels:
      None

      Description

      ZKDatabase.committedLog retains the past 500 transactions to enable fast catch-up. This works great, but it's using triple the memory it needs to by retaining three copies of the data part of any transaction.

      • The first is in committedLog[i].request.request.hb - a heap-allocated ByteBuffer.
      • The second is in committedLog[i].request.txn.data - a jute-serialised record of the transaction
      • The third is in committedLog[i].packet.data - also jute-serialised, seemingly uninitialised data.

      This means that a ZK-server could be using 1G of memory more than it should be in the worst case. We should use just one copy of the data, even if we really have to refer to it 3 times.

      1. ZOOKEEPER-1473.patch
        2 kB
        Michi Mutsuzaki
      2. ZOOKEEPER-1473.patch
        2 kB
        Thawan Kooburat
      3. ZOOKEEPER-1473.patch
        2 kB
        Thawan Kooburat

        Activity

        Hide
        Thawan Kooburat added a comment -
        • Don't keep pointer to request object since it isn't used for synchronization.
        • Make commitLogCount configurable
        Show
        Thawan Kooburat added a comment - Don't keep pointer to request object since it isn't used for synchronization. Make commitLogCount configurable
        Hide
        Thawan Kooburat added a comment -

        We ran into memory problem in an ensemble with very large znode (~50MB). Hopefully, this will fix most of the problem.

        Show
        Thawan Kooburat added a comment - We ran into memory problem in an ensemble with very large znode (~50MB). Hopefully, this will fix most of the problem.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12578347/ZOOKEEPER-1473.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 passed core unit tests.

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

        Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1455//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1455//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1455//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/12578347/ZOOKEEPER-1473.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 passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1455//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1455//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1455//console This message is automatically generated.
        Hide
        Thawan Kooburat added a comment -
        • Make it possible to disable committedLog (should only be used by the observers only)
        • Remove unused variable related to commitLog
        Show
        Thawan Kooburat added a comment - Make it possible to disable committedLog (should only be used by the observers only) Remove unused variable related to commitLog
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12578459/ZOOKEEPER-1473.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 passed core unit tests.

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

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

        Any chance to get this patch into the next release?

        Show
        Mathias H. added a comment - Any chance to get this patch into the next release?
        Hide
        Hadoop QA added a comment -

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

        +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 patch. The patch command could not apply the patch.

        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1918//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/12578459/ZOOKEEPER-1473.patch against trunk revision 1561672. +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 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1918//console This message is automatically generated.
        Hide
        Raul Gutierrez Segales added a comment -

        Some nits:

        +    public static final String COMMIT_LOG_COUNT = "zoookeeper.commitLogCount";
        

        typo: zookeeper

        +        LOG.info(COMMIT_LOG_COUNT+ "=" + commitLogCount);
        

        can we have string extrapolation instead of concatenation (more readable, easy to do mass changes if you want to change the LOG calls, etc):

        +        LOG.info("{} = {}", COMMIT_LOG_COUNT, commitLogCount);
        
        Show
        Raul Gutierrez Segales added a comment - Some nits: + public static final String COMMIT_LOG_COUNT = "zoookeeper.commitLogCount"; typo: zookeeper + LOG.info(COMMIT_LOG_COUNT+ "=" + commitLogCount); can we have string extrapolation instead of concatenation (more readable, easy to do mass changes if you want to change the LOG calls, etc): + LOG.info("{} = {}", COMMIT_LOG_COUNT, commitLogCount);
        Hide
        Michi Mutsuzaki added a comment -

        +1 I'll update the patch to address Raul's comments and check this in.

        Show
        Michi Mutsuzaki added a comment - +1 I'll update the patch to address Raul's comments and check this in.
        Hide
        Camille Fournier added a comment -

        Is this new config option, commitLogCount, documented anywhere? I think we need some documentation if we're going to add a new config.

        Show
        Camille Fournier added a comment - Is this new config option, commitLogCount, documented anywhere? I think we need some documentation if we're going to add a new config.
        Hide
        Hadoop QA added a comment -

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

        +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/2005//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2005//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2005//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/12637674/ZOOKEEPER-1473.patch against trunk revision 1583083. +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/2005//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2005//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2005//console This message is automatically generated.
        Hide
        Michi Mutsuzaki added a comment -

        Ok, I'm canceling the patch. Thawan, could you add documentation for this config parameter?

        Show
        Michi Mutsuzaki added a comment - Ok, I'm canceling the patch. Thawan, could you add documentation for this config parameter?

          People

          • Assignee:
            Thawan Kooburat
            Reporter:
            Henry Robinson
          • Votes:
            1 Vote for this issue
            Watchers:
            8 Start watching this issue

            Dates

            • Created:
              Updated:

              Development