[GitHub] carbondata pull request #2588: [CARBONDATA-2807] Fixed data load performance...

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2588#discussion_r208199601
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/loading/sort/unsafe/merger/UnsafeIntermediateMerger.java ---
    @@ -111,18 +108,15 @@ public void addFileToMerge(File sortTempFile) {
       }
     
       public void startFileMergingIfPossible() {
    -    File[] fileList = null;
    -    synchronized (lockObject) {
    -      if (procFiles.size() >= parameters.getNumberOfIntermediateFileToBeMerged()) {
    +    File[] fileList;
    +    if (procFiles.size() >= parameters.getNumberOfIntermediateFileToBeMerged()) {
    +      synchronized (lockObject) {
             fileList = procFiles.toArray(new File[procFiles.size()]);
             this.procFiles = new ArrayList<File>();
    -        if (LOGGER.isDebugEnabled()) {
    -          LOGGER
    -              .debug("Submitting request for intermediate merging no of files: " + fileList.length);
    -        }
           }
    -    }
    -    if (null != fileList) {
    +      if (LOGGER.isDebugEnabled()) {
    +        LOGGER.debug("Sumitting request for intermediate merging no of files: " + fileList.length);
    +      }
           startIntermediateMerging(fileList);
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2588#discussion_r208199760
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/loading/sort/unsafe/merger/UnsafeIntermediateMerger.java ---
    @@ -111,18 +108,15 @@ public void addFileToMerge(File sortTempFile) {
       }
     
       public void startFileMergingIfPossible() {
    -    File[] fileList = null;
    -    synchronized (lockObject) {
    -      if (procFiles.size() >= parameters.getNumberOfIntermediateFileToBeMerged()) {
    +    File[] fileList;
    +    if (procFiles.size() >= parameters.getNumberOfIntermediateFileToBeMerged()) {
    --- End diff --
   
    ok


---

[qiuchenjian-2]

Github user brijoobopanna commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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


---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---