Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

建议afterExecute()扩展点添加Runable和Throwable参数 #377

Closed
wuaping opened this issue Apr 24, 2022 · 9 comments
Closed

建议afterExecute()扩展点添加Runable和Throwable参数 #377

wuaping opened this issue Apr 24, 2022 · 9 comments
Assignees
Labels
📐 design discussion ❓question Further information is requested

Comments

@wuaping
Copy link

wuaping commented Apr 24, 2022

由于replayrestore都是在run方法里,所以ThreadPoolExecutor里的afterExecute()扩展点就无法继承TTL

image

@wuaping wuaping changed the title 建议添加afterExecute()扩展点 建议afterExecute()扩展点添加Runable和Throwable参数 Apr 24, 2022
@oldratlee
Copy link
Member

oldratlee commented Apr 24, 2022

看起来挺好。

你有什么实际的需求场景,需要用到额外的这2个参数吗? @wuaping

PS:

这个扩展点(TransmittableThreadLocal#afterExecute()),用的很少。
自己之前碰到的使用场景,其实找到合理的设计实现方式后,也不需要用了。

即我个人认为这个扩展点有些过度设计了,所以不想更复杂化了。

到了现在,因为库的兼容性,也不能删除掉这个扩展点了 😢

@oldratlee oldratlee added the ❓question Further information is requested label Apr 24, 2022
@wuaping
Copy link
Author

wuaping commented Apr 24, 2022

我现在遇到的场景:
我在打印日志时会有个traceId,使用TTL保存。
当在线程池里出现exception时,我实现了ThreadPoolExecutor里的afterExecute()方法来打印异常日志。但是发现此时的TTL已经被restore了。
image

我发现了TTL也有afterExecute()的扩展点,但是没有入参,所以这个扩展点无法实现相关业务。

我现在的解决方案是:只能是实现spi里的TtlEnhanced来修改run方法,进行catch并打印日志

我的建议是TTLafterExecute()增加入参来丰富使用场景

@oldratlee
Copy link
Member

oldratlee commented Apr 24, 2022

线程池执行的异常,看起来/直觉上 应该在线程池的扩展方法中解决(就是你上面示例代码的ThreadPoolExecutor#afterExecute方法中)。

一个执行的任务 涉及 多个TTL实例,生命周期/职责也不对。

TTL的扩展方法来 拦截处理 一个TTL实例自己数据,至少在生命周期/职责是对的。

一个例子参见 Issue:
(后面找到合适的设计方案后,这个场景也不需要用了TTL的扩展方法了 😂)


我的建议是TTL的afterExecute()增加入参来丰富使用场景

个人觉得, @wuaping

  • 应该做好 聚焦核心的功能
  • 而不是提供 不正交功能(或说过度设计、看起来可以用的功能)
    这会引发带来 业务的交错使用方式和混乱职责设计,系统难维护推理(或说容易魔幻)
    虽然刚开始可能实现得快

@wuaping
Copy link
Author

wuaping commented Apr 24, 2022

谢谢指导🙏

不过这个场景与我的不太一样,
我的目的是为了监控线程池的异常,线程池异常如果不进行异常打印的话,堆栈信息无法被ELK收集

@wuaping
Copy link
Author

wuaping commented Apr 24, 2022

我理解并认同您说的职责,

我在思考因为TTL是在Runnable上实现数据的复制和restore,但是在数据方面牺牲了线程池的afterExecute()扩展点,

是否考虑在TtlRunnable上新增afterExecute()扩展点

@oldratlee
Copy link
Member

oldratlee commented Apr 24, 2022

我在思考因为TTL是在Runnable上实现数据的复制和restore,但是在数据方面牺牲了线程池的afterExecute()扩展点

是否考虑在TtlRunnable上新增afterExecute()扩展点

为什么『在TtlRunnable上实现数据的复制和restore,在数据方面,会牺牲了 线程池的afterExecute()扩展点』? @wuaping


  • 线程池的afterExecute()扩展点 与 线程池中执行的Runnable 应该是正交的。
    • 在线程池看来,执行的都是 普通的Runnable
      (或者说成:不关心具体的实现是什么,TtlRunnable or 其它Runnable实现类)
  • TtlRunnable这层Wrapper也没有 呑异常。
    (即出异常的情况下,使用TtlRunnable异常保持不变)

@wuaping
Copy link
Author

wuaping commented Apr 24, 2022

『在TtlRunnable上实现数据的复制和restore,在数据方面,牺牲了线程池的afterExecute()扩展点』
--- 从使用者角度觉得,在使用TTL后,线程池的afterExecute()里无法使用TTL的数据业务,而且还是同一个线程

我能理解您想表达TTL的职责边界就是Runnable,所以我才会想在Runnable去添加一些扩展点来弥补这种使用场景

我认可你所说的聚焦核心的功能,thx anyway

@oldratlee
Copy link
Member

oldratlee commented Apr 24, 2022

从使用者角度觉得,在使用TTL后,线程池的afterExecute()里无法使用TTL的数据业务

大概明白你说的了 😅 @wuaping

你说的『TTL的数据业务』是指 业务提交的Runnable 吗?


如果是,在使用TTL后,线程池的afterExecute()中 也是一样可以使用TTL的业务数据(即提交的业务Runnable):

  • 对于TTL Agent的使用方式
    • 线程池的afterExecute()回调收到的是 提交的业务Runnable
      即不是TtlRunnable(没有TTL wrap过)。
    • 即自动的wrap 对应了 自动unwrap。这样对应一致的方式 保证了 对用户使用的透明。
    • 注意使用新的TTL版本:在版本v2.12.6中有修复这个处理逻辑的Bug。
  • 对于TTL API(如TtlExecutors#getTtlExecutorService())手动wrap的使用,则需要对应的手动做unwrap
    protected void afterExecute(Runnable r, Throwable t) {
        // unwrap TaskFuture
        // 由AbstractExecutorService#submit生成的TaskFuture
        // (这个识别要准确要再看看想想)
        ... ...
        // 由AbstractExecutorService#submit生成的TaskFuture运行完成后,
        // TaskFuture引用的原始Runnable/Callable会被置空,拿不到了
        // PS:
        //  - submit生成的TaskFuture也让`TTL Agent`自动`TTL unwrap`失效了。
        //  - submit生成的TaskFuture这样的实现方式,真的很不方便统一拦截!…… 😫 
        //    要解好,看着就觉得累人…
    
        // unwrap TtlRunnable
        r = TtlRunnable.unwrap(runnable);
    
        // biz code
        ... ...
    }

more note about TaskFuture

对于AbstractExecutorService#submit()方法,入参Runnable/Callable会转成TaskFuture
TaskFuture完成时,置空入参转入的Runnable/Callable(即业务Runnable)。

需要重写protected AbstractExecutorService#newTaskFor(),来实现TaskFuture完成时,还能拿到入参Runnable/Callable

上面这些获取业务对象的处理逻辑 正交于 是否使用 TtlRunnable

@wuaping
Copy link
Author

wuaping commented Apr 25, 2022

👍十分感谢

@wuaping wuaping closed this as completed Apr 25, 2022
@oldratlee oldratlee self-assigned this Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📐 design discussion ❓question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants