yihua commented on code in PR #5688:
URL: https://github.com/apache/hudi/pull/5688#discussion_r883395786
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/rollback/BaseRollbackActionExecutor.java:
##########
@@ -88,11 +88,11 @@ public BaseRollbackActionExecutor(HoodieEngineContext
context,
this.resolvedInstant = instantToRollback;
this.deleteInstants = deleteInstants;
this.skipTimelinePublish = skipTimelinePublish;
- this.useMarkerBasedStrategy = useMarkerBasedStrategy;
- if (useMarkerBasedStrategy) {
- ValidationUtils.checkArgument(!instantToRollback.isCompleted(),
- "Cannot use marker based rollback strategy on completed instant:" +
instantToRollback);
+ if (instantToRollback.isCompleted() && useMarkerBasedStrategy) {
+ useMarkerBasedStrategy = false;
+ LOG.warn("Cannot use marker based rollback strategy on completed
instant:" + instantToRollback + ", the strategy has been automatically
disabled.");
Review Comment:
The validation is required because the marker-based rollback cannot be
applied to a complete instant since there is no marker for complete instants.
Instead of fixing the logic here to hide the anti-usage pattern, the caller
of `BaseRollbackActionExecutor` should make sure that marker-based rollback is
not used for completed instants. @watermelon12138 could you clarify which UTs
are relevant and why we need the fix here?
--
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]
