hudi-agent commented on code in PR #19946:
URL: https://github.com/apache/hudi/pull/19946#discussion_r4056082803
##########
hudi-timeline-service/src/main/java/org/apache/hudi/timeline/service/RequestHandler.java:
##########
@@ -759,14 +760,48 @@ private boolean isLocalViewBehind(Context ctx) {
return
!localTimeline.containsOrBeforeTimelineStarts(lastKnownInstantFromClient);
}
+ /**
+ * Determines whether the local view needs to be refreshed before handling
the request.
+ *
+ * <p>An exact extension does not require a refresh because reloading
cannot make the
Review Comment:
🤖 This premise doesn't hold when instants get deleted from storage, and I
think it's what's failing Azure on this commit:
`TestSavepointRestoreMergeOnRead#testCleaningDeltaCommits` /
`#testRestoreWithFileGroupCreatedWithDeltaCommits` fail 4/4 reruns with a 400
from `getLatestFileSlicesBeforeOrOn` in `ListingBasedRollbackStrategy` (earlier
commits were green). Restore's per-commit rollbacks run with
`skipTimelinePublish=true` and delete completed commit instants, so the
executor's reloaded timeline is an exact prefix of the server's now
stale-*ahead* view; master would sync here and catch back up, whereas this
skips the sync and the strict final check rejects. Since the final check only
ever serves a non-equal timeline when the extension is a trailing clean, could
the skip be narrowed to that case (e.g. reuse
`shouldThrowExceptionIfLocalViewBehind` on the prefix)? For any other extension
master syncs-then-throws anyway, so skipping buys nothing there but loses the
deleted-instan
t recovery.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
