[GitHub] [hudi] codope commented on a diff in pull request #8972: [HUDI-6370] Automatically decide archival boundary based on cleaning configs

["codope (via GitHub)" <gi...@apache.org>]

codope commented on code in PR #8972:
URL: https://github.com/apache/hudi/pull/8972#discussion_r1230094869


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/HoodieTimelineArchiver.java:
##########
@@ -115,9 +118,67 @@ public HoodieTimelineArchiver(HoodieWriteConfig config, HoodieTable<T, I, K, O>
     this.table = table;
     this.metaClient = table.getMetaClient();
     this.archiveFilePath = HoodieArchivedTimeline.getArchiveLogPath(metaClient.getArchivePath());
-    this.maxInstantsToKeep = config.getMaxCommitsToKeep();
-    this.minInstantsToKeep = config.getMinCommitsToKeep();
     this.txnManager = new TransactionManager(config, table.getMetaClient().getFs());
+
+    Option<HoodieInstant> earliestCommitToRetain = Option.empty();
+    HoodieTimeline completedCommitsTimeline = table.getCompletedCommitsTimeline();
+    String latestCommitTime = completedCommitsTimeline.lastInstant().get().getTimestamp();
+    HoodieCleaningPolicy cleanerPolicy = config.getCleanerPolicy();
+    int cleanerCommitsRetained = config.getCleanerCommitsRetained();
+    int cleanerHoursRetained = config.getCleanerHoursRetained();
+
+    try {
+      earliestCommitToRetain = CleanerUtils.getEarliestCommitToRetain(
+          metaClient.getActiveTimeline().getCommitsTimeline(),
+          cleanerPolicy,
+          cleanerCommitsRetained,
+          HoodieActiveTimeline.parseDateFromInstantTime(latestCommitTime).toInstant(),
+          cleanerHoursRetained,
+          metaClient.getTableConfig().getTimelineTimezone());
+    } catch (ParseException e) {
+      LOG.warn("Error parsing instant time: " + latestCommitTime);
+    }
+
+    int configuredMinInstantsToKeep = config.getMinCommitsToKeep();
+    int configuredMaxInstantsToKeep = config.getMaxCommitsToKeep();
+    if (earliestCommitToRetain.isPresent()) {
+      int minInstantsToKeepBasedOnCleaning =
+          completedCommitsTimeline.findInstantsAfter(earliestCommitToRetain.get().getTimestamp())
+              .countInstants() + 1;
+      if (configuredMinInstantsToKeep < minInstantsToKeepBasedOnCleaning) {
+        this.maxInstantsToKeep = minInstantsToKeepBasedOnCleaning
+            + configuredMaxInstantsToKeep - configuredMinInstantsToKeep;
+        this.minInstantsToKeep = minInstantsToKeepBasedOnCleaning;

Review Comment:
   Logic looks good. Can we add a test for this scenario?



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org