SEZ9 commented on PR #12271: URL: https://github.com/apache/seatunnel/pull/12271#issuecomment-5674461101
Quick follow-up on my three earlier points, looking at the `0f0e78a4c8` merge. I don't have the merged diff in front of me here, so I'd rather confirm than assert: **F1 – location-keyed terminal cleanup.** The intent is that cleanup now goes through the identity-checked `finishExecution()` / `executionContexts.compute()` path and that `getTaskClassLoader()` fails fast rather than returning a silent `null`. Could you point me at the relevant hunk in the merge so I can close this out? **F2 – `BlockingWorker` reading the location-keyed map.** I understand `BlockingWorker` received a latch-before-lookup ordering fix. Does it now also resolve the loader from the tracker-owned context rather than the location-keyed map? If so, a pointer to that change would let me mark this resolved. **F3 – regression test coverage.** Still open as a coverage gap, though non-blocking: neither existing unit test drives the `BlockingWorker` or `CooperativeTaskWorker` path through a real worker iteration. An end-to-end test that publishes two `TaskGroupContext` generations at the same `TaskGroupLocation`, runs an older-generation task through the real worker, and asserts it resolves the older generation's loader while leaving the newer context intact would be a good follow-up. With F1/F2 confirmed, nothing else is open from my side. Thanks! <!-- streview-comment:1068 --> -- 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]
