danny0405 commented on code in PR #19226:
URL: https://github.com/apache/hudi/pull/19226#discussion_r3578284221
##########
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:
A requested compaction can be rolled back during restore.
`BaseRollbackPlanActionExecutor` line 106 does not filter out the instant; it
only prevents generating file-level rollback requests:
```java
if (!instantToRollback.isRequested()) {
rollbackRequests.addAll(...);
}
```
The resulting rollback plan still identifies the requested compaction, with
an empty rollbackRequests list: [BaseRollbackPlanActionExecutor.java (line
102)](/Users/chenyuzhao/workspace/hudi-dev/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/rollback/BaseRollbackPlanActionExecutor.java:102).
The subsequent flow is:
1. `RestorePlanActionExecutor` includes the requested compaction because
`getWriteTimeline()` contains `COMPACTION_ACTION`.
2. `MergeOnReadRestoreActionExecutor` accepts the compaction action and
schedules its rollback.
3. `MergeOnReadRollbackActionExecutor` explicitly treats a requested instant
as having no physical work:
```java
// nothing to do rollback other than deleting the timeline file
if (!resolvedInstant.isRequested()) {
...
}
```
See [MergeOnReadRollbackActionExecutor.java (line
81)](/Users/chenyuzhao/workspace/hudi-dev/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/rollback/MergeOnReadRollbackActionExecutor.java:81).
4. finishRollback() still invokes timeline deletion with
deleteInstants=true: [BaseRollbackActionExecutor.java (line
281)](/Users/chenyuzhao/workspace/hudi-dev/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/rollback/BaseRollbackActionExecutor.java:281).
5. deletePending(requestedCompactionInstant) removes the
.compaction.requested file and its plan: [BaseRollbackActionExecutor.java (line
307)](/Users/chenyuzhao/workspace/hudi-dev/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/rollback/BaseRollbackActionExecutor.java:307).
So the behavior is:
- No data files are deleted.
- No rollback requests are generated.
- The requested compaction plan is removed from the timeline.
- Restore records that instant as rolled back.
The current ordering—requested compaction first—is therefore fine.
One remaining gap is test coverage: testCleaningPendingCompaction calls
compactWithoutCommit(), so it tests an inflight compaction rather than a purely
REQUESTED one. A schedule-only restore test would explicitly lock in the
requested-compaction behavior.
--
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]
