SEZ9 commented on PR #11727: URL: https://github.com/apache/seatunnel/pull/11727#issuecomment-5650209500
@abdessalems thanks for raising this before either side merges — my preference is option one: let #12238 carry the ownership model and strip this PR back to the unconditional `startedLatch` release for #11679. The reasoning is mostly about the open findings on this thread. Nearly all of them (F1–F6, F8) are about the cherry-picked ownership model rather than the latch fix: the deploy-vs-`taskDone` race on `cancellationFutures` isn't actually closed because `deployLocalTask()` installs the context and cancellation future without the monitor `finishOwnedResources()` claims to share (and its Javadoc says otherwise), `BlockingWorker` still resolves its context from the shared `executionContexts` map instead of the tracker's `ownedContext`, the stale-cleanup branch returns without cancelling the old generation's async functions / timer flushes, the plain `put` overwrites a still-live older generation, and the heavy teardown now runs under the service-wide monitor. Fixing all of that here while #12238 re-keys the same futures by `TaskGroupContext` would mean converging two designs in the same methods, which is exactly the conflict you're worried about. Keeping #11727 to the latch fix sidest eps that and gives #11679 a small, easy-to-land change. Concretely, what I'd ask for: - Drop 588314d9c, 1fed68145 and 912e8bd4c (and the follow-up work that builds on the `OwnedFuture` tagging) from this branch, leaving only the unconditional `startedLatch` release. - Rework `TaskDeployStaleContextRaceTest` accordingly: keep whatever exercises the deploy-returns / latch contract, drop the parts that reflect into the ownership internals that will no longer exist here (F7). - Please carry the substantive points above (F1, F2, F4, F5, F6, F8) over to the #12238 discussion so they aren't lost when the ownership model lands there. Once the branch is trimmed I'll take another look and we can get #11679 closed independently of #12238's timing. <!-- streview-comment:1003 --> -- 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]
