SEZ9 commented on PR #12494: URL: https://github.com/apache/seatunnel/pull/12494#issuecomment-5903171247
Thanks for the follow-up at 7edb4031. Going back over the earlier points, I'd like to confirm a few things against the actual changes before signing off: - **F1 / F2 (location-keyed CANCELED applied to the new generation):** could you point me to where the terminal notification of a reset-flagged context is suppressed, and confirm it covers both orderings — the old generation's callback firing before the redeploy lands, and after the new context has already been registered at the same `TaskGroupLocation`? The second ordering is the one that flipped the fresh vertex to CANCELED and released its slot. - **F3 (cascade under the service monitor on the merge thread):** does `deployTask` still hold the monitor across `taskGroup.init()`, per-task `task.init()` and `startedLatch.await()` at this head? If so, a merge arriving during connector initialisation can still block on it, even if the `reset()` side no longer runs under the monitor. - **F4 (non-deterministic unit test):** could you confirm the test now uses a task that ignores interrupts so the "cancelled but still active" deploy branch is actually hit? That was the gap I was after. - **F5 (IT topology):** I didn't spot a change here. As far as I can tell the IT still resets workers that never left the cluster while the master tracks them as RUNNING, so it exercises a failover-like path rather than a real split-then-merge. If a true split/merge is not feasible in the e2e harness, a short comment in the test stating what it does and does not cover would be fine with me. - **F6 (`SeaTunnelServer.reset()` worker-only):** if keeping this worker-only is intentional for this PR, that's acceptable to me — please just add a comment in `reset()` noting that on MASTER_AND_WORKER members the coordinator, slot service and finished-context bookkeeping are not reset, so the gap is visible to the next person. Summary of asks: (1) pointers/confirmation for the two orderings in F1/F2, (2) whether `deployTask` still holds the monitor across init/latch await (F3), (3) confirmation of the interrupt-ignoring task in the unit test (F4), (4) either adjust the IT or document its scope (F5), (5) the `reset()` comment for F6. <!-- streview-comment:1418 --> -- 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]
