[GitHub] [carbondata] Zhangshunyu opened a new pull request #3832: [HOTFIX] Fix getting block name in compacted segment with dot for horizontal compaction delta files

[GitBox]

Zhangshunyu opened a new pull request #3832:
URL: https://github.com/apache/carbondata/pull/3832


   
    ### Why is this PR needed?
    Currentlly, during horizontal compaction, driver will list delta files in segment to merge them, during this comparasion it will use block name, current code only consider the blocks in normal segment, but not consider the compacted segement, for example, the block name XXXX-12.2-time.deletedelta, the block name should be XXXX-12.2 not XXXX, if use XXX it will not list the delta files and horizontal compaction will not merge delta files.
   
    ### What changes were proposed in this PR?
   Correct the parse of block name
       
    ### Does this PR introduce any user interface change?
    - No
   
    ### Is any new testcase added?
    - No
   
       
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3832:
URL: https://github.com/apache/carbondata/pull/3832#issuecomment-655909475


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3333/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3832:
URL: https://github.com/apache/carbondata/pull/3832#issuecomment-655909870


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/1593/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

Indhumathi27 commented on a change in pull request #3832:
URL: https://github.com/apache/carbondata/pull/3832#discussion_r451973238



##########
File path: core/src/main/java/org/apache/carbondata/core/statusmanager/SegmentUpdateStatusManager.java
##########
@@ -450,8 +450,7 @@ public boolean accept(CarbonFile pathName) {
             String fileName = pathName.getName();
             if (fileName.endsWith(CarbonCommonConstants.DELETE_DELTA_FILE_EXT)
                 && pathName.getSize() > 0) {
-              String firstPart = fileName.substring(0, fileName.indexOf('.'));
-              String blkName = firstPart.substring(0, firstPart.lastIndexOf("-"));
+              String blkName = fileName.substring(0, fileName.lastIndexOf("-"));

Review comment:
       Can you please add a testcase to check num of files after update/delete in a compacted segment




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3832:
URL: https://github.com/apache/carbondata/pull/3832#issuecomment-656119978


   Build Failed  with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/1598/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3832:
URL: https://github.com/apache/carbondata/pull/3832#issuecomment-656120519


   Build Failed  with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3338/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

marchpure commented on a change in pull request #3832:
URL: https://github.com/apache/carbondata/pull/3832#discussion_r452606626



##########
File path: core/src/main/java/org/apache/carbondata/core/statusmanager/SegmentUpdateStatusManager.java
##########
@@ -450,8 +450,7 @@ public boolean accept(CarbonFile pathName) {
             String fileName = pathName.getName();
             if (fileName.endsWith(CarbonCommonConstants.DELETE_DELTA_FILE_EXT)
                 && pathName.getSize() > 0) {

Review comment:
        if ( pathName.getSize() > 0 && fileName.endsWith(CarbonCommonConstants.DELETE_DELTA_FILE_EXT))  
   will be better




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3832:
URL: https://github.com/apache/carbondata/pull/3832#issuecomment-656466931


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3342/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3832:
URL: https://github.com/apache/carbondata/pull/3832#issuecomment-656467186


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/1602/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

Zhangshunyu commented on a change in pull request #3832:
URL: https://github.com/apache/carbondata/pull/3832#discussion_r452639364



##########
File path: core/src/main/java/org/apache/carbondata/core/statusmanager/SegmentUpdateStatusManager.java
##########
@@ -450,8 +450,7 @@ public boolean accept(CarbonFile pathName) {
             String fileName = pathName.getName();
             if (fileName.endsWith(CarbonCommonConstants.DELETE_DELTA_FILE_EXT)
                 && pathName.getSize() > 0) {
-              String firstPart = fileName.substring(0, fileName.indexOf('.'));
-              String blkName = firstPart.substring(0, firstPart.lastIndexOf("-"));
+              String blkName = fileName.substring(0, fileName.lastIndexOf("-"));

Review comment:
       @Indhumathi27 added test case




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3832:
URL: https://github.com/apache/carbondata/pull/3832#issuecomment-656502354


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3343/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3832:
URL: https://github.com/apache/carbondata/pull/3832#issuecomment-656504368


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/1603/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

Indhumathi27 commented on a change in pull request #3832:
URL: https://github.com/apache/carbondata/pull/3832#discussion_r452646468



##########
File path: core/src/main/java/org/apache/carbondata/core/mutate/CarbonUpdateUtil.java
##########
@@ -471,6 +471,13 @@ public static void cleanUpDeltaFiles(CarbonTable table, boolean forceDelete) thr
     LoadMetadataDetails[] details =
         SegmentStatusManager.readLoadMetadata(table.getMetadataPath());
 
+    SegmentUpdateStatusManager updateStatusManager = new SegmentUpdateStatusManager(table);
+    SegmentUpdateDetails[] segmentUpdateDetails = updateStatusManager.getUpdateStatusDetails();

Review comment:
       Please add comments why this change is required




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

Zhangshunyu commented on a change in pull request #3832:
URL: https://github.com/apache/carbondata/pull/3832#discussion_r452650935



##########
File path: core/src/main/java/org/apache/carbondata/core/mutate/CarbonUpdateUtil.java
##########
@@ -471,6 +471,13 @@ public static void cleanUpDeltaFiles(CarbonTable table, boolean forceDelete) thr
     LoadMetadataDetails[] details =
         SegmentStatusManager.readLoadMetadata(table.getMetadataPath());
 
+    SegmentUpdateStatusManager updateStatusManager = new SegmentUpdateStatusManager(table);
+    SegmentUpdateDetails[] segmentUpdateDetails = updateStatusManager.getUpdateStatusDetails();

Review comment:
       @Indhumathi27 added




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

Zhangshunyu commented on a change in pull request #3832:
URL: https://github.com/apache/carbondata/pull/3832#discussion_r452652701



##########
File path: core/src/main/java/org/apache/carbondata/core/statusmanager/SegmentUpdateStatusManager.java
##########
@@ -450,8 +450,7 @@ public boolean accept(CarbonFile pathName) {
             String fileName = pathName.getName();
             if (fileName.endsWith(CarbonCommonConstants.DELETE_DELTA_FILE_EXT)
                 && pathName.getSize() > 0) {

Review comment:
       @marchpure OK




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3832:
URL: https://github.com/apache/carbondata/pull/3832#issuecomment-656570031


   Build Failed  with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/1605/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3832:
URL: https://github.com/apache/carbondata/pull/3832#issuecomment-656571415


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/3345/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

Zhangshunyu commented on pull request #3832:
URL: https://github.com/apache/carbondata/pull/3832#issuecomment-656684521


   retest this please


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

Zhangshunyu closed pull request #3832:
URL: https://github.com/apache/carbondata/pull/3832


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA1 commented on pull request #3832:
URL: https://github.com/apache/carbondata/pull/3832#issuecomment-656754576


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/1612/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]