fhan688 commented on code in PR #19226:
URL: https://github.com/apache/hudi/pull/19226#discussion_r3575825119
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/rollback/MergeOnReadRollbackActionExecutor.java:
##########
@@ -90,4 +91,9 @@ protected List<HoodieRollbackStat>
executeRollback(HoodieRollbackPlan hoodieRoll
log.info("Time(in ms) taken to finish rollback {}",
rollbackTimer.endTimer());
return allRollbackStats;
}
+
+ private boolean shouldExecuteRollback(HoodieInstant resolvedInstant) {
+ return !resolvedInstant.isRequested()
+ ||
HoodieTimeline.COMPACTION_ACTION.equals(instantToRollback.getAction());
Review Comment:
> do we still need this fix since the restore procedure can already handle
the compaction rollback?
Yes, this is still needed at the rollback executor level. The restore
procedure eventually delegates to the same MOR rollback executor, so restore
support does not make this branch redundant.
The two changes work together: BaseRollbackPlanActionExecutor now
generates rollback requests for requested compaction, and
MergeOnReadRollbackActionExecutor must execute those requests. Without this
branch, the requested-compaction rollback plan can be produced but skipped
because resolvedInstant.isRequested() is true.
That said, this is not meant to change restore_to_instant procedure
semantics. It covers the lower-level rollback path for a requested compaction
that already has files associated with the compaction instant, which can happen
in abnormal/partial cleanup scenarios. I can clarify the code comment/test name
to make that scope explicit.
--
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]
