[GitHub] carbondata pull request #1948: [CARBONDATA-2143] Fixed query memory leak iss...

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #1948: [CARBONDATA-2143] Fixed query memory leak iss...

qiuchenjian-2
GitHub user manishgupta88 opened a pull request:

    https://github.com/apache/carbondata/pull/1948

    [CARBONDATA-2143] Fixed query memory leak issue for task failure during initialization of record reader

    **Problem:**
    Whenever a query is executed, in the internalCompute method of CarbonScanRdd class record reader is initialized. A task completion listener is attached to each task after initialization of the record reader.
    During record reader initialization, queryResultIterator is initialized and one blocklet is processed. The blocklet processed will use available unsafe memory.
    Lets say there are 100 columns and 80 columns get the space but there is no space left for the remaining columns to be stored in the unsafe memory. This will result is memory exception and record reader initialization will fail leading to failure in query.
    In the above case the unsafe memory allocated for 80 columns will not be freed and will always remain occupied till the JVM process persists.
   
    **Impact**
    It is memory leak in the system and can lead to query failures for queries executed after one one query fails due to the above reason.
   
    **Solution:**
    Attach the task completion listener before record reader initialization so that if the query fails at the very first instance after using unsafe memory, still that memory will be cleared.
   
     - [ ] Any interfaces changed?
     No
     - [ ] Any backward compatibility impacted?
     No
     - [ ] Document update required?
    No
     - [ ] Testing done
    Manually tested      
     - [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
    NA


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

    $ git pull https://github.com/manishgupta88/carbondata query_memory_leak_fix

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

    https://github.com/apache/carbondata/pull/1948.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 #1948
   
----
commit 2575db6757b21881fe8d7c706f6283f561f43555
Author: m00258959 <manish.gupta@...>
Date:   2018-02-07T06:37:33Z

    Fixed memory leak issue. In case of any task failure unsafe memory for that task in not getting cleared from the executor if the task fails
    during initialization of the record reader

----


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1948: [CARBONDATA-2143] Fixed query memory leak issue for ...

qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1948
 
    Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/2332/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1948: [CARBONDATA-2143] Fixed query memory leak issue for ...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1948
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/3570/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1948: [CARBONDATA-2143] Fixed query memory leak issue for ...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1948
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/3413/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #1948: [CARBONDATA-2143] Fixed query memory leak iss...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1948#discussion_r166841331
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/executor/impl/AbstractQueryExecutor.java ---
    @@ -586,16 +586,27 @@ private int getKeySize(List<QueryDimension> queryDimension,
        */
       @Override public void finish() throws QueryExecutionException {
         CarbonUtil.clearBlockCache(queryProperties.dataBlocks);
    +    UnsafeMemoryManager.INSTANCE.freeMemoryAll(ThreadLocalTaskInfo.getCarbonTaskInfo().getTaskId());
    --- End diff --
   
    Better move down it to after closing of queryIterator


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #1948: [CARBONDATA-2143] Fixed query memory leak iss...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user manishgupta88 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1948#discussion_r166842293
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/executor/impl/AbstractQueryExecutor.java ---
    @@ -586,16 +586,27 @@ private int getKeySize(List<QueryDimension> queryDimension,
        */
       @Override public void finish() throws QueryExecutionException {
         CarbonUtil.clearBlockCache(queryProperties.dataBlocks);
    +    UnsafeMemoryManager.INSTANCE.freeMemoryAll(ThreadLocalTaskInfo.getCarbonTaskInfo().getTaskId());
    --- End diff --
   
    ok


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1948: [CARBONDATA-2143] Fixed query memory leak issue for ...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1948
 
    Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/2337/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1948: [CARBONDATA-2143] Fixed query memory leak issue for ...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1948
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/3575/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1948: [CARBONDATA-2143] Fixed query memory leak issue for ...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1948
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/3425/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1948: [CARBONDATA-2143] Fixed query memory leak issue for ...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1948
 
    LGTM


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #1948: [CARBONDATA-2143] Fixed query memory leak iss...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user asfgit closed the pull request at:

    https://github.com/apache/carbondata/pull/1948


---