nsivabalan commented on a change in pull request #4385:
URL: https://github.com/apache/hudi/pull/4385#discussion_r803291165
##########
File path:
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/clean/CleanPlanActionExecutor.java
##########
@@ -58,8 +59,34 @@ public CleanPlanActionExecutor(HoodieEngineContext context,
this.extraMetadata = extraMetadata;
}
- protected Option<HoodieCleanerPlan> createCleanerPlan() {
- return execute();
+ private int getCommitInfo() {
+ Option<HoodieInstant> lastCleanInstant =
table.getActiveTimeline().getCleanerTimeline().filterCompletedInstants().lastInstant();
+ HoodieTimeline commitTimeline =
table.getActiveTimeline().getCommitTimeline().filterCompletedInstants();
Review comment:
nope. cleaner could clean log files too.
##########
File path:
hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/table/TestCleaner.java
##########
@@ -1148,7 +1167,9 @@ public void testKeepLatestCommits(boolean
simulateFailureRetry, boolean enableIn
.withIncrementalCleaningMode(enableIncrementalClean)
.withFailedWritesCleaningPolicy(HoodieFailedWritesCleaningPolicy.EAGER)
.withCleanBootstrapBaseFileEnabled(enableBootstrapSourceClean)
-
.withCleanerPolicy(HoodieCleaningPolicy.KEEP_LATEST_COMMITS).retainCommits(2).build())
+ .withCleanerPolicy(HoodieCleaningPolicy.KEEP_LATEST_COMMITS)
+ .retainCommits(2)
Review comment:
yeah. can we try to add more tests around clean plan executor level. at
a higher level, we can have just one set of cleaner tests. But for diff
strategies would be better to avoid more tests at write client or table level.
--
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]
