ad1happy2go commented on code in PR #20273:
URL: https://github.com/apache/hudi/pull/20273#discussion_r4259650503


##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/versioning/v2/CompletionTimeQueryViewV2.java:
##########
@@ -102,14 +128,18 @@ public CompletionTimeQueryViewV2(HoodieTableMetaClient 
metaClient) {
    */
   public boolean isCompleted(String instantTime) {
     // archival does not proceed beyond the first savepoint, so any instant 
before that is completed.
-    return this.instantTimeToCompletionTimeMap.containsKey(instantTime) || 
isArchived(instantTime);
+    return this.instantTimeToCompletionTimeMap.containsKey(instantTime)
+        || InstantComparison.compareTimestamps(instantTime, LESSER_THAN, 
this.firstNonSavepointCommit);
   }
 
   /**
    * Returns whether the instant is archived.
    */
   public boolean isArchived(String instantTime) {
-    return InstantComparison.compareTimestamps(instantTime, LESSER_THAN, 
this.firstNonSavepointCommit);
+    // with archival beyond savepoint, the write instants between a 
savepointed commit and the first active write
+    // instant are archived as well, while the savepointed commit itself stays 
active.
+    return InstantComparison.compareTimestamps(instantTime, LESSER_THAN, 
this.firstActiveWriteInstant)
+        && !this.activeSavepointedInstants.contains(instantTime);

Review Comment:
   Good catch, fixed in 4d9be634a. `splitInstantByActiveness` now checks each 
instant with `isArchived` instead of cutting at the first active one, and the 
merged instant list is ordered by instant time. 
`testArchivedInstantsAfterSavepointedCommit` now also covers a window starting 
at `1000`: 2–6 come back as archived and 1, 7, 8 as active.



##########
hudi-common/src/main/java/org/apache/hudi/core/read/BaseHoodieTableFileIndex.java:
##########
@@ -95,6 +96,10 @@
 @Slf4j
 public abstract class BaseHoodieTableFileIndex implements AutoCloseable {
 
+  private static final Set<String> WRITE_AND_SAVEPOINT_ACTIONS = 
CollectionUtils.createSet(

Review Comment:
   Done in 4d9be634a: `WRITE_AND_SAVEPOINT_ACTIONS` is now defined once in 
`HoodieTimeline` and used by both classes.



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

To unsubscribe, e-mail: [email protected]

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

Reply via email to