Ashfaqbs commented on code in PR #1111:
URL: https://github.com/apache/flink-agents/pull/1111#discussion_r4012312655
##########
plan/src/main/java/org/apache/flink/agents/plan/actions/ToolCallAction.java:
##########
@@ -206,6 +212,12 @@ private static void executeParallel(
for (int i = 0; i < outcomes.size(); i++) {
recordOutcome(executions.get(i), outcomes.get(i), ctx,
success, error, responses);
}
+ } catch (InterruptedException e) {
Review Comment:
Good catch, thanks. Pushed c27c9647: `finalizeExecutedOutcomes()` in
`JavaRunnerContextImpl` now checks whether an executed outcome's error is an
`InterruptedException` (which is exactly what both the JDK 11 fallback and JDK
21 continuation batch executors fold a cancelled supplier into), restores the
interrupt flag, and rethrows before calling `finalizeCallAt` for that slot.
Slots finalized earlier in the loop keep their persisted outcome; that slot and
everything after it in iteration order stay in their already-reserved pending
state, so recovery re-executes them instead of replaying a false failure. The
rethrow propagates up through `durableExecuteAllAsync()` into
`executeParallel()`'s existing `catch (InterruptedException e)`, which was
already correct.
Added
`testDurableExecuteAllAsyncInterruptionLeavesRemainingSlotsPendingAndPropagates`
using the existing `InspectingContinuationActionExecutor` double (mirrors the
real executors' catch-and-wrap behavior) rather than mocking
`durableExecuteAllAsync()` directly, so it exercises the actual
`JavaRunnerContextImpl` path this bug lived in.
--
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]
