[GitHub] carbondata pull request #2606: [CARBONDATA-2817]Thread Leak in Update and in...

[qiuchenjian-2]

GitHub user BJangir opened a pull request:

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

    [CARBONDATA-2817]Thread Leak in Update and in No sort flow

    Issue :-  After Update Command is finished , Loading threads are not getting stopped.
   
    Root Cause :-
    1. In Update flow  DataLoadExecutor 's close method is not called so all Executors services are not closed.
    2. In Exceptions are not handled property in AFDW class's closeExecutorService() which is cuasing Thread leak if Job is killed from SparkUI..  
   
    Solution :-
    1.  Add Task Completion Listener and  call close method of DataLoadExecutor  to it .
    2. Handle Exception in closeExecutor Service so that all Writer steps Threads can be closed.
   
   
   
    Be sure to do all of the following checklist to help us incorporate
    your contribution quickly and easily:
   
     - [ ] Any interfaces changed?
     NO
     - [ ] Any backward compatibility impacted?
     NO
     - [ ] Document update required?
    NO
     - [ ] Testing done
          Verified in cluster manually .  
     - [ ] 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/BJangir/incubator-carbondata update_threadLeak

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

    https://github.com/apache/carbondata/pull/2606.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 #2606
   
----
commit c6c8b484284842169d9239ca403af7d46c67a346
Author: BJangir <babulaljangir111@...>
Date:   2018-08-02T16:21:07Z

    [CARBONDATA-2817]Thread Leak in Update and in No sort flow

----


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2606
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/7748/



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2606#discussion_r207321150
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/loading/steps/CarbonRowDataWriterProcessorStepImpl.java ---
    @@ -80,11 +82,16 @@
     
       private Map<String, LocalDictionaryGenerator> localDictionaryGeneratorMap;
     
    +  private List<CarbonFactHandler> carbonFactHandlers;
    +
    +  ExecutorService executorService = null;
    --- End diff --
   
    make it private


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2606
 
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6496/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2606
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/6137/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2606
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/6153/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2606
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/6155/



---

[qiuchenjian-2]

Github user brijoobopanna commented on the issue:

    https://github.com/apache/carbondata/pull/2606
 
    retest this please



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2606
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/7785/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2606#discussion_r207961430
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/loading/steps/CarbonRowDataWriterProcessorStepImpl.java ---
    @@ -169,24 +171,36 @@ private void doExecute(Iterator<CarbonRowBatch> iterator, int iteratorIndex) thr
           if (rowsNotExist) {
             rowsNotExist = false;
             dataHandler = CarbonFactHandlerFactory.createCarbonFactHandler(model);
    +        this.carbonFactHandlers.add(dataHandler);
             dataHandler.initialise();
           }
           processBatch(iterator.next(), dataHandler, iteratorIndex);
         }
    -    if (!rowsNotExist) {
    -      finish(dataHandler, iteratorIndex);
    +    try {
    +      if (!rowsNotExist) {
    +        finish(dataHandler, iteratorIndex);
    +      }
    +    } finally {
    +      carbonFactHandlers.remove(dataHandler);
         }
    +
    +
       }
     
       @Override protected String getStepName() {
         return "Data Writer";
       }
     
       private void finish(CarbonFactHandler dataHandler, int iteratorIndex) {
    +    CarbonDataWriterException exception = null;
    --- End diff --
   
    Please handle for closeHandler method as it can also throw exception


---

[qiuchenjian-2]

Github user kumarvishal09 commented on the issue:

    https://github.com/apache/carbondata/pull/2606
 
    @BJangir Please handle thread leak scenario for BatchSortWriter in case of any exception. DataWriterBatchProcessorStepImpl.java


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2606
 
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6541/



---

[qiuchenjian-2]

Github user brijoobopanna commented on the issue:

    https://github.com/apache/carbondata/pull/2606
 
    retest this please



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---