sangkyoonnam commented on code in PR #1172:
URL: https://github.com/apache/flink-agents/pull/1172#discussion_r4235329018
##########
runtime/src/main/java/org/apache/flink/agents/runtime/operator/ActionExecutionOperator.java:
##########
@@ -312,6 +319,22 @@ public void processElement(StreamRecord<IN> record) throws
Exception {
// Otherwise, the new event is processed immediately. Its failures are
attributed to the
// input run created by processInputEvent.
processInputEvent(key, inputEvent);
+ waitForCurrentInputInBatchMode();
Review Comment:
In the rebased code, this call and the one at L311 run while
`processElement` holds `ParallelExecutionLock`. `waitInFlightEventsFinished()`
then tries to acquire it again. I'd remove both and add one call after the
`if/else` in `processElement`, as in the summary. The drain still finishes
before the next record can switch keys.
Could you add a serial variant of the five-key batch test with
`AgentExecutionOptions.PARALLEL_EXECUTION_ENABLED=false` in the plan config?
After rebasing, I'd also pin one batch test to the parallel engine: set the
option to `true` and add
`assumeFalse(ContinuationActionExecutor.isContinuationSupported())`, like the
parallel tests in `ActionExecutionOperatorTest`. The current batch tests rely
on the default engine selection.
##########
runtime/API.md:
##########
Review Comment:
Optional cleanup: I'd remove `runtime/API.md` and `runtime/Method.md`.
Nothing in the repository references them, and they sit outside `docs/content`,
so the docs build doesn't publish them. The Javadoc on
`waitForCurrentInputInBatchMode` already states the contract. If no user-facing
doc change is planned, `doc-not-needed` fits better than `doc-included`. A note
that BATCH now works would belong under `docs/content`.
--
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]
