weiqingy commented on issue #983: URL: https://github.com/apache/flink-agents/issues/983#issuecomment-5249537644
Thanks, that holds up. I traced it on `main`. The interleaving is real. When the action hits the await, the coroutine returns not-done (`plan/function.py:436`), the operator hands the task back unfinished and re-queues it as a new mail (`ActionExecutionOperator.java:502-508,537`), and `currentProcessingKeys` holds a list rather than one key (`OperatorStateManager.java:265,277`). So another key can run `switchContext` (`RunnerContextImpl.java:185`) while the first key's callable is still waiting on the pool. One correction to the mechanism. The worker does not read `self.key` when it calls Mem0. Each op copies it into a local on entry (`mem0_long_term_memory.py:488`), so the window is only from `executor.submit` (`flink_runner_context.py:159`) to the callable's first line. That is much narrower than it looks, but it is still a pool thread reading state the mailbox thread writes at `:310`, and there is no lock or thread-local in the class. Like you, I have not reproduced it. It is also wider than just the write path. `get` copies the key the same way (`:553`), so an item stored correctly can still be read back under the wrong `agent_id`. And `_observation_id` and `_observation_suppressed` are copied in the same three lines in all five ops (`:450, :488, :553, :608, :665`), so observations can be misattributed on their own. Would it work to capture the key at submit time and pass it into the callable, rather than reading shared state on the pool thread? Same question for the observation fields. On how to handle #984, given we still do not know the root cause. My preference is to keep it as detection rather than turn it into the fix. It now checks the cross-key leak in both directions, so if this race is what happened, the next occurrence says so instead of just reporting an empty set. The alternative is to repurpose #984 into the race fix, but then the leak check can never fire and we give up the chance to confirm the race was behind the original failure. Either way I would keep #983 open until we have a real root cause, rather than closing it when #984 merges. If you prefer the detection-first route, do you want to open a separate issue for the race, or shall I? -- 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]
