SEZ9 commented on PR #11727: URL: https://github.com/apache/seatunnel/pull/11727#issuecomment-5628492921
@abdessalems thanks for the follow-up on f1e02f942. Taking the points in order: **F3 / F1** — Thanks for laying out the structure. If the redeploy guard and the `deployLocalTask(...)` call both sit inside the `synchronized (this)` block opened by `deployTask(TaskGroupImmutableInformation)` as you describe, then the "same monitor" claim holds for that entry point, and I need to revisit the premise of F1 rather than just re-scope it. I'll re-read the diff on this head to confirm the enclosing block and the reworded Javadoc before marking either finding resolved; I'd rather verify against the code than close them on the description alone. If it checks out, the remaining gap is only a caller reaching `deployLocalTask` directly, and the follow-up issue you pointed at for that path is fine with me. **F8** — The comment at `finishExecutionContext` explaining why the stale branch deliberately stays out of `finishedExecutionContexts` addresses the intent question. One small leftover from the same finding: did the redundant `get` before `remove(key, value)` also get dropped in this push, or is it still there? Either is acceptable, I just want to record the state correctly. **F7** — Decision: keep the reflection. I don't want to widen the production API of `TaskExecutionService` purely for test access, and `ReflectionUtils.getField` on those four fields is tolerable for a race-regression test. Since the redeploy-vs-taskDone coverage naturally moves with F1/F5, please add a one-line note to the PR description that the test for that race is deferred with them. **F1/F5** — Seen the note in the PR description; that matches what we discussed. Remaining asks: (1) confirm the F8 `get`/`remove` status, (2) the deferred-test note for F7 in the description. I'll also want to see a green CI run on this head before signing off. <!-- streview-comment:954 --> -- 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]
