abdessalems commented on PR #11727: URL: https://github.com/apache/seatunnel/pull/11727#issuecomment-5417359245
@DanielLeens @davidzollo — thanks both, though I think you've pointed me in slightly different directions, and I'd rather ask than guess again. Daniel, you suggested landing this PR with just the getClassLoaders() guard and leaving the eviction and ownership work entirely to #11757. David, you'd prefer the ownership fix here — the tracker holding the TaskGroupContext it owns, with an identity-checked remove(location, ownedContext) — and #11757 rebasing on top, and you said you wouldn't merge this without it since this PR is what makes taskDone() reliably reachable on the failure path. That last point makes sense to me, so unless Daniel objects I'll go with David's direction. I'd keep it tight: just the tracker-owned context and the identity-checked remove, plus a cross-generation regression test. I'd leave the broader teardown work — cancellationFutures, async functions, timer flush, the synchronized block — to #11757, since that's clearly the more complete design there. @waterWang — sorry for the overlap, it wasn't intentional. If it lands this way, #11757 would need a rebase on top of this. Happy to coordinate on where the line sits so we don't write the same thing twice, and if you'd rather keep the identity check on your side I'm fine with that too — just say and I'll leave it out here. Holding off on pushing until this is settled. -- 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]
