pltbkd commented on PR #1354: URL: https://github.com/apache/flink-agents/pull/1354#issuecomment-6734741043
Thanks for the pr. Overall LGTM. There is a pre-existing issue that this change exposes on the non-coroutine path. It also affects the coroutine path, so perhaps we should open a separate issue to track and fix it. The issue is that ctx.chat shares the parent Action’s memory cache, and a child Action’s persistMemory() can flush the parent’s updates before the parent finishes. For example: 1. The parent increments a memory counter from 0 to 1. 2. It awaits ctx.chat(...); a child Action commits the shared cache. 3. The parent waits on another async call, and a checkpoint captures counter = 1 together with the unfinished parent. 4. On recovery, the parent replays from the beginning and increments the counter again, producing 2 instead of 1. This reproduces with RocksDB on both execution paths, including when ActionState logs are retained: reusing a completed call’s result does not skip the parent’s preceding memory update. The underlying memory-sharing behavior predates this PR. The previous pre-barrier drain avoided this checkpoint window on the non-coroutine path; removing it makes the existing issue reachable there as well. The follow-up should align the shared-memory commit boundary with the root Action’s replay boundary. -- 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]
