BinTaoMa commented on PR #11999:
URL: https://github.com/apache/seatunnel/pull/11999#issuecomment-5475016673
Thanks @JeremyXin and @DanielLeens for the detailed comparison.
I agree with the proposed consolidation path. #11999 is intended to
continue #11757 rather than compete with it: it
preserves @waterWang's original commits and authorship, rebases the fix
onto dev, and closes the remaining stale-
generation classloader cleanup gap.
Given its smaller scope and current clean merge state, I suggest using
#11999 as the focused fix for #11755. After it
lands, #11757 can be closed as superseded with a pointer back here. #11924
can continue as follow-up hardening for
generation-scoped async/timer registries after its remaining
atomic-cleanup races are addressed and it is rebased on
the landed fix.
Please let me know if you prefer a different consolidation path. Thanks!
> 你好@JeremyXin感谢指出这一点——你说得对，实际上是三方重叠，而不仅仅是两方重叠。我对比了这三个 PR
的实际差异，`TaskExecutionService.java`发现如下：
>
> **#11757和#11999是同一个修复，并非两个相互竞争的实现。** #11999的描述中明确指出它是“[ #11757 的延续”，git
历史记录也证实了这一点：
](https://github.com/apache/seatunnel/pull/11757)#11999的前三个提交[与#11757](https://github.com/apache/seatunnel/pull/11757)的当前版本完全相同，保留了……@waterWang的原始作者身份。#11999中唯一的新提交增加了一个额外的步骤——在返回之前释放过时的生成层自身的类加载器——填补了我之前在[#11757
的](https://github.com/apache/seatunnel/pull/11757)几轮审查中提出的一个空白。因此，这两个提交只需要协调：合并#11999（这是一个干净的
rebase
操作`dev`；#11757目前显示为`dirty`/conflicting），然后将#11757关闭，并附上一个指示注释。@waterWang保留对最初修复工作的贡献。
>
> **#11924针对同一根本原因（#11755）采取了不同的、更广泛的方法。** #11757 /[
#11999](https://github.com/apache/seatunnel/pull/11999)使用身份检查来保护共享的`executionContexts`映射，`cancellationFutures`以防止过时的代完成时驱逐较新的代的活动上下文。[
#11924](https://github.com/apache/seatunnel/pull/11924)更进一步：它引入了一个`ExecutionGeneration`贯穿整个映射的显式标记`TaskExecutionContext`，并使用它来限定每个代的异步函数未来和定时器刷新未来注册表的范围，而不仅仅是每个映射`TaskGroupLocation`。这是同一类
bug 的一个真正额外的优势——在范围更窄的#11757 /[
#11999](https://github.com/apache/seatunnel/pull/11999)修复中，这两个注册表仅按位置键，因此原则上，仍在运行的代的异步/定时器资源仍然可能被共享该位置的同级代以不同时间点的清理调用访问。其代价是更大的影响范围
：一个新的公共嵌套类型以及大约六个文件中的多个调用点签名更改。
>
> **[即使撇开上述重叠不谈，
#11924](https://github.com/apache/seatunnel/pull/11924)本身也还不能合并。**它目前的版本仍然存在一个我上次审查时遗留的未解决的阻塞问题：几个新的、作用于代的清理路径使用了非原子性的`isEmpty()`-then-``remove(key,
map)`模式，这可能会导致并发寄存器调用发生竞争。其中一个完全相同的实例已被发现并修复，但截至目前，仍有大约五个结构相同的问题尚未解决。
>
>
**建议的后续步骤：**首先合并#11999，因为它是针对核心竞态条件的更简洁、已审核且准备就绪的修复方案；关闭#11757，因为它已被取代。[#11924为异步/定时器
future
添加了基于注册表的生成范围限制，这是对](https://github.com/apache/seatunnel/pull/11924)#11999合并内容的有益后续强化，前提是其自身的并发问题已修复并基于新的基线进行重构。@BinTaoMa
@zhangshenghang- 等您有机会查看之后，我很乐意再仔细检查一下这两个
PR；我想在提出方向之前，先阐明两种方法之间的实际技术差异，而不是直接选择其中一种。
--
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]
