[GitHub] carbondata pull request #2752: [WIP]Even after carbon file is copied to targ...

[qiuchenjian-2]

GitHub user Indhumathi27 opened a pull request:

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

    [WIP]Even after carbon file is copied to targetfolder(local/hdfs), carbon files is not deleted from temp directory

    **Problem:**
     Even after carbon file is copied to targetfolder(local/hdfs), carbon files is not deleted from temp   directory.
   
     - [ ] Any interfaces changed?
     
     - [ ] Any backward compatibility impacted?
     
     - [ ] Document update required?
   
     - [ ] Testing done  
           
     - [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
   


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

    $ git pull https://github.com/Indhumathi27/carbondata deletetemp

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

    https://github.com/apache/carbondata/pull/2752.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 #2752
   
----
commit d47beb2990791feb923215e4ab6bd8b8ef005068
Author: Indhumathi27 <indhumathim27@...>
Date:   2018-09-24T06:58:47Z

    Even after carbon file is copied to targetfolder(local/hdfs), carbon files is not deleted from temp directory

----


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2752
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/430/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2752
 
    Build Failed  with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/8678/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2752
 
    Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/608/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2752
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/439/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2752
 
    Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/619/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2752
 
    Build Failed  with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/8689/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2752
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/448/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2752
 
    Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/628/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2752
 
    Build Failed  with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/8698/



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2752#discussion_r219893862
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/store/writer/AbstractFactDataWriter.java ---
    @@ -418,6 +423,24 @@ protected void closeExecutorService() throws CarbonDataWriterException {
         CarbonDataWriterException exception = null;
         try {
           listener.finish();
    +      String tableName = this.model.getTableName();
    +      // Delete CarbonData and CarbonIndex files after copying from temp location
    +      if (null != carbonDataFileTempPath && null == model.getTableSpec().getCarbonTable()
    +          .getBucketingInfo(tableName)) {
    +        String deleteDataAndIndexInTempPath =
    --- End diff --
   
    Why not delete this path recursively so that we do not need the traverse and delete them one by one?
   
    You can find the delete* methods in `CarbonUtil`


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2752#discussion_r219894302
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/store/writer/AbstractFactDataWriter.java ---
    @@ -266,16 +268,18 @@ private void notifyDataMapBlockEnd() {
        * Finish writing current file. It will flush stream, copy and rename temp file to final file
        * @param copyInCurrentThread set to false if want to do data copy in a new thread
        */
    -  protected void commitCurrentFile(boolean copyInCurrentThread) {
    +  protected void commitCurrentFile(boolean copyInCurrentThread) throws IOException {
         notifyDataMapBlockEnd();
         CarbonUtil.closeStreams(this.fileOutputStream, this.fileChannel);
    -    if (!enableDirectlyWriteData2Hdfs) {
    -      if (copyInCurrentThread) {
    -        CarbonUtil.copyCarbonDataFileToCarbonStorePath(carbonDataFileTempPath,
    -            model.getCarbonDataDirectoryPath(), fileSizeInBytes);
    -      } else {
    -        executorServiceSubmitList.add(executorService.submit(
    -            new CompleteHdfsBackendThread(carbonDataFileTempPath)));
    +    if (null != carbonDataFileTempPath && FileFactory.isFileExist(carbonDataFileTempPath)) {
    --- End diff --
   
    Why do we need this judgement? In what circumstance will the temp file do not exist?


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2752
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/455/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2752
 
    Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/635/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2752
 
    Build Failed  with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/8705/



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2752#discussion_r220142943
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/store/writer/AbstractFactDataWriter.java ---
    @@ -418,6 +423,24 @@ protected void closeExecutorService() throws CarbonDataWriterException {
         CarbonDataWriterException exception = null;
         try {
           listener.finish();
    +      String tableName = this.model.getTableName();
    +      // Delete CarbonData and CarbonIndex files after copying from temp location
    +      if (null != carbonDataFileTempPath && null == model.getTableSpec().getCarbonTable()
    +          .getBucketingInfo(tableName)) {
    +        String deleteDataAndIndexInTempPath =
    --- End diff --
   
    Changed the code fix. Delete carbon index and carbon data files in temp location, once it is copied from temp to target path


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2752
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/472/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2752
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/475/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2752
 
    Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/654/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2752
 
    Build Success with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/8722/



---