SEZ9 commented on PR #11727: URL: https://github.com/apache/seatunnel/pull/11727#issuecomment-5882695366
Thanks for the detailed walkthrough against `a80834ebf` (same as `06c2ef03e` for the touched files). I can't mark the earlier points resolved from a description alone, so a few concrete asks before this is ready: **Redeploy-vs-in-flight-`taskDone()` race (F1 / F5 / F7)** – I'm fine scoping that half out of this PR if the boundary is documented in code. Please point to the diff hunk in the race test that states this, and add a matching note where `deployLocalTask()` installs the new context. **`finishOwnedResources()` Javadoc (F3)** – it still says it runs "under the same monitor used by deployment", while `deployLocalTask()` does not take that monitor around the context installation. Please either reword the Javadoc to describe the actual guarantee (per-key `executionContexts.compute(...)` ownership check) or add the monitor. **Teardown under the service-wide monitor (F6)** – if `recycleClassLoader` / `cancelAsyncFunctionFutures` / `cancelTimerFlushFutures` now run after the `compute(...)` transition and outside any `synchronized` block, please point to that hunk in the diff so it can be verified. **Reflection in the race test (F7)** – accepting reflection as a documented tradeoff is reasonable; please point to the diff hunk where that justification lives. **Stale path (F4 / F8)** – your summary was cut off at item 4, so these are still open: - Does the stale-cleanup branch cancel the old generation's async functions and timer-flush tasks after recycling class loaders? If it is intentionally deferred, please say so in a code comment. - Should the stale path move `ownedContext` into `finishedExecutionContexts`, and has the redundant `get` before `remove(key, value)` in `finishExecutionContext()` been dropped? **`BlockingWorker` context resolution (F2)** – not covered in your summary; does it now use the tracker's `ownedContext` rather than the shared `executionContexts` map? Once these are answered with the corresponding diff references, I'm happy to proceed as @zhangshenghang indicated. <!-- streview-comment:1397 --> -- 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]
