Uploaded image for project: 'Tajo'
  1. Tajo
  2. TAJO-1954

Fix memory leak in physical operator

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.10.0
    • Fix Version/s: 0.12.0, 0.11.1
    • Component/s: Physical Operator
    • Labels:
      None

      Description

      A physical operator should be closed but some operator does not call to close.

      it's garbage-collected. See http://netty.io/wiki/reference-counted-objects.html for more information.
      Recent access records: 1
      #1:
      	io.netty.buffer.AdvancedLeakAwareByteBuf.nioBuffer(AdvancedLeakAwareByteBuf.java:673)
      	org.apache.tajo.storage.RawFile$RawFileScanner.init(RawFile.java:103)
      	org.apache.tajo.engine.planner.physical.SeqScanExec.initScanner(SeqScanExec.java:286)
      	org.apache.tajo.engine.planner.physical.SeqScanExec.init(SeqScanExec.java:191)
      	org.apache.tajo.engine.planner.physical.UnaryPhysicalExec.init(UnaryPhysicalExec.java:53)
      	org.apache.tajo.engine.planner.physical.ExternalSortExec.init(ExternalSortExec.java:147)
      	org.apache.tajo.engine.planner.physical.UnaryPhysicalExec.init(UnaryPhysicalExec.java:53)
      	org.apache.tajo.engine.planner.physical.ColPartitionStoreExec.init(ColPartitionStoreExec.java:117)
      	org.apache.tajo.worker.TaskImpl.run(TaskImpl.java:403)
      	org.apache.tajo.worker.TaskContainer.run(TaskContainer.java:65)
      	java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
      	java.util.concurrent.FutureTask.run(FutureTask.java:266)
      	java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
      	java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
      	java.lang.Thread.run(Thread.java:745)
      
      2015-10-27 14:10:05,557 ERROR: io.netty.util.ResourceLeakDetector (error(527)) - LEAK: ByteBuf.release() was not called before it's garbage-collected. See http://netty.io/wiki/reference-counted-objects.html for more information.
      Recent access records: 1
      #1:
      	io.netty.buffer.AdvancedLeakAwareByteBuf.nioBuffer(AdvancedLeakAwareByteBuf.java:673)
      	org.apache.tajo.storage.RawFile$RawFileScanner.init(RawFile.java:103)
      	org.apache.tajo.engine.planner.physical.SeqScanExec.initScanner(SeqScanExec.java:286)
      	org.apache.tajo.engine.planner.physical.SeqScanExec.init(SeqScanExec.java:191)
      	org.apache.tajo.engine.planner.physical.UnaryPhysicalExec.init(UnaryPhysicalExec.java:53)
      	org.apache.tajo.engine.planner.physical.ExternalSortExec.init(ExternalSortExec.java:147)
      	org.apache.tajo.engine.planner.physical.UnaryPhysicalExec.init(UnaryPhysicalExec.java:53)
      	org.apache.tajo.engine.planner.physical.AggregationExec.init(AggregationExec.java:53)
      	org.apache.tajo.engine.planner.physical.DistinctGroupbySortAggregationExec.<init>(DistinctGroupbySortAggregationExec.java:73)
      

        Activity

        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user jinossy opened a pull request:

        https://github.com/apache/tajo/pull/838

        TAJO-1954: Fix memory leak in physical operator.

        You can merge this pull request into a Git repository by running:

        $ git pull https://github.com/jinossy/tajo TAJO-1954

        Alternatively you can review and apply these changes as the patch at:

        https://github.com/apache/tajo/pull/838.patch

        To close this pull request, make a commit to your master/trunk branch
        with (at least) the following in the commit message:

        This closes #838


        commit 356a07dcab9d8205c04b9774e6137432c1c3dbc7
        Author: Jinho Kim <jhkim@apache.org>
        Date: 2015-11-02T07:44:51Z

        TAJO-1954: Fix memory leak in physical operator.


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user jinossy opened a pull request: https://github.com/apache/tajo/pull/838 TAJO-1954 : Fix memory leak in physical operator. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jinossy/tajo TAJO-1954 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/838.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #838 commit 356a07dcab9d8205c04b9774e6137432c1c3dbc7 Author: Jinho Kim <jhkim@apache.org> Date: 2015-11-02T07:44:51Z TAJO-1954 : Fix memory leak in physical operator.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jihoonson commented on the pull request:

        https://github.com/apache/tajo/pull/838#issuecomment-152969407

        +1 LGTM!

        Show
        githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/838#issuecomment-152969407 +1 LGTM!
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user asfgit closed the pull request at:

        https://github.com/apache/tajo/pull/838

        Show
        githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/tajo/pull/838
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Tajo-master-CODEGEN-build #578 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/578/)
        TAJO-1954: Fix memory leak in physical operator. (jhkim: rev f4abd4e993f149b8d796da43d2dfdb47d1701dba)

        • CHANGES
        • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/RawFile.java
        • tajo-core-tests/src/test/java/org/apache/tajo/ha/TestHAServiceHDFSImpl.java
        • tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/DistinctGroupbySortAggregationExec.java
        • tajo-core/src/main/java/org/apache/tajo/master/exec/NonForwardQueryResultFileScanner.java
        • tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/SortBasedColPartitionStoreExec.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Tajo-master-CODEGEN-build #578 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/578/ ) TAJO-1954 : Fix memory leak in physical operator. (jhkim: rev f4abd4e993f149b8d796da43d2dfdb47d1701dba) CHANGES tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/RawFile.java tajo-core-tests/src/test/java/org/apache/tajo/ha/TestHAServiceHDFSImpl.java tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/DistinctGroupbySortAggregationExec.java tajo-core/src/main/java/org/apache/tajo/master/exec/NonForwardQueryResultFileScanner.java tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/SortBasedColPartitionStoreExec.java
        Hide
        jhkim Jinho Kim added a comment -

        committed it
        Thanks

        Show
        jhkim Jinho Kim added a comment - committed it Thanks
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-master-build #954 (See https://builds.apache.org/job/Tajo-master-build/954/)
        TAJO-1954: Fix memory leak in physical operator. (jhkim: rev f4abd4e993f149b8d796da43d2dfdb47d1701dba)

        • tajo-core-tests/src/test/java/org/apache/tajo/ha/TestHAServiceHDFSImpl.java
        • tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/DistinctGroupbySortAggregationExec.java
        • CHANGES
        • tajo-core/src/main/java/org/apache/tajo/master/exec/NonForwardQueryResultFileScanner.java
        • tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/SortBasedColPartitionStoreExec.java
        • tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/RawFile.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #954 (See https://builds.apache.org/job/Tajo-master-build/954/ ) TAJO-1954 : Fix memory leak in physical operator. (jhkim: rev f4abd4e993f149b8d796da43d2dfdb47d1701dba) tajo-core-tests/src/test/java/org/apache/tajo/ha/TestHAServiceHDFSImpl.java tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/DistinctGroupbySortAggregationExec.java CHANGES tajo-core/src/main/java/org/apache/tajo/master/exec/NonForwardQueryResultFileScanner.java tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/SortBasedColPartitionStoreExec.java tajo-storage/tajo-storage-hdfs/src/main/java/org/apache/tajo/storage/RawFile.java
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user charsyam commented on a diff in the pull request:

        https://github.com/apache/tajo/pull/838#discussion_r43616625

        — Diff: tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/DistinctGroupbySortAggregationExec.java —
        @@ -172,6 +168,13 @@ public void close() throws IOException {

        @Override
        public void init() throws IOException {
        + super.init();
        +
        + if (aggregateExecs != null) {
        — End diff –

        @jinossy I have a question? Do we need check null check for aggregateExecs?

        Show
        githubbot ASF GitHub Bot added a comment - Github user charsyam commented on a diff in the pull request: https://github.com/apache/tajo/pull/838#discussion_r43616625 — Diff: tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/DistinctGroupbySortAggregationExec.java — @@ -172,6 +168,13 @@ public void close() throws IOException { @Override public void init() throws IOException { + super.init(); + + if (aggregateExecs != null) { — End diff – @jinossy I have a question? Do we need check null check for aggregateExecs?
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jinossy commented on a diff in the pull request:

        https://github.com/apache/tajo/pull/838#discussion_r43707748

        — Diff: tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/DistinctGroupbySortAggregationExec.java —
        @@ -172,6 +168,13 @@ public void close() throws IOException {

        @Override
        public void init() throws IOException {
        + super.init();
        +
        + if (aggregateExecs != null) {
        — End diff –

        @charsyam
        I refer to close() but It seems to the unnecessary checking. Are you working on this operator ?

        Show
        githubbot ASF GitHub Bot added a comment - Github user jinossy commented on a diff in the pull request: https://github.com/apache/tajo/pull/838#discussion_r43707748 — Diff: tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/DistinctGroupbySortAggregationExec.java — @@ -172,6 +168,13 @@ public void close() throws IOException { @Override public void init() throws IOException { + super.init(); + + if (aggregateExecs != null) { — End diff – @charsyam I refer to close() but It seems to the unnecessary checking. Are you working on this operator ?
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user charsyam commented on a diff in the pull request:

        https://github.com/apache/tajo/pull/838#discussion_r43709328

        — Diff: tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/DistinctGroupbySortAggregationExec.java —
        @@ -172,6 +168,13 @@ public void close() throws IOException {

        @Override
        public void init() throws IOException {
        + super.init();
        +
        + if (aggregateExecs != null) {
        — End diff –

        @jinossy I'm sorry.
        I tested below code. and it causes NPE.
        ```java
        List<String> lists = null;
        for(String a : lists) {
        }
        ```

        I think null checking is needed. sorry to disturb you.

        Show
        githubbot ASF GitHub Bot added a comment - Github user charsyam commented on a diff in the pull request: https://github.com/apache/tajo/pull/838#discussion_r43709328 — Diff: tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/DistinctGroupbySortAggregationExec.java — @@ -172,6 +168,13 @@ public void close() throws IOException { @Override public void init() throws IOException { + super.init(); + + if (aggregateExecs != null) { — End diff – @jinossy I'm sorry. I tested below code. and it causes NPE. ```java List<String> lists = null; for(String a : lists) { } ``` I think null checking is needed. sorry to disturb you.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jinossy commented on a diff in the pull request:

        https://github.com/apache/tajo/pull/838#discussion_r43711383

        — Diff: tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/DistinctGroupbySortAggregationExec.java —
        @@ -172,6 +168,13 @@ public void close() throws IOException {

        @Override
        public void init() throws IOException {
        + super.init();
        +
        + if (aggregateExecs != null) {
        — End diff –

        PhysicalPlannerImpl.createSortAggregationDistinctGroupbyExec always set the array, so it can not occurs NPE.
        Next time, I will refactor these code.

        Thanks.

        Show
        githubbot ASF GitHub Bot added a comment - Github user jinossy commented on a diff in the pull request: https://github.com/apache/tajo/pull/838#discussion_r43711383 — Diff: tajo-core/src/main/java/org/apache/tajo/engine/planner/physical/DistinctGroupbySortAggregationExec.java — @@ -172,6 +168,13 @@ public void close() throws IOException { @Override public void init() throws IOException { + super.init(); + + if (aggregateExecs != null) { — End diff – PhysicalPlannerImpl.createSortAggregationDistinctGroupbyExec always set the array, so it can not occurs NPE. Next time, I will refactor these code. Thanks.

          People

          • Assignee:
            jhkim Jinho Kim
            Reporter:
            jhkim Jinho Kim
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development