You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by zzcclp <gi...@git.apache.org> on 2018/03/08 08:53:05 UTC

[GitHub] carbondata pull request #2045: [CARBONDATA-2230]Add a path into table path t...

GitHub user zzcclp opened a pull request:

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

    [CARBONDATA-2230]Add a path into table path to store lock files and delete useless segment lock files before loading

    After PR1984[https://github.com/apache/carbondata/pull/1984] merged, it doesn't delete the lock files when unlock, there are many useless lock files in table path, especially segment lock files, they grow after every batch loading.
    
    Solution :
    1. add a child path into table path, called Locks, all lock files will be stored in this path;
    2. Before loading, get all useless segment lock files and delete them, because just segment lock files will grow, other lock files dosen't grow.
    
    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
            Please provide details on 
            - Whether new unit test cases have been added or why no new tests are required?
            - How it is tested? Please attach test report.
            - Is it a performance related change? Please attach the performance test report.
            - Any additional information to help reviewers in testing this change.
        Test manually    
    
     - [ ] 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/zzcclp/carbondata CARBONDATA-2230

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

    https://github.com/apache/carbondata/pull/2045.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 #2045
    
----
commit 79f2c6db97f91f7609eb9cc432a561815bdefc9b
Author: Zhang Zhichao <44...@...>
Date:   2018-03-08T07:18:09Z

    [CARBONDATA-2230]Add a path into table path to store lock files and delete useless segment lock files before loading
    
    After PR1984[https://github.com/apache/carbondata/pull/1984] merged, it doesn't delete the lock files when unlock, there are many useless lock files in table path, especially segment lock files, they grow after every batch loading.
    
    Solution :
    1. add a child path into table path, called Locks, all lock files will be stored in this path;
    2. Before loading, get all useless segment lock files and delete them, because just segment lock files will grow, other lock files dosen't grow.

----


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2045: [CARBONDATA-2230]Add a path into table path t...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2045#discussion_r173716693
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/statusmanager/SegmentStatusManager.java ---
    @@ -834,6 +836,40 @@ private static void writeLoadMetadata(AbsoluteTableIdentifier identifier,
         }
       }
     
    +  /**
    +   * Currently the segment lock files are not deleted immediately when unlock,
    +   * so it needs to delete expired lock files before delete loads.
    +   */
    +  private static void deleteExpiredSegmentLockFiles(CarbonTable carbonTable) {
    --- End diff --
    
    It's better to move this function to CarbonLockUtil.
    Done


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on the issue:

    https://github.com/apache/carbondata/pull/2045
  
    retest sdv please


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on the issue:

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


---

[GitHub] carbondata pull request #2045: [CARBONDATA-2230]Add a path into table path t...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on the issue:

    https://github.com/apache/carbondata/pull/2045
  
    @ravipesala @jackylk please help to review. thanks.


---

[GitHub] carbondata pull request #2045: [CARBONDATA-2230]Add a path into table path t...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2045#discussion_r174233092
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/locks/CarbonLockUtil.java ---
    @@ -107,4 +114,36 @@ public static int getLockProperty(String property, int defaultValue) {
         }
       }
     
    +  /**
    +   * Currently the segment lock files are not deleted immediately when unlock,
    +   * so it needs to delete expired lock files before delete loads.
    +   */
    +  public static void deleteExpiredSegmentLockFiles(CarbonTable carbonTable) {
    +    LoadMetadataDetails[] details =
    --- End diff --
    
    I think you can better list the segment lock files from locks folder and check the modified time of it and remove . Otherwise if you do as per tablestatus then namenode calls would be many.


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on the issue:

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


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2045: [CARBONDATA-2230]Add a path into table path t...

Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2045#discussion_r175009993
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/locks/CarbonLockUtil.java ---
    @@ -107,4 +114,36 @@ public static int getLockProperty(String property, int defaultValue) {
         }
       }
     
    +  /**
    +   * Currently the segment lock files are not deleted immediately when unlock,
    +   * so it needs to delete expired lock files before delete loads.
    +   */
    +  public static void deleteExpiredSegmentLockFiles(CarbonTable carbonTable) {
    +    LoadMetadataDetails[] details =
    +        SegmentStatusManager.readLoadMetadata(carbonTable.getMetadataPath());
    +    if (details != null && details.length > 0) {
    +      AbsoluteTableIdentifier absoluteTableIdentifier = carbonTable.getAbsoluteTableIdentifier();
    +      long segmentLockFilesPreservTime =
    +          CarbonProperties.getInstance().getSegmentLockFilesPreserveHours();
    +      long currTime = System.currentTimeMillis();
    +      for (LoadMetadataDetails oneRow : details) {
    +        if (oneRow.getVisibility().equalsIgnoreCase("false") ||
    +            SegmentStatus.SUCCESS == oneRow.getSegmentStatus() ||
    +            SegmentStatus.LOAD_FAILURE == oneRow.getSegmentStatus() ||
    +            SegmentStatus.LOAD_PARTIAL_SUCCESS == oneRow.getSegmentStatus() ||
    +            SegmentStatus.COMPACTED == oneRow.getSegmentStatus()) {
    +          String location = CarbonTablePath
    +              .getSegmentLockFilePath(absoluteTableIdentifier.getTablePath(),
    +                  oneRow.getLoadName());
    +          CarbonFile carbonFile =
    +              FileFactory.getCarbonFile(location, FileFactory.getFileType(location));
    +          if (carbonFile.exists()) {
    --- End diff --
    
    This operation will call HDFS NameNode, so if there are 1000 lock files, this operation will be for each lock files so it is very time consuming. A better way is to do a CarbonFile.listFile on the lock folder and get all CarbonFile in one NameNode call. So you do not need to read table status (code from line 122 to line 137 is not required)


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on the issue:

    https://github.com/apache/carbondata/pull/2045
  
    @akashrn5 It works fine on our env, if you configured the preserver time to 1 hour (carbon.segment.lock.files.preserve.hours=1), it will delete the **segment** lock files which the last modified time is older than 1 hour.
    
    But there are one scenario that it will not delete the segment lock files:
    there are some 'MARKED_FOR_DELETE' or 'COMPACTED' or 'INSERT_IN_PROGRESS' or ''INSERT_OVERWRITE_IN_PROGRESS' segments but the last modified time of these segments were all within 1 hour.
    Maybe we can delete the segment lock files before this check , @jackylk  @ravipesala what do you think about this? is it ok to do this?


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2045: [CARBONDATA-2230]Add a path into table path t...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2045#discussion_r173716287
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/locks/HdfsFileLock.java ---
    @@ -39,16 +39,18 @@
        */
       private String location;
     
    -  private DataOutputStream dataOutputStream;
    +  private String locationPath;
    --- End diff --
    
    Done


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

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



---

[GitHub] carbondata pull request #2045: [CARBONDATA-2230]Add a path into table path t...

Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2045#discussion_r173642089
  
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/util/AlterTableUtil.scala ---
    @@ -114,6 +114,7 @@ object AlterTableUtil {
         val lockLocation = tablePath
         locks.zip(locksAcquired).foreach { case (carbonLock, lockType) =>
           val lockFilePath = lockLocation + CarbonCommonConstants.FILE_SEPARATOR +
    --- End diff --
    
    add a static function in `CarbonTablePath` to return the lockFilePath


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on the issue:

    https://github.com/apache/carbondata/pull/2045
  
    retest sdv please


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on the issue:

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


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on the issue:

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


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2045: [CARBONDATA-2230]Add a path into table path t...

Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2045#discussion_r173642153
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/statusmanager/SegmentStatusManager.java ---
    @@ -834,6 +836,40 @@ private static void writeLoadMetadata(AbsoluteTableIdentifier identifier,
         }
       }
     
    +  /**
    +   * Currently the segment lock files are not deleted immediately when unlock,
    +   * so it needs to delete expired lock files before delete loads.
    +   */
    +  private static void deleteExpiredSegmentLockFiles(CarbonTable carbonTable) {
    +    LoadMetadataDetails[] details =
    +        SegmentStatusManager.readLoadMetadata(carbonTable.getMetadataPath());
    +    if (details != null && details.length > 0) {
    +      AbsoluteTableIdentifier absoluteTableIdentifier = carbonTable.getAbsoluteTableIdentifier();
    +      long segmentLockFilesPreservTime =
    +          CarbonProperties.getInstance().getSegmentLockFilesPreserveHours();
    +      long currTime = System.currentTimeMillis();
    +      for (LoadMetadataDetails oneRow : details) {
    +        if (oneRow.getVisibility().equalsIgnoreCase("false") ||
    +            SegmentStatus.SUCCESS == oneRow.getSegmentStatus() ||
    +            SegmentStatus.LOAD_FAILURE == oneRow.getSegmentStatus() ||
    +            SegmentStatus.LOAD_PARTIAL_SUCCESS == oneRow.getSegmentStatus() ||
    +            SegmentStatus.COMPACTED == oneRow.getSegmentStatus()) {
    +          String location = absoluteTableIdentifier.getTablePath() +
    --- End diff --
    
    add a static function in CarbonTablePath to return the lockFilePath


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

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



---

[GitHub] carbondata pull request #2045: [CARBONDATA-2230]Add a path into table path t...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2045#discussion_r174338430
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/locks/CarbonLockUtil.java ---
    @@ -107,4 +114,36 @@ public static int getLockProperty(String property, int defaultValue) {
         }
       }
     
    +  /**
    +   * Currently the segment lock files are not deleted immediately when unlock,
    +   * so it needs to delete expired lock files before delete loads.
    +   */
    +  public static void deleteExpiredSegmentLockFiles(CarbonTable carbonTable) {
    +    LoadMetadataDetails[] details =
    --- End diff --
    
    I think it is better to use this method to check the status and last modified time of the segment at the same time. As the size of the tablestatus file is getting larger, it is a problem, there are many places will scan this file, not just here, we need to solve this problem. I have raise a [topic](http://apache-carbondata-dev-mailing-list-archive.1130556.n5.nabble.com/The-size-of-the-tablestatus-file-is-getting-larger-does-it-impact-the-performance-of-reading-this-fi-td41941.html) to discuss this on mailling list.


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on the issue:

    https://github.com/apache/carbondata/pull/2045
  
    @ravipesala @jackylk please help to review this, thanks.


---

[GitHub] carbondata pull request #2045: [CARBONDATA-2230]Add a path into table path t...

Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2045#discussion_r173829328
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/locks/CarbonLockUtil.java ---
    @@ -107,4 +115,37 @@ public static int getLockProperty(String property, int defaultValue) {
         }
       }
     
    +  /**
    +   * Currently the segment lock files are not deleted immediately when unlock,
    +   * so it needs to delete expired lock files before delete loads.
    +   */
    +  public static void deleteExpiredSegmentLockFiles(CarbonTable carbonTable) {
    +    LoadMetadataDetails[] details =
    +        SegmentStatusManager.readLoadMetadata(carbonTable.getMetadataPath());
    +    if (details != null && details.length > 0) {
    +      AbsoluteTableIdentifier absoluteTableIdentifier = carbonTable.getAbsoluteTableIdentifier();
    +      long segmentLockFilesPreservTime =
    +          CarbonProperties.getInstance().getSegmentLockFilesPreserveHours();
    +      long currTime = System.currentTimeMillis();
    +      for (LoadMetadataDetails oneRow : details) {
    +        if (oneRow.getVisibility().equalsIgnoreCase("false") ||
    +            SegmentStatus.SUCCESS == oneRow.getSegmentStatus() ||
    +            SegmentStatus.LOAD_FAILURE == oneRow.getSegmentStatus() ||
    +            SegmentStatus.LOAD_PARTIAL_SUCCESS == oneRow.getSegmentStatus() ||
    +            SegmentStatus.COMPACTED == oneRow.getSegmentStatus()) {
    +          String location = CarbonTablePath
    +              .getLockFilesDirPath(absoluteTableIdentifier.getTablePath()) +
    +              CarbonCommonConstants.FILE_SEPARATOR +
    +              CarbonTablePath.addSegmentPrefix(oneRow.getLoadName()) + LockUsage.LOCK;
    --- End diff --
    
    Put this string concatenate logic into one function in CarbonTablePath


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on the issue:

    https://github.com/apache/carbondata/pull/2045
  
    retest sdv please


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on the issue:

    https://github.com/apache/carbondata/pull/2045
  
    after (PR2051)[https://github.com/apache/carbondata/pull/2051] merged, i will rebase and commit the changes.


---

[GitHub] carbondata pull request #2045: [CARBONDATA-2230]Add a path into table path t...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2045#discussion_r173716485
  
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/util/AlterTableUtil.scala ---
    @@ -114,6 +114,7 @@ object AlterTableUtil {
         val lockLocation = tablePath
         locks.zip(locksAcquired).foreach { case (carbonLock, lockType) =>
           val lockFilePath = lockLocation + CarbonCommonConstants.FILE_SEPARATOR +
    --- End diff --
    
    Done


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2045: [CARBONDATA-2230]Add a path into table path t...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2045#discussion_r175252906
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/locks/CarbonLockUtil.java ---
    @@ -107,4 +114,36 @@ public static int getLockProperty(String property, int defaultValue) {
         }
       }
     
    +  /**
    +   * Currently the segment lock files are not deleted immediately when unlock,
    +   * so it needs to delete expired lock files before delete loads.
    +   */
    +  public static void deleteExpiredSegmentLockFiles(CarbonTable carbonTable) {
    +    LoadMetadataDetails[] details =
    +        SegmentStatusManager.readLoadMetadata(carbonTable.getMetadataPath());
    +    if (details != null && details.length > 0) {
    +      AbsoluteTableIdentifier absoluteTableIdentifier = carbonTable.getAbsoluteTableIdentifier();
    +      long segmentLockFilesPreservTime =
    +          CarbonProperties.getInstance().getSegmentLockFilesPreserveHours();
    +      long currTime = System.currentTimeMillis();
    +      for (LoadMetadataDetails oneRow : details) {
    +        if (oneRow.getVisibility().equalsIgnoreCase("false") ||
    +            SegmentStatus.SUCCESS == oneRow.getSegmentStatus() ||
    +            SegmentStatus.LOAD_FAILURE == oneRow.getSegmentStatus() ||
    +            SegmentStatus.LOAD_PARTIAL_SUCCESS == oneRow.getSegmentStatus() ||
    +            SegmentStatus.COMPACTED == oneRow.getSegmentStatus()) {
    +          String location = CarbonTablePath
    +              .getSegmentLockFilePath(absoluteTableIdentifier.getTablePath(),
    +                  oneRow.getLoadName());
    +          CarbonFile carbonFile =
    +              FileFactory.getCarbonFile(location, FileFactory.getFileType(location));
    +          if (carbonFile.exists()) {
    --- End diff --
    
    Done


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

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



---

[GitHub] carbondata pull request #2045: [CARBONDATA-2230]Add a path into table path t...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2045#discussion_r173716459
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/locks/HdfsFileLock.java ---
    @@ -39,16 +39,18 @@
        */
       private String location;
    --- End diff --
    
    I change this to lockFileDir, it need this parameter to mkdir.


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on the issue:

    https://github.com/apache/carbondata/pull/2045
  
    retest sdv please


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2045: [CARBONDATA-2230]Add a path into table path t...

Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2045#discussion_r173829976
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/locks/CarbonLockUtil.java ---
    @@ -107,4 +115,37 @@ public static int getLockProperty(String property, int defaultValue) {
         }
       }
     
    +  /**
    +   * Currently the segment lock files are not deleted immediately when unlock,
    +   * so it needs to delete expired lock files before delete loads.
    +   */
    +  public static void deleteExpiredSegmentLockFiles(CarbonTable carbonTable) {
    +    LoadMetadataDetails[] details =
    +        SegmentStatusManager.readLoadMetadata(carbonTable.getMetadataPath());
    +    if (details != null && details.length > 0) {
    +      AbsoluteTableIdentifier absoluteTableIdentifier = carbonTable.getAbsoluteTableIdentifier();
    +      long segmentLockFilesPreservTime =
    +          CarbonProperties.getInstance().getSegmentLockFilesPreserveHours();
    +      long currTime = System.currentTimeMillis();
    +      for (LoadMetadataDetails oneRow : details) {
    +        if (oneRow.getVisibility().equalsIgnoreCase("false") ||
    +            SegmentStatus.SUCCESS == oneRow.getSegmentStatus() ||
    +            SegmentStatus.LOAD_FAILURE == oneRow.getSegmentStatus() ||
    +            SegmentStatus.LOAD_PARTIAL_SUCCESS == oneRow.getSegmentStatus() ||
    +            SegmentStatus.COMPACTED == oneRow.getSegmentStatus()) {
    +          String location = CarbonTablePath
    +              .getLockFilesDirPath(absoluteTableIdentifier.getTablePath()) +
    +              CarbonCommonConstants.FILE_SEPARATOR +
    +              CarbonTablePath.addSegmentPrefix(oneRow.getLoadName()) + LockUsage.LOCK;
    +          CarbonFile carbonFile =
    +              FileFactory.getCarbonFile(location, FileFactory.getFileType(location));
    +          if (carbonFile.exists()) {
    --- End diff --
    
    And add one static function in FileFactory:
    `public static boolean isFileExist(String filePath) throws IOException`


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2045: [CARBONDATA-2230]Add a path into table path t...

Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2045#discussion_r173642168
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/statusmanager/SegmentStatusManager.java ---
    @@ -834,6 +836,40 @@ private static void writeLoadMetadata(AbsoluteTableIdentifier identifier,
         }
       }
     
    +  /**
    +   * Currently the segment lock files are not deleted immediately when unlock,
    +   * so it needs to delete expired lock files before delete loads.
    +   */
    +  private static void deleteExpiredSegmentLockFiles(CarbonTable carbonTable) {
    --- End diff --
    
    move this function to CarbonTable


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on the issue:

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


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2045: [CARBONDATA-2230]Add a path into table path t...

Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2045#discussion_r173829652
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/locks/CarbonLockUtil.java ---
    @@ -107,4 +115,37 @@ public static int getLockProperty(String property, int defaultValue) {
         }
       }
     
    +  /**
    +   * Currently the segment lock files are not deleted immediately when unlock,
    +   * so it needs to delete expired lock files before delete loads.
    +   */
    +  public static void deleteExpiredSegmentLockFiles(CarbonTable carbonTable) {
    +    LoadMetadataDetails[] details =
    +        SegmentStatusManager.readLoadMetadata(carbonTable.getMetadataPath());
    +    if (details != null && details.length > 0) {
    +      AbsoluteTableIdentifier absoluteTableIdentifier = carbonTable.getAbsoluteTableIdentifier();
    +      long segmentLockFilesPreservTime =
    +          CarbonProperties.getInstance().getSegmentLockFilesPreserveHours();
    +      long currTime = System.currentTimeMillis();
    +      for (LoadMetadataDetails oneRow : details) {
    +        if (oneRow.getVisibility().equalsIgnoreCase("false") ||
    +            SegmentStatus.SUCCESS == oneRow.getSegmentStatus() ||
    +            SegmentStatus.LOAD_FAILURE == oneRow.getSegmentStatus() ||
    +            SegmentStatus.LOAD_PARTIAL_SUCCESS == oneRow.getSegmentStatus() ||
    +            SegmentStatus.COMPACTED == oneRow.getSegmentStatus()) {
    +          String location = CarbonTablePath
    +              .getLockFilesDirPath(absoluteTableIdentifier.getTablePath()) +
    +              CarbonCommonConstants.FILE_SEPARATOR +
    +              CarbonTablePath.addSegmentPrefix(oneRow.getLoadName()) + LockUsage.LOCK;
    +          CarbonFile carbonFile =
    +              FileFactory.getCarbonFile(location, FileFactory.getFileType(location));
    +          if (carbonFile.exists()) {
    --- End diff --
    
    you can call FileFactory.isFileExist directly


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

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



---

[GitHub] carbondata pull request #2045: [CARBONDATA-2230]Add a path into table path t...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2045#discussion_r173887885
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/locks/CarbonLockUtil.java ---
    @@ -107,4 +115,37 @@ public static int getLockProperty(String property, int defaultValue) {
         }
       }
     
    +  /**
    +   * Currently the segment lock files are not deleted immediately when unlock,
    +   * so it needs to delete expired lock files before delete loads.
    +   */
    +  public static void deleteExpiredSegmentLockFiles(CarbonTable carbonTable) {
    +    LoadMetadataDetails[] details =
    +        SegmentStatusManager.readLoadMetadata(carbonTable.getMetadataPath());
    +    if (details != null && details.length > 0) {
    +      AbsoluteTableIdentifier absoluteTableIdentifier = carbonTable.getAbsoluteTableIdentifier();
    +      long segmentLockFilesPreservTime =
    +          CarbonProperties.getInstance().getSegmentLockFilesPreserveHours();
    +      long currTime = System.currentTimeMillis();
    +      for (LoadMetadataDetails oneRow : details) {
    +        if (oneRow.getVisibility().equalsIgnoreCase("false") ||
    +            SegmentStatus.SUCCESS == oneRow.getSegmentStatus() ||
    +            SegmentStatus.LOAD_FAILURE == oneRow.getSegmentStatus() ||
    +            SegmentStatus.LOAD_PARTIAL_SUCCESS == oneRow.getSegmentStatus() ||
    +            SegmentStatus.COMPACTED == oneRow.getSegmentStatus()) {
    +          String location = CarbonTablePath
    +              .getLockFilesDirPath(absoluteTableIdentifier.getTablePath()) +
    +              CarbonCommonConstants.FILE_SEPARATOR +
    +              CarbonTablePath.addSegmentPrefix(oneRow.getLoadName()) + LockUsage.LOCK;
    +          CarbonFile carbonFile =
    +              FileFactory.getCarbonFile(location, FileFactory.getFileType(location));
    +          if (carbonFile.exists()) {
    --- End diff --
    
    Added static function 'isFileExist(String filePath)' in FileFactory.
    But I didn't use this function directly, it still needs to use 'CarbonFile' to get the last modified time below.


---

[GitHub] carbondata pull request #2045: [CARBONDATA-2230]Add a path into table path t...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2045#discussion_r173886926
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/locks/CarbonLockUtil.java ---
    @@ -107,4 +115,37 @@ public static int getLockProperty(String property, int defaultValue) {
         }
       }
     
    +  /**
    +   * Currently the segment lock files are not deleted immediately when unlock,
    +   * so it needs to delete expired lock files before delete loads.
    +   */
    +  public static void deleteExpiredSegmentLockFiles(CarbonTable carbonTable) {
    +    LoadMetadataDetails[] details =
    +        SegmentStatusManager.readLoadMetadata(carbonTable.getMetadataPath());
    +    if (details != null && details.length > 0) {
    +      AbsoluteTableIdentifier absoluteTableIdentifier = carbonTable.getAbsoluteTableIdentifier();
    +      long segmentLockFilesPreservTime =
    +          CarbonProperties.getInstance().getSegmentLockFilesPreserveHours();
    +      long currTime = System.currentTimeMillis();
    +      for (LoadMetadataDetails oneRow : details) {
    +        if (oneRow.getVisibility().equalsIgnoreCase("false") ||
    +            SegmentStatus.SUCCESS == oneRow.getSegmentStatus() ||
    +            SegmentStatus.LOAD_FAILURE == oneRow.getSegmentStatus() ||
    +            SegmentStatus.LOAD_PARTIAL_SUCCESS == oneRow.getSegmentStatus() ||
    +            SegmentStatus.COMPACTED == oneRow.getSegmentStatus()) {
    +          String location = CarbonTablePath
    +              .getLockFilesDirPath(absoluteTableIdentifier.getTablePath()) +
    +              CarbonCommonConstants.FILE_SEPARATOR +
    +              CarbonTablePath.addSegmentPrefix(oneRow.getLoadName()) + LockUsage.LOCK;
    --- End diff --
    
    Done


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on the issue:

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


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on the issue:

    https://github.com/apache/carbondata/pull/2045
  
    retest sdv please


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on the issue:

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


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on the issue:

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


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on the issue:

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


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by akashrn5 <gi...@git.apache.org>.
Github user akashrn5 commented on the issue:

    https://github.com/apache/carbondata/pull/2045
  
    @zzcclp i had configured the preserve time to 1hour, so after one hour i loaded the table again, but it didnt clean the lock files, when these lock files wll be deleted?


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2045: [CARBONDATA-2230]Add a path into table path t...

Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2045#discussion_r173642070
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/locks/HdfsFileLock.java ---
    @@ -39,16 +39,18 @@
        */
       private String location;
    --- End diff --
    
    Is this required anymore?


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on the issue:

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


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2045: [CARBONDATA-2230]Add a path into table path t...

Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2045#discussion_r173642041
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/locks/HdfsFileLock.java ---
    @@ -39,16 +39,18 @@
        */
       private String location;
     
    -  private DataOutputStream dataOutputStream;
    +  private String locationPath;
    --- End diff --
    
    change to `lockFilePath`


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2045: [CARBONDATA-2230]Add a path into table path t...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2045#discussion_r173716505
  
    --- Diff: core/src/main/java/org/apache/carbondata/core/statusmanager/SegmentStatusManager.java ---
    @@ -834,6 +836,40 @@ private static void writeLoadMetadata(AbsoluteTableIdentifier identifier,
         }
       }
     
    +  /**
    +   * Currently the segment lock files are not deleted immediately when unlock,
    +   * so it needs to delete expired lock files before delete loads.
    +   */
    +  private static void deleteExpiredSegmentLockFiles(CarbonTable carbonTable) {
    +    LoadMetadataDetails[] details =
    +        SegmentStatusManager.readLoadMetadata(carbonTable.getMetadataPath());
    +    if (details != null && details.length > 0) {
    +      AbsoluteTableIdentifier absoluteTableIdentifier = carbonTable.getAbsoluteTableIdentifier();
    +      long segmentLockFilesPreservTime =
    +          CarbonProperties.getInstance().getSegmentLockFilesPreserveHours();
    +      long currTime = System.currentTimeMillis();
    +      for (LoadMetadataDetails oneRow : details) {
    +        if (oneRow.getVisibility().equalsIgnoreCase("false") ||
    +            SegmentStatus.SUCCESS == oneRow.getSegmentStatus() ||
    +            SegmentStatus.LOAD_FAILURE == oneRow.getSegmentStatus() ||
    +            SegmentStatus.LOAD_PARTIAL_SUCCESS == oneRow.getSegmentStatus() ||
    +            SegmentStatus.COMPACTED == oneRow.getSegmentStatus()) {
    +          String location = absoluteTableIdentifier.getTablePath() +
    --- End diff --
    
    Done


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on the issue:

    https://github.com/apache/carbondata/pull/2045
  
    @jackylk @ravipesala please help to review, thanks


---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2045: [CARBONDATA-2230]Add a path into table path to store...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---