wangxianghu commented on a change in pull request #2325:
URL: https://github.com/apache/hudi/pull/2325#discussion_r601092940



##########
File path: 
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieArchivedTimeline.java
##########
@@ -254,4 +288,19 @@ private int getArchivedFileSuffix(FileStatus f) {
       return 0;
     }
   }
+
+  @Override
+  public HoodieDefaultTimeline getCommitsAndCompactionTimeline() {
+    // filter in-memory instants
+    Set<String> validActions = CollectionUtils.createSet(COMMIT_ACTION, 
DELTA_COMMIT_ACTION, COMPACTION_ACTION, REPLACE_COMMIT_ACTION);
+    return new HoodieDefaultTimeline(getInstants().filter(i ->
+        readCommits.keySet().contains(i.getTimestamp()))
+        .filter(s -> validActions.contains(s.getAction())), details);
+  }
+
+  public HoodieArchivedTimeline filterArchivedCompactionInstant() {
+    // filter INFLIGHT compaction instants
+    return new HoodieArchivedTimeline(this.metaClient, getInstants().filter(i 
->
+        i.isInflight() && 
i.getAction().equals(HoodieTimeline.COMPACTION_ACTION)));
+  }

Review comment:
       This method seems not used anywhere?

##########
File path: 
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieArchivedTimeline.java
##########
@@ -108,6 +120,14 @@ public void loadInstantDetailsInMemory(String startTs, 
String endTs) {
     loadInstants(startTs, endTs);
   }
 
+  public void loadCompactionDetailsInMemory(String startTs, String endTs) {
+    // load compactionPlan
+    loadInstants(new TimeRangeFilter(startTs, endTs), true, record ->
+        
record.get(ACTION_TYPE_KEY).toString().equals(HoodieTimeline.COMPACTION_ACTION)
+            && 
HoodieInstant.State.INFLIGHT.toString().equals(record.get("actionState").toString())

Review comment:
       "actionState" -> ACTION_STATE

##########
File path: 
hudi-cli/src/main/java/org/apache/hudi/cli/commands/CompactionCommand.java
##########
@@ -175,25 +174,26 @@ public String compactionShowArchived(
     HoodieArchivedTimeline archivedTimeline = client.getArchivedTimeline();
     HoodieInstant instant = new HoodieInstant(HoodieInstant.State.COMPLETED,
             HoodieTimeline.COMPACTION_ACTION, compactionInstantTime);
-    String startTs = CommitUtil.addHours(compactionInstantTime, -1);
-    String endTs = CommitUtil.addHours(compactionInstantTime, 1);

Review comment:
       if we want to load a `ts` equals `compactionInstantTime`, can we add a 
new method that takes only one `instantTime` as input param? WDYT

##########
File path: 
hudi-cli/src/main/java/org/apache/hudi/cli/commands/CompactionCommand.java
##########
@@ -175,25 +174,26 @@ public String compactionShowArchived(
     HoodieArchivedTimeline archivedTimeline = client.getArchivedTimeline();
     HoodieInstant instant = new HoodieInstant(HoodieInstant.State.COMPLETED,
             HoodieTimeline.COMPACTION_ACTION, compactionInstantTime);
-    String startTs = CommitUtil.addHours(compactionInstantTime, -1);
-    String endTs = CommitUtil.addHours(compactionInstantTime, 1);

Review comment:
       why remove this?

##########
File path: 
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/TimelineMetadataUtils.java
##########
@@ -176,4 +179,13 @@ public static HoodieReplaceCommitMetadata 
deserializeHoodieReplaceMetadata(byte[
     ValidationUtils.checkArgument(fileReader.hasNext(), "Could not deserialize 
metadata of type " + clazz);
     return fileReader.next();
   }
+
+  public static <T extends SpecificRecordBase> T 
deserializeAvroRecordMetadata(byte[] bytes, Schema schema, Class<T> clazz)
+      throws IOException {
+    return  deserializeAvroRecordMetadata(HoodieAvroUtils.bytesToAvro(bytes, 
schema), schema, clazz);
+  }
+
+  public static <T extends SpecificRecordBase> T 
deserializeAvroRecordMetadata(Object object, Schema schema, Class<T> clazz) {

Review comment:
       param `Class<T> clazz` seems redundant

##########
File path: 
hudi-cli/src/main/java/org/apache/hudi/cli/commands/CompactionCommand.java
##########
@@ -175,25 +174,26 @@ public String compactionShowArchived(
     HoodieArchivedTimeline archivedTimeline = client.getArchivedTimeline();
     HoodieInstant instant = new HoodieInstant(HoodieInstant.State.COMPLETED,
             HoodieTimeline.COMPACTION_ACTION, compactionInstantTime);
-    String startTs = CommitUtil.addHours(compactionInstantTime, -1);
-    String endTs = CommitUtil.addHours(compactionInstantTime, 1);
     try {
-      archivedTimeline.loadInstantDetailsInMemory(startTs, endTs);
-      HoodieCompactionPlan compactionPlan = 
TimelineMetadataUtils.deserializeCompactionPlan(
-              archivedTimeline.getInstantDetails(instant).get());
+      archivedTimeline.loadCompactionDetailsInMemory(compactionInstantTime, 
compactionInstantTime);
+      HoodieCompactionPlan compactionPlan = 
TimelineMetadataUtils.deserializeAvroRecordMetadata(
+              archivedTimeline.getInstantDetails(instant).get(), 
HoodieCompactionPlan.getClassSchema(),
+              HoodieCompactionPlan.class);
       return printCompaction(compactionPlan, sortByField, descending, limit, 
headerOnly);
     } finally {
-      archivedTimeline.clearInstantDetailsFromMemory(startTs, endTs);
+      archivedTimeline.clearInstantDetailsFromMemory(compactionInstantTime, 
compactionInstantTime);

Review comment:
       same here




-- 
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:
[email protected]


Reply via email to