DanielLeens commented on PR #11727: URL: https://github.com/apache/seatunnel/pull/11727#issuecomment-5649652966
@abdessalems Thanks for flagging this explicitly rather than letting the two land in conflict — that's the right call to ask for. I checked both PRs' file lists directly rather than going from memory: #12238 touches `TaskExecutionService.java` + `TaskGroupContext.java` (its re-keying model) plus its own new test, while this PR touches `TaskExecutionService.java` plus `TaskExecutionServiceTest.java` — the same test file #12238 also modifies. So the overlap isn't just conceptual (two ownership models for the same problem), it's a literal same-file, same-test-file collision if both land as-is. My recommendation: let #12238 land first. It's already gone through a full review round from me with no outstanding code-side blocker and green CI, and re-keying by `TaskGroupContext` is the more self-contained fix — it doesn't need to introduce a new tagging concept (`OwnedFuture`) into `TaskExecutionService`, it scopes the fix to the actual execution-context boundary. Once it's in, I'd ask you to rebase this PR back to its original, narrower scope — the unconditional `startedLatch` release for #11679 — and drop the three cherry-picked `OwnedFuture` commits, since #12238 will already have solved the ownership-tagging problem they were addressing. That keeps this PR focused on the bug it originally set out to fix and avoids maintaining two parallel ownership models in the same methods. To be clear, the final call on sequencing is a maintainer decision (looping in @davidzollo and @SEZ9 as you did) — this is my technical read based on the actual diffs, not an official ruling. But from where I sit, "strip this back to the latch fix and let #12238 carry the ownership-model fix" is the cleaner outcome for the codebase. Appreciate you keeping this constructive instead of just letting the merge conflict happen — that made this a much easier question to answer. -- 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]
