Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.4.3
    • Fix Version/s: 3.3.6, 3.4.4, 3.5.0
    • Component/s: contrib-bindings
    • Labels:
      None
    • Environment:

      RHEL 6.0, self-built from 3.3.3 sources

      Description

      I'm seeing a memory leakage when using the "aget" method.

      It leaks tuples and dicts, both containing "stats".

      1. zktest3.py
        0.6 kB
        johan rydberg
      2. zktest4.py
        0.5 kB
        johan rydberg
      3. pyzk-mem-leak-fix.diff
        2 kB
        Kapil Thangavelu
      4. zk.patch
        2 kB
        André Cruz

        Activity

        Hide
        Hudson added a comment -

        Integrated in ZooKeeper-trunk #1590 (See https://builds.apache.org/job/ZooKeeper-trunk/1590/)
        ZOOKEEPER-1431. zkpython async calls leak memory (Kapil Thangavelu and Andre Cruz via henryr) (Revision 1351541)

        Result = SUCCESS
        henry : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1351541
        Files :

        • /zookeeper/trunk/CHANGES.txt
        • /zookeeper/trunk/src/contrib/zkpython/src/c/zookeeper.c
        Show
        Hudson added a comment - Integrated in ZooKeeper-trunk #1590 (See https://builds.apache.org/job/ZooKeeper-trunk/1590/ ) ZOOKEEPER-1431 . zkpython async calls leak memory (Kapil Thangavelu and Andre Cruz via henryr) (Revision 1351541) Result = SUCCESS henry : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1351541 Files : /zookeeper/trunk/CHANGES.txt /zookeeper/trunk/src/contrib/zkpython/src/c/zookeeper.c
        Hide
        Henry Robinson added a comment -

        Just committed to 3.3, 3.4 and 3.5. Thanks Kapil and Andre!

        Show
        Henry Robinson added a comment - Just committed to 3.3, 3.4 and 3.5. Thanks Kapil and Andre!
        Hide
        Henry Robinson added a comment -

        Patch looks good, I'll commit shortly.

        Show
        Henry Robinson added a comment - Patch looks good, I'll commit shortly.
        Hide
        André Cruz added a comment -

        I would also like to know. Tests were not included because it's not easy to test for a memory leak.

        Show
        André Cruz added a comment - I would also like to know. Tests were not included because it's not easy to test for a memory leak.
        Hide
        Paul Giannaros added a comment -

        Is this patch going to be applied soon? Are you waiting for justification on why it doesn't include new tests?

        Show
        Paul Giannaros added a comment - Is this patch going to be applied soon? Are you waiting for justification on why it doesn't include new tests?
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12526345/zk.patch
        against trunk revision 1336467.

        +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/1070//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1070//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1070//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/12526345/zk.patch against trunk revision 1336467. +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/1070//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1070//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1070//console This message is automatically generated.
        Hide
        André Cruz added a comment -

        Corrected patch that applies to trunk and 3.4 branch.

        Show
        André Cruz added a comment - Corrected patch that applies to trunk and 3.4 branch.
        Hide
        Patrick Hunt added a comment -

        Kapil the patch is failing to apply. Please update it and re-attach, thanks. (you might check if the patch applies cleanly to both 3.4 and trunk branches, it might be that it only applies to one, in which case you would need to submit two patches, one for each branch). thanks!

        Show
        Patrick Hunt added a comment - Kapil the patch is failing to apply. Please update it and re-attach, thanks. (you might check if the patch applies cleanly to both 3.4 and trunk branches, it might be that it only applies to one, in which case you would need to submit two patches, one for each branch). thanks!
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12519666/pyzk-mem-leak-fix.diff
        against trunk revision 1330043.

        +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/1050//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/12519666/pyzk-mem-leak-fix.diff against trunk revision 1330043. +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/1050//console This message is automatically generated.
        Hide
        Kapil Thangavelu added a comment -

        Also wanted to note fwiw, we're going to apply the patch i attached to this issue, in the zookeeper 3.3.5 pkg distributed in ubuntu 12.04/precise.

        Show
        Kapil Thangavelu added a comment - Also wanted to note fwiw, we're going to apply the patch i attached to this issue, in the zookeeper 3.3.5 pkg distributed in ubuntu 12.04/precise.
        Hide
        Kapil Thangavelu added a comment -

        Noting the problem is generic, it also applies to all extant versions 3.3x and 3.4

        Show
        Kapil Thangavelu added a comment - Noting the problem is generic, it also applies to all extant versions 3.3x and 3.4
        Hide
        Kapil Thangavelu added a comment -

        Fix for async python api memory leaks.

        Show
        Kapil Thangavelu added a comment - Fix for async python api memory leaks.
        Hide
        Kapil Thangavelu added a comment -

        This bug applies to ALL async api calls with python.

        Show
        Kapil Thangavelu added a comment - This bug applies to ALL async api calls with python.
        Hide
        johan rydberg added a comment -

        https://github.com/apache/zookeeper/blob/trunk/src/contrib/zkpython/src/c/zookeeper.c#L487

        Shouldn't arglist be decref'ed after the call?

        That drops my tuple count from ~55k to ~5k after a zktest3.py run.

        Show
        johan rydberg added a comment - https://github.com/apache/zookeeper/blob/trunk/src/contrib/zkpython/src/c/zookeeper.c#L487 Shouldn't arglist be decref'ed after the call? That drops my tuple count from ~55k to ~5k after a zktest3.py run.
        Hide
        johan rydberg added a comment -

        Here's two test cases reproducing the error.

        Both use pykeeper (ease_install pykeeper) to set things up, but then use the zkpython bindings to do the actual testing.

        zktest3.py leaks, zktest4.py does not.

        Show
        johan rydberg added a comment - Here's two test cases reproducing the error. Both use pykeeper (ease_install pykeeper) to set things up, but then use the zkpython bindings to do the actual testing. zktest3.py leaks, zktest4.py does not.

          People

          • Assignee:
            Kapil Thangavelu
            Reporter:
            johan rydberg
          • Votes:
            1 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 1h
              1h
              Remaining:
              Remaining Estimate - 1h
              1h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development