[GitHub] [incubator-dubbo] lexburner opened issue #3324: [Discuss] The Timer Design in Dubbo from 2.6 to 2.7

["lexburner (GitHub)" <gi...@apache.org>]

> this issue is a copy of the email in the Dubbo mailing list, I will give the final conclusion here.

最近在研究 Dubbo 2.7 代码时，我发现了一个有意思的改动，Dubbo 2.6 中大量使用了 `ScheduledThreadPoolExecutor`，例如实现定时发送心跳，开启定时器用于判断异步请求超时等场景，在 Dubbo 2.7 之中替换为了由 Netty 实现的 `HashedWheelTimer`。

我对这一改动比较好奇，于是对定时器的实现做了一些调研，探讨目的是想为 2.7 选用一个最合适的定时器实现。

JDK 的 `Timer` 存在设计问题，不予讨论，我想主要对比 `HashedWheelTimer` 和 `ScheduledThreadPoolExecutor` 的区别。

Netty 在 3.x 中有大量使用 `HashedWheelTimer`，但是在 4.1 中，我们可以发现，Netty 保留了 `HashedWheelTimer`，但在其源码中并未使用它，而是选择了 `ScheduledThreadPoolExecutor`，在 Stack Overflow 中也有人提出这样的疑问：[efficient-timer-algorithm](https://stackoverflow.com/questions/867621/efficient-timer-algorithm)。

Netty 提供的 `HashedWheelTimer` 正如其 java doc 中所言：`A Timer optimized for approximated I/O timeout scheduling`. 是适用于固定场景的，如果有深入研究过时间轮算法，会发现，其并没有实现多级时间轮，时间轮算法可以参考这篇论文：[timing-wheels](http://www.cs.columbia.edu/~nahum/w6998/papers/sosp87-timing-wheels.pdf)

我并没有找到 2.7 有关于这个改动的邮件，不太清楚去除 `ScheduledThreadPoolExecutor`, 使用 `HashedWheelTimer` 的原因，不知道我的检查是否有所遗漏。

下面是四个讨论点，索引一下，方便大家讨论：

1. 2.7 使用 Netty 的 HashedWheelTimer 是为了解决什么现有问题？
2. 使用 ScheduledThreadPoolExecutor 可以更好的控制多线程执行定时任务（虽然现在的 HashWheelTimer 也可以做到），尽量保持了和 2.6 的风格一致。
3. Netty4.1 没有使用 HashedWheelTimer，我没有搜索到相关的原因，但我觉得 Netty 这款优秀的框架必然考虑到了什么，希望得到大家的讨论。由于 Netty 的这一点改动，我更倾向于在 Dubbo 中也使用 ScheduledThreadPoolExecutor
4. 在心跳这样的场景，我们可以把定时器交给 netty 的 eventLoop 去做 Schedule，这样并不是耦合，可以做成可配置的项来选择性的重载，可以保证 Timer 线程和请求线程的线程安全（这一点是参考了 IdleStateHandler 的设计）。在 Dubbo 2.7 中，Timer reput 的逻辑耦合在了抽象类中，这样导致了 Timer 并不具备很高的重用性。
5. HashedWheelTimer 和 ScheduledThreadPoolExecutor 分别适用的场景

期待大家的讨论

---

Recently, when I was researching Dubbo 2.7 code, I found an interesting change. Dubbo 2.6 used the ScheduledThreadPoolExecutor extensively. For example, Dubbo implemented a timed heartbeat and started the timer to judge the asynchronous request timeout. It is replaced in Dubbo 2.7 using HashedWheelTimer implemented by Netty.

I was curious about this change, so I did some research on the implementation of the timer. The purpose of the discussion was to choose the most suitable timer for 2.7.

JDK's Timer has design issues that is no use to be discussed. I want to compare the differences between HashedWheelTimer and ScheduledThreadPoolExecutor.

Netty has a lot of use of HashedWheelTimer in 3.x, but in 4.1, we can find that Netty retains HashedWheelTimer, but it is not used in its source code, but chose ScheduledThreadPoolExecutor, which was also proposed in Stack Overflow. Question: https://stackoverflow.com/questions/867621/efficient-timer-algorithm.

The HashedWheelTimer provided by Netty is as described in its java doc: A Timer optimized for approximated I/O timeout scheduling. It is suitable for fixed scenes. If you have studied the time wheel algorithm in depth, you will find that it does not implement multi-level time wheel. The time wheel algorithm can refer to this paper: http://www.cs.columbia.edu/~nahum/w6998/papers/sosp87-timing-wheels.pdf

I didn't find the 2.7 email about this change. I don't know why I removed the ScheduledThreadPoolExecutor and used HashedWheelTimer. I don't know if my check is missing.

Here are four discussion points, indexing, for your convenience:

1. 2.7 Is Netty's HashedWheelTimer used to solve any existing problems?
2. Use ScheduledThreadPoolExecutor to better control multi-threaded execution timing tasks (although HashWheelTimer can do it now), and try to keep the style consistent with 2.6.
3. Netty4.1 didn't use HashedWheelTimer, I didn't search for related reasons, but I think Netty's excellent framework must take into account what I want, and I hope to get everyone's discussion. Due to this change in Netty, I prefer to use ScheduledThreadPoolExecutor in Dubbo as well.
4. In a scenario like heartbeat, we can pass the timer to netty's eventLoop to do the Schedule. This is not a coupling. It can be made into a configurable item for selective overloading. It can guarantee the thread of the Timer thread and the request thread. Security (this is a reference to the design of the IdleStateHandler). In Dubbo 2.7, the logic of Timer reput is coupled in an abstract class, which results in a Timer that is not highly reusable.
5. The scenarios for HashedWheelTimer and ScheduledThreadPoolExecutor respectively

Look forward to your discussions.

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3324 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] carryxyh commented on issue #3324: [Discuss] The Timer Design in Dubbo from 2.6 to 2.7

["carryxyh (GitHub)" <gi...@apache.org>]

@kimmking 
Agree. I will notice this point next time.

@lexburner 
Can we close this issue now? Seems like there are some conclusions in mailing list.

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3324 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] kimmking commented on issue #3324: [Discuss] The Timer Design in Dubbo from 2.6 to 2.7

["kimmking (GitHub)" <gi...@apache.org>]

https://github.com/netty/netty/issues/8774

normanmaurer：just because we usually want to be on the same thread as the EventLoop of the Channel. that said there is nothing wrong about the HashedWheelTimer.

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3324 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] beiwei30 closed issue #3324: [Discuss] The Timer Design in Dubbo from 2.6 to 2.7

["beiwei30 (GitHub)" <gi...@apache.org>]

[ issue closed by beiwei30 ]

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3324 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] biyuhao commented on issue #3324: [Discuss] The Timer Design in Dubbo from 2.6 to 2.7

["biyuhao (GitHub)" <gi...@apache.org>]

ref #1371

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3324 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] kimmking commented on issue #3324: [Discuss] The Timer Design in Dubbo from 2.6 to 2.7

["kimmking (GitHub)" <gi...@apache.org>]

Whatever, I guess any important change should be discussed in Issue board or dev mail list before it happened. 

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3324 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org