SEZ9 commented on PR #11727: URL: https://github.com/apache/seatunnel/pull/11727#issuecomment-5390960726
Thanks @DanielLeens for the thorough follow-up, and for verifying that the merge commit `51c6a46a` on top of `72268def` is a no-op on `TaskExecutionService.java` and `TaskDeployStaleContextRaceTest.java` — that saves everyone a redundant re-review pass. I agree both points you've carried forward are in scope for this PR and should be fixed here rather than deferred, since they sit in the same method and the same stale-generation race family this fix targets: 1. **Cross-generation eviction race** — the suggested direction sounds right to me: `taskDone()` should capture `TaskGroupContext finishedContext = executionContexts.remove(taskGroupLocation);` (or better, an ownership-aware `remove(key, value)` with an identity check against the context pinned at deploy time) and recycle only that specific instance, instead of `recycleClassLoader()` re-resolving by key with its own `executionContexts.get(taskGroupLocation)`. Key-only cleanup is unsafe precisely because `TaskGroupLocation` is reused across restore generations, which is the premise of this fix. 2. **NPE / silent-null class loader path** — `taskGroupContext.getClassLoaders().get(t.getTaskID())` in `BlockingWorker.run()` needs a guard after the existing null-context check, and a missing per-task entry should fail fast rather than silently installing a null context class loader. Concrete asks before merge: - Push a fix commit addressing both points (ownership-aware cleanup in `taskDone()`/`recycleClassLoader()`, plus the guard/fail-fast in `BlockingWorker.run()`). - If feasible, extend `TaskDeployStaleContextRaceTest` (or add a companion test) to cover the cross-generation scenario where a newer generation installs its context under the same key before the older generation's `taskDone()` runs, so the eviction race stays regression-tested. @DanielLeens once the fix commit lands, a fresh pass from you on just those two locations would be much appreciated. And as mentioned earlier, feel free to ping the #contributors channel on the SeaTunnel Slack if CI gives you any trouble. <!-- streview-comment:518 --> -- 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]
