[GitHub] [carbondata] shenjiayu17 opened a new pull request #3986: [CARBONDATA-4034] Improve the time-consuming of Horizontal Compaction for update

[GitBox]

shenjiayu17 commented on a change in pull request #3986:
URL: https://github.com/apache/carbondata/pull/3986#discussion_r506020781



##########
File path: processing/src/main/java/org/apache/carbondata/processing/merger/CarbonDataMergerUtil.java
##########
@@ -1210,6 +1198,39 @@ private static boolean checkDeleteDeltaFilesInSeg(Segment seg,
     return blockLists;
   }
 
+  private static List<String> checkAndGetDeleteDeltaFilesInSeg(Segment seg,
+      SegmentUpdateStatusManager segmentUpdateStatusManager, int numberDeltaFilesThreshold) {
+
+    List<String> blockLists = new ArrayList<>();
+
+    Map<String, List<CarbonFile>> blockAndDeleteDeltaFilesMap =
+      segmentUpdateStatusManager.getDeleteDeltaFilesForSegment(seg);
+
+    List<String> blockNameList =
+      segmentUpdateStatusManager.getBlockNameFromSegment(seg.getSegmentNo());
+
+    Set<String> uniqueBlocks = new HashSet<String>();
+    for (final String blockName : blockNameList) {
+
+      List<CarbonFile> deleteDeltaFiles = blockAndDeleteDeltaFilesMap.get(blockName);
+
+      if (null != deleteDeltaFiles) {
+        for (CarbonFile blocks : deleteDeltaFiles) {

Review comment:
       Done.
   Added judgement:
   if (deleteDeltaFiles.size() <= numberDeltaFilesThreshold) continue




----------------------------------------------------------------
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]

shenjiayu17 commented on a change in pull request #3986:
URL: https://github.com/apache/carbondata/pull/3986#discussion_r506020436



##########
File path: processing/src/main/java/org/apache/carbondata/processing/merger/CarbonDataMergerUtil.java
##########
@@ -1039,22 +1039,10 @@ private static boolean isSegmentValid(LoadMetadataDetails seg) {
     if (CompactionType.IUD_DELETE_DELTA == compactionTypeIUD) {
       int numberDeleteDeltaFilesThreshold =
           CarbonProperties.getInstance().getNoDeleteDeltaFilesThresholdForIUDCompaction();
-      List<Segment> deleteSegments = new ArrayList<>();
       for (Segment seg : segments) {
-        if (checkDeleteDeltaFilesInSeg(seg, segmentUpdateStatusManager,

Review comment:
       Done.
   Combined function checkDeleteDeltaFilesInSeg and function getDeleteDeltaFilesInSeg to new function checkAndGetDeleteDeltaFilesInSeg




----------------------------------------------------------------
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]

shenjiayu17 commented on a change in pull request #3986:
URL: https://github.com/apache/carbondata/pull/3986#discussion_r506202461



##########
File path: core/src/main/java/org/apache/carbondata/core/statusmanager/SegmentUpdateStatusManager.java
##########
@@ -455,6 +455,51 @@ public boolean accept(CarbonFile pathName) {
     return null;
   }
 
+  public Map<String, List<CarbonFile>> getDeleteDeltaFilesForSegment(final Segment seg) {

Review comment:
       Done.
   Keep function name getDeleteDeltaFilesList and change the return type to Map<String, List<CarbonFile>>




----------------------------------------------------------------
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]

shenjiayu17 commented on a change in pull request #3986:
URL: https://github.com/apache/carbondata/pull/3986#discussion_r506203367



##########
File path: core/src/main/java/org/apache/carbondata/core/statusmanager/SegmentUpdateStatusManager.java
##########
@@ -455,6 +455,51 @@ public boolean accept(CarbonFile pathName) {
     return null;
   }
 
+  public Map<String, List<CarbonFile>> getDeleteDeltaFilesForSegment(final Segment seg) {
+    String segmentPath = CarbonTablePath.getSegmentPath(
+      identifier.getTablePath(), seg.getSegmentNo());

Review comment:
       Done.




----------------------------------------------------------------
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 #3986:
URL: https://github.com/apache/carbondata/pull/3986#discussion_r506261693



##########
File path: processing/src/main/java/org/apache/carbondata/processing/merger/CarbonDataMergerUtil.java
##########
@@ -1246,8 +1209,22 @@ public static boolean isHorizontalCompactionEnabled() {
     // set the update status.
     segmentUpdateStatusManager.setUpdateStatusDetails(segmentUpdateDetails);
 
-    CarbonFile[] deleteDeltaFiles =
-        segmentUpdateStatusManager.getDeleteDeltaFilesList(new Segment(seg), blockName);
+    // only when SegmentUpdateDetails contain the specified block
+    // will the method getDeleteDeltaFilesList be executed
+    List<String> blockNameList = segmentUpdateStatusManager.getBlockNameFromSegment(seg);
+    Map<String, List<CarbonFile>> blockAndDeleteDeltaFilesMap = new HashMap<>();
+    CarbonFile[] deleteDeltaFiles = null;
+    if (blockNameList.contains(blockName)) {
+      blockAndDeleteDeltaFilesMap =
+          segmentUpdateStatusManager.getDeleteDeltaFilesList(new Segment(seg));
+    }
+    if (blockAndDeleteDeltaFilesMap.containsKey(blockName)) {
+      List<CarbonFile> deleteDeltaFileList = blockAndDeleteDeltaFilesMap.get(blockName);
+      deleteDeltaFiles = deleteDeltaFileList.toArray(new CarbonFile[deleteDeltaFileList.size()]);
+    }
+
+    // CarbonFile[] deleteDeltaFiles =

Review comment:
       delete these 2 lines

##########
File path: processing/src/main/java/org/apache/carbondata/processing/merger/CarbonDataMergerUtil.java
##########
@@ -1246,8 +1209,22 @@ public static boolean isHorizontalCompactionEnabled() {
     // set the update status.
     segmentUpdateStatusManager.setUpdateStatusDetails(segmentUpdateDetails);
 
-    CarbonFile[] deleteDeltaFiles =
-        segmentUpdateStatusManager.getDeleteDeltaFilesList(new Segment(seg), blockName);
+    // only when SegmentUpdateDetails contain the specified block
+    // will the method getDeleteDeltaFilesList be executed
+    List<String> blockNameList = segmentUpdateStatusManager.getBlockNameFromSegment(seg);
+    Map<String, List<CarbonFile>> blockAndDeleteDeltaFilesMap = new HashMap<>();
+    CarbonFile[] deleteDeltaFiles = null;

Review comment:
       = new CarbonFile[0]

##########
File path: processing/src/main/java/org/apache/carbondata/processing/merger/CarbonDataMergerUtil.java
##########
@@ -1246,8 +1209,22 @@ public static boolean isHorizontalCompactionEnabled() {
     // set the update status.
     segmentUpdateStatusManager.setUpdateStatusDetails(segmentUpdateDetails);
 
-    CarbonFile[] deleteDeltaFiles =
-        segmentUpdateStatusManager.getDeleteDeltaFilesList(new Segment(seg), blockName);
+    // only when SegmentUpdateDetails contain the specified block
+    // will the method getDeleteDeltaFilesList be executed
+    List<String> blockNameList = segmentUpdateStatusManager.getBlockNameFromSegment(seg);
+    Map<String, List<CarbonFile>> blockAndDeleteDeltaFilesMap = new HashMap<>();
+    CarbonFile[] deleteDeltaFiles = null;
+    if (blockNameList.contains(blockName)) {

Review comment:
       if (blockNameList.contains(blockName)) {
     blockAndDeleteDeltaFilesMap =
     if (blockAndDeleteDeltaFilesMap.containsKey(blockName)) {
     }
   }

##########
File path: processing/src/main/java/org/apache/carbondata/processing/merger/CarbonDataMergerUtil.java
##########
@@ -1246,8 +1209,22 @@ public static boolean isHorizontalCompactionEnabled() {
     // set the update status.
     segmentUpdateStatusManager.setUpdateStatusDetails(segmentUpdateDetails);
 
-    CarbonFile[] deleteDeltaFiles =
-        segmentUpdateStatusManager.getDeleteDeltaFilesList(new Segment(seg), blockName);
+    // only when SegmentUpdateDetails contain the specified block
+    // will the method getDeleteDeltaFilesList be executed
+    List<String> blockNameList = segmentUpdateStatusManager.getBlockNameFromSegment(seg);
+    Map<String, List<CarbonFile>> blockAndDeleteDeltaFilesMap = new HashMap<>();
+    CarbonFile[] deleteDeltaFiles = null;
+    if (blockNameList.contains(blockName)) {
+      blockAndDeleteDeltaFilesMap =
+          segmentUpdateStatusManager.getDeleteDeltaFilesList(new Segment(seg));
+    }
+    if (blockAndDeleteDeltaFilesMap.containsKey(blockName)) {

Review comment:
       no need to covert to array




----------------------------------------------------------------
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 #3986:
URL: https://github.com/apache/carbondata/pull/3986#issuecomment-709976082


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


----------------------------------------------------------------
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 #3986:
URL: https://github.com/apache/carbondata/pull/3986#issuecomment-709984578


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


----------------------------------------------------------------
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 pull request #3986:
URL: https://github.com/apache/carbondata/pull/3986#issuecomment-710780197


   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]

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


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


----------------------------------------------------------------
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 #3986:
URL: https://github.com/apache/carbondata/pull/3986#issuecomment-710849261


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


----------------------------------------------------------------
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 pull request #3986:
URL: https://github.com/apache/carbondata/pull/3986#issuecomment-711459338


   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]

shenjiayu17 commented on a change in pull request #3986:
URL: https://github.com/apache/carbondata/pull/3986#discussion_r507380031



##########
File path: processing/src/main/java/org/apache/carbondata/processing/merger/CarbonDataMergerUtil.java
##########
@@ -1246,8 +1209,22 @@ public static boolean isHorizontalCompactionEnabled() {
     // set the update status.
     segmentUpdateStatusManager.setUpdateStatusDetails(segmentUpdateDetails);
 
-    CarbonFile[] deleteDeltaFiles =
-        segmentUpdateStatusManager.getDeleteDeltaFilesList(new Segment(seg), blockName);
+    // only when SegmentUpdateDetails contain the specified block
+    // will the method getDeleteDeltaFilesList be executed
+    List<String> blockNameList = segmentUpdateStatusManager.getBlockNameFromSegment(seg);
+    Map<String, List<CarbonFile>> blockAndDeleteDeltaFilesMap = new HashMap<>();
+    CarbonFile[] deleteDeltaFiles = null;
+    if (blockNameList.contains(blockName)) {
+      blockAndDeleteDeltaFilesMap =
+          segmentUpdateStatusManager.getDeleteDeltaFilesList(new Segment(seg));
+    }
+    if (blockAndDeleteDeltaFilesMap.containsKey(blockName)) {
+      List<CarbonFile> deleteDeltaFileList = blockAndDeleteDeltaFilesMap.get(blockName);
+      deleteDeltaFiles = deleteDeltaFileList.toArray(new CarbonFile[deleteDeltaFileList.size()]);
+    }
+
+    // CarbonFile[] deleteDeltaFiles =

Review comment:
       Done




----------------------------------------------------------------
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]

shenjiayu17 commented on a change in pull request #3986:
URL: https://github.com/apache/carbondata/pull/3986#discussion_r507380251



##########
File path: processing/src/main/java/org/apache/carbondata/processing/merger/CarbonDataMergerUtil.java
##########
@@ -1246,8 +1209,22 @@ public static boolean isHorizontalCompactionEnabled() {
     // set the update status.
     segmentUpdateStatusManager.setUpdateStatusDetails(segmentUpdateDetails);
 
-    CarbonFile[] deleteDeltaFiles =
-        segmentUpdateStatusManager.getDeleteDeltaFilesList(new Segment(seg), blockName);
+    // only when SegmentUpdateDetails contain the specified block
+    // will the method getDeleteDeltaFilesList be executed
+    List<String> blockNameList = segmentUpdateStatusManager.getBlockNameFromSegment(seg);
+    Map<String, List<CarbonFile>> blockAndDeleteDeltaFilesMap = new HashMap<>();
+    CarbonFile[] deleteDeltaFiles = null;

Review comment:
       Done




----------------------------------------------------------------
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]

shenjiayu17 commented on a change in pull request #3986:
URL: https://github.com/apache/carbondata/pull/3986#discussion_r507380663



##########
File path: processing/src/main/java/org/apache/carbondata/processing/merger/CarbonDataMergerUtil.java
##########
@@ -1246,8 +1209,22 @@ public static boolean isHorizontalCompactionEnabled() {
     // set the update status.
     segmentUpdateStatusManager.setUpdateStatusDetails(segmentUpdateDetails);
 
-    CarbonFile[] deleteDeltaFiles =
-        segmentUpdateStatusManager.getDeleteDeltaFilesList(new Segment(seg), blockName);
+    // only when SegmentUpdateDetails contain the specified block
+    // will the method getDeleteDeltaFilesList be executed
+    List<String> blockNameList = segmentUpdateStatusManager.getBlockNameFromSegment(seg);
+    Map<String, List<CarbonFile>> blockAndDeleteDeltaFilesMap = new HashMap<>();
+    CarbonFile[] deleteDeltaFiles = null;
+    if (blockNameList.contains(blockName)) {

Review comment:
       Done. Combined the two judgement




----------------------------------------------------------------
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]

shenjiayu17 commented on a change in pull request #3986:
URL: https://github.com/apache/carbondata/pull/3986#discussion_r507380824



##########
File path: processing/src/main/java/org/apache/carbondata/processing/merger/CarbonDataMergerUtil.java
##########
@@ -1246,8 +1209,22 @@ public static boolean isHorizontalCompactionEnabled() {
     // set the update status.
     segmentUpdateStatusManager.setUpdateStatusDetails(segmentUpdateDetails);
 
-    CarbonFile[] deleteDeltaFiles =
-        segmentUpdateStatusManager.getDeleteDeltaFilesList(new Segment(seg), blockName);
+    // only when SegmentUpdateDetails contain the specified block
+    // will the method getDeleteDeltaFilesList be executed
+    List<String> blockNameList = segmentUpdateStatusManager.getBlockNameFromSegment(seg);
+    Map<String, List<CarbonFile>> blockAndDeleteDeltaFilesMap = new HashMap<>();
+    CarbonFile[] deleteDeltaFiles = null;
+    if (blockNameList.contains(blockName)) {
+      blockAndDeleteDeltaFilesMap =
+          segmentUpdateStatusManager.getDeleteDeltaFilesList(new Segment(seg));
+    }
+    if (blockAndDeleteDeltaFilesMap.containsKey(blockName)) {

Review comment:
       Done




----------------------------------------------------------------
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 #3986:
URL: https://github.com/apache/carbondata/pull/3986#issuecomment-711479262


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


----------------------------------------------------------------
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 #3986:
URL: https://github.com/apache/carbondata/pull/3986#issuecomment-711479603


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


----------------------------------------------------------------
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 #3986:
URL: https://github.com/apache/carbondata/pull/3986#issuecomment-711484392


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


----------------------------------------------------------------
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 #3986:
URL: https://github.com/apache/carbondata/pull/3986#issuecomment-711484706


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


----------------------------------------------------------------
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 #3986:
URL: https://github.com/apache/carbondata/pull/3986#issuecomment-711817892


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


----------------------------------------------------------------
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]