danny0405 commented on code in PR #13340:
URL: https://github.com/apache/hudi/pull/13340#discussion_r2113093212
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/clean/CleanPlanActionExecutor.java:
##########
@@ -166,56 +163,49 @@ HoodieCleanerPlan requestClean(HoodieEngineContext
context) {
private Map<String, String> prepareExtraMetadata(List<String>
savepointedTimestamps) {
if (savepointedTimestamps.isEmpty()) {
- return Collections.emptyMap();
+ return extraMetadata.orElse(Collections.emptyMap());
} else {
- Map<String, String> extraMetadata = new HashMap<>();
- extraMetadata.put(SAVEPOINTED_TIMESTAMPS,
savepointedTimestamps.stream().collect(Collectors.joining(",")));
- return extraMetadata;
+ Map<String, String> metadataWithSavepoints = extraMetadata.orElseGet(()
-> new HashMap<>());
+ metadataWithSavepoints.put(SAVEPOINTED_TIMESTAMPS,
savepointedTimestamps.stream().collect(Collectors.joining(",")));
+ return metadataWithSavepoints;
}
}
/**
* Creates a Cleaner plan if there are files to be cleaned and stores them
in instant file.
* Cleaner Plan contains absolute file paths.
*
- * @param startCleanTime Cleaner Instant Time
* @return Cleaner Plan if generated
*/
- protected Option<HoodieCleanerPlan> requestClean(String startCleanTime) {
+ protected Option<HoodieCleanerPlan> requestClean() {
// Check if the last clean completed successfully and wrote out its
metadata. If not, it should be retried.
Option<HoodieInstant> lastClean =
table.getCleanTimeline().filterCompletedInstants().lastInstant();
- if (lastClean.isPresent()) {
+ while (lastClean.map(table.getActiveTimeline()::isEmpty).orElse(false)) {
HoodieInstant cleanInstant = lastClean.get();
HoodieActiveTimeline activeTimeline = table.getActiveTimeline();
- if (activeTimeline.isEmpty(cleanInstant)) {
- activeTimeline.deleteEmptyInstantIfExists(cleanInstant);
- HoodieInstant cleanPlanInstant = new
HoodieInstant(HoodieInstant.State.REQUESTED, cleanInstant.getAction(),
cleanInstant.requestedTime(),
InstantComparatorV1.REQUESTED_TIME_BASED_COMPARATOR);
- try {
- // Deserialize plan.
- return Option.of(activeTimeline.readCleanerPlan(cleanPlanInstant));
- } catch (IOException ex) {
- // If it is empty we catch error and repair.
- if (activeTimeline.isEmpty(cleanPlanInstant)) {
- return Option.of(new HoodieCleanerPlan());
- }
+ activeTimeline.deleteEmptyInstantIfExists(cleanInstant);
+ HoodieInstant cleanPlanInstant =
instantGenerator.getCleanRequestedInstant(cleanInstant.requestedTime());
+ try {
+ // Deserialize plan.
+ return Option.of(activeTimeline.readCleanerPlan(cleanPlanInstant));
+ } catch (IOException ex) {
+ // If it is empty we catch error and repair by deleting the empty plan
and inflight instant.
Review Comment:
+1, looks more reasonable now in general.
--
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]
