sangkyoonnam opened a new pull request, #1211: URL: https://github.com/apache/flink-agents/pull/1211
Linked issue: Closes #1189 ### Purpose of change A fresh completion-only `durable_execute` call, or an individually awaited `durable_execute_async` call without a reconciler, now raises when Java fails to persist its call result. Before, the error was logged at most and the call returned success, so recovery from that journal ran the call again. #### Runtime flow `_record_call_completion` serializes the result and exception, then calls `recordCallCompletion` on the Java runner context. Serialization stays inside the `try`: a result that can't be pickled is logged and returned unrecorded, as before. The Java write now sits after the `try`, so its error propagates through `_execute_and_record_completion_only` and the async completion-only wrapper to the caller. #### Key decisions - Propagate rather than retry or log: the pending-slot path already calls `finalizeCurrentCall` without a `try`, and Java's `durableExecuteCompletionOnly` lets `recordDurableCompletion` errors through. This makes the Python completion-only path match both. - Drop the special case that stayed silent when the error text contained `recordCallCompletion`. It guarded against a missing Java method, but the Python bridge's `PythonRunnerContextImpl` already inherits it from `RunnerContextImpl`, which skips persistence without a durable context. Stacked on #1182, which changes the same function. Only the last commit is new. ### Behavioral Semantics #### Interaction decisions | Call outcome | Serialization | Java write | Caller sees | |---|---|---|---| | returns | ok | ok | the result (unchanged) | | returns | ok | fails | the write error (was: the result) | | returns | result can't be pickled | not attempted | the result, unrecorded (unchanged) | | raises | ok | ok | the call's exception (unchanged) | | raises | ok | fails | the write error, replacing the call's exception (was: the call's exception) | | raises | fails | not attempted | the call's exception, unrecorded (unchanged) | | detected Java interruption | not attempted | not attempted | the original exception (unchanged) | #### Behavioral contracts 1. When the Java write fails after a successful call, sync `durable_execute` raises the write error instead of returning the result. 2. The same holds for an individually awaited `durable_execute_async` without a reconciler. 3. A result that can't be pickled is still returned, with no write attempted. 4. When the call itself raised and the write fails, the caller gets the write error, and the call's exception is not attached as its cause, as in Java's `durableExecuteCompletionOnly`. On the async path its implicit `__context__` is the internal wrapper that holds the call's exception; on the sync path it has none. #### Failure behavior The Java write error is raised as-is, with no retry and no log on this path. Serialization failures are absorbed as before, and now always logged at warning level; the old filter also hid a serialization error whose text contained `recordCallCompletion`. Pending-slot and `gather` paths are untouched: `finalizeCurrentCall` already propagates, and `gather` turns a `finalizeCallAt` failure into that item's failure. ### Tests | Contract | Tests (`test_flink_runner_context_reconcilable.py`) | |---|---| | 1 | `test_call_result_persistence_failure_reaches_the_caller[False]` | | 2 | `test_call_result_persistence_failure_reaches_the_caller[True]` | | 3 | `test_result_that_cannot_be_pickled_is_returned_unrecorded[False\|True]` | | 4 | `test_persistence_failure_replaces_the_call_exception[False\|True]` | The changed behavior (1, 2, 4) fails without the change: four of the six new cases fail on the parent commit, and contract 3's two pass on both. Not verified: - A real persister failure through pemja; the tests fail a fake Java runner context the way `DurableExecutionManager.persist` does. - Recovery after the raised error. The issue's Py4J probe expects the first call to return, so it doesn't reach that path. - An action that catches the write error and retries the same call. Java keeps the unpersisted result in memory and doesn't advance the call index, so a retry may replay it, as on the Java path. <details> <summary>Implementation invariants and test runs</summary> - The fake Java context counts one write attempt and stores no call result when the write fails; the call itself runs once. - `pytest flink_agents -k "not e2e_tests" -m "not integration"`: 1796 passed, 15 skipped. `ruff check` passes. </details> ### API No signature changes. A durable call whose result can't be persisted now fails instead of returning; nothing changes when the write succeeds, when the result can't be pickled, or on the pending-slot and `gather` paths. ### Documentation - [ ] `doc-needed` - [x] `doc-not-needed` - [ ] `doc-included` ### Was this patch authored or co-authored using generative AI tooling? - [x] Yes - [ ] No Generated-by: Claude Code 2.1.292 (Claude Opus 5.5) -- 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]
