the-other-tim-brown commented on code in PR #13660:
URL: https://github.com/apache/hudi/pull/13660#discussion_r2250261839


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/savepoint/SavepointActionExecutor.java:
##########
@@ -145,10 +132,41 @@ public HoodieSavepointMetadata execute() {
           .saveAsComplete(
               true, 
instantGenerator.createNewInstant(HoodieInstant.State.INFLIGHT, 
HoodieTimeline.SAVEPOINT_ACTION, instantTime), Option.of(metadata),
               savepointCompletedInstant -> 
table.getMetaClient().getTableFormat().savepoint(savepointCompletedInstant, 
table.getContext(), table.getMetaClient(), table.getViewManager()));
-      LOG.info("Savepoint " + instantTime + " created");
+      LOG.info("Savepoint {} created", instantTime);
       return metadata;
     } catch (HoodieIOException e) {
       throw new HoodieSavepointException("Failed to savepoint " + instantTime, 
e);
     }
   }
+
+  @VisibleForTesting
+  String getLastCommitRetained() {
+    Option<HoodieInstant> cleanInstant = 
table.getCleanTimeline().lastInstant();
+    // if there are no clean instants, we can use the first completed commit
+    if (cleanInstant.isEmpty()) {
+      return 
table.getCompletedCommitsTimeline().firstInstant().get().requestedTime();
+    }
+    return cleanInstant.map(instant -> {
+      try {
+        if (instant.isCompleted()) {
+          return table.getActiveTimeline().readCleanMetadata(instant)
+              .getEarliestCommitToRetain();
+        } else {
+          // clean is pending or inflight
+          return table.getActiveTimeline().readCleanerPlan(
+                  instantGenerator.createNewInstant(REQUESTED, 
instant.getAction(), instant.requestedTime()))
+              .getEarliestInstantToRetain().getTimestamp();
+        }
+      } catch (IOException e) {
+        throw new HoodieSavepointException("Failed to savepoint " + 
instantTime, e);
+      }
+    }).orElseGet(() ->
+      // If there is no earliest commit to retain, but there are clean 
instants on the timeline, we must assume KEEP_LATEST_FILE_VERSIONS policy is 
used.
+      // In that case, the last commit before the clean instant is the last 
commit guaranteed to be retained.
+      table.getActiveTimeline().getWriteTimeline().filterCompletedInstants()
+          .filter(instant -> compareTimestamps(instant.requestedTime(), 
LESSER_THAN_OR_EQUALS, cleanInstant.get().requestedTime()))

Review Comment:
   Updated to use the clean instant instead of throwing an exception



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