GitHub user Ashfaqbs added a comment to the discussion: Human-in-the-loop 
approval: pluggable approver (decision model first, human fallback) - design 
discussion for #1067

That `InputEvent` variant for approval responses matches what I'd want here, 
with one addition on the "avoid being buffered behind the task it needs to 
resume" requirement: the event needs to skip the operator's normal per-key 
action-queue entirely, not just be prioritized within it. If it goes through 
the same enqueue path as an ordinary keyed input, it still has to wait for its 
turn behind whatever's ahead of it for that key, which includes the task it's 
meant to unblock. The resume event needs its own short-circuit at 
`processElement` (or wherever keys get dispatched to the queue) that looks up 
the suspended task by its pending-approval id and resumes it directly, 
bypassing the FIFO queue for that key rather than jumping it to the front.

On timeout: I'd model it as a second event type racing the approval response 
rather than a special case inside it - a `ApprovalTimeoutEvent` carrying the 
same pending-approval id, scheduled via a processing-time timer when the task 
suspends. Whichever arrives first resumes the task and the other becomes a 
no-op (the suspended-task lookup by id returns nothing, since it's already 
gone). That keeps "what happens on timeout" as application-visible behavior 
(the resumed task sees "timed out" vs "approved" as the resume payload) instead 
of buried runtime policy, and it reuses the exact same bypass-the-queue resume 
path rather than needing a second mechanism.

On duplicate/late responses: this is the same problem as the idempotency-key 
point above, just one level up the stack. Once the suspended task is resumed 
and removed from the pending-approval index, a second response (duplicate 
delivery, or a human approving after a timeout already resumed it as rejected) 
finds nothing to resume by that id and should be dropped with a logged warning 
rather than erroring - silently accepting it twice is exactly the 
double-dispatch risk from the gist. If the audit trail needs to show a late 
response was discarded rather than just vanishing, that's a case for writing it 
to the pending-approval's `alert_data`-equivalent record *after* it's no longer 
pending, as a "late, ignored" entry rather than reopening the resumed state.

Suspension surviving recovery: since the suspension state (pending-approval id, 
the key, what to resume with) has to be durable action state anyway for the 
resume-by-id lookup to work after a restart, that's the same action-state store 
this thread's already assuming exists per #1067 - no separate persistence 
mechanism needed, just confirming recovery replays "still pending" rather than 
re-triggering the original request.

GitHub link: 
https://github.com/apache/flink-agents/discussions/1304#discussioncomment-19379425

----
This is an automatically sent email for [email protected].
To unsubscribe, please send an email to: [email protected]

Reply via email to