Summary of several issues of graceful shutdown

[rangtao]

我们在测试及生产环境下总结目前Dubbo优雅停机问题存在几个问题：
[1]Spring可能先于框架关闭，导致业务服务无法正确执行完成；
[2]tomcat+spring场景，中间件停机可能会导致class被回收，导致log日志无法输出，甚至报class not found；
[3]如果存在多个hook，如spring本身有hook，无法保证hook执行顺序；
[4]zk在大数据量同步通知下延迟严重，导致注销后仍有请求（目前版本简单sleep了一下）；
[5]停机时间无法设置，官方关于“停机设置标志位”的描述有误。

综上所述，我们的优化作法是将框架的优雅停机委托给Spring管理，通过监听Spring的close事件触发停机，这样做有两个好处：(1)保证框架停机优于Spring关闭;(2)应用用起来简单，只需要维护Spring生命周期；(3)相对于hook方式，主动调用停机接口更让人放心。

[ralf0131]

Please try 2.6.3, it contains the fix just as you described.

[zonghaishang]

This has been fixed in the new version, now temporarily closed, open anyway if there is a problem

[rangtao]

确实我看到2.6.3版本有相关修改，基本和我描述思路一致，有几个问题问题：
1.个人觉得思路有点偏差，最好的思路是将dubbo停机完全放到spring事件中，最好不要搞框架停机hook，额外增加复杂性，还做不好。比如2.6.3版本DubboBootstrap中registerShutHook，单纯注册dubbo的停机hook没太大的意义，还是需要开发注册spring的停机hook，本身hook执行是无序的，可能导致spring先关闭。在我们的使用场景中，基本都是加载dubbo相关xml启动的。
2. 为什么通过ApplicationContextInitializer注册的方式，需要考虑web.xml加载问题，而且也不针对web项目吧。为什么不在类似于ApplicationConfig类中直接implements ApplicaionListener，监听close事件，对于使用者来说不需要配置，更简单。
3.在destroyAll方法中，之前版本为了注册中心通知延迟，故意sleep了一段时间，2.6.3版本删除了相关代码逻辑，请问是怎么考虑的？

[ralf0131]

1.个人觉得思路有点偏差，最好的思路是将dubbo停机完全放到spring事件中，最好不要搞框架停机hook，额外增加复杂性，还做不好。比如2.6.3版本DubboBootstrap中registerShutHook，单纯注册dubbo的停机hook没太大的意义，还是需要开发注册spring的停机hook，本身hook执行是无序的，可能导致spring先关闭。在我们的使用场景中，基本都是加载dubbo相关xml启动的。

Please consider scenarios where Dubbo may be used without Spring.

2. 为什么通过ApplicationContextInitializer注册的方式，需要考虑web.xml加载问题，而且也不针对web项目吧。为什么不在类似于ApplicationConfig类中直接implements ApplicaionListener，监听close事件，对于使用者来说不需要配置，更简单。

Again, please consider scenarios where Dubbo may be used without Spring. Please note that the core design principles of Dubbo is that the core should not have strong dependency to other framework like Spring.

3.在destroyAll方法中，之前版本为了注册中心通知延迟，故意sleep了一段时间，2.6.3版本删除了相关代码逻辑，请问是怎么考虑的？

Which destroyAll method do you exactly mean? I can't find the logic you mentioned.

[rangtao]

1. 我同意您的说法，Dubbo容器不局限于Spring，可能我描述不太好，Spring容器是一个典型场景。
2. 确实您没有正面回答我的问题，那么我描述下2.6.3版本停机在如下场景下存在问题：如果应用程序通过停机hook触发，DubboShutDown Hook会调用destroyAll方法，因为容器（比如Spring）也通过Hook停机，此时Spirng会提前关闭，可能导致dubbo方法内部无法获取业务Bean。因为多个hook的执行的无序性，destroyAll方法第二次执行会立即返回，所以可能导致容器先摧毁。
3. 关于第三个问题，在2.6.1中ProtocolConfig类中，destroyAll方法包含如下sleep代码。2.6.2中将此部分sleep代码删除了。

2.6.1

public static void destroyAll() {
        if (!destroyed.compareAndSet(false, true)) {
            return;
        }
        AbstractRegistryFactory.destroyAll();
        // Wait for registry notification
        try {
            Thread.sleep(ConfigUtils.getServerShutdownTimeout());
        } catch (InterruptedException e) {
            logger.warn("Interrupted unexpectedly when waiting for registry notification during shutdown process!");
        }
        ExtensionLoader<Protocol> loader = ExtensionLoader.getExtensionLoader(Protocol.class);
        for (String protocolName : loader.getLoadedExtensions()) {
            try {
                Protocol protocol = loader.getLoadedExtension(protocolName);
                if (protocol != null) {
                    protocol.destroy();
                }
            } catch (Throwable t) {
                logger.warn(t.getMessage(), t);
            }
        }
    }

2.6.2

    // TODO: 2017/8/30 to move this method somewhere else
    public static void destroyAll() {
        if (!destroyed.compareAndSet(false, true)) {
            return;
        }
        AbstractRegistryFactory.destroyAll();
        ExtensionLoader<Protocol> loader = ExtensionLoader.getExtensionLoader(Protocol.class);
        for (String protocolName : loader.getLoadedExtensions()) {
            try {
                Protocol protocol = loader.getLoadedExtension(protocolName);
                if (protocol != null) {
                    protocol.destroy();
                }
            } catch (Throwable t) {
                logger.warn(t.getMessage(), t);
            }
        }
    }

[ralf0131]

确实您没有正面回答我的问题，那么我描述下2.6.3版本停机在如下场景下存在问题：如果应用程序通过停机hook触发，DubboShutDown Hook会调用destroyAll方法，因为容器（比如Spring）也通过Hook停机，此时Spirng会提前关闭，可能导致dubbo方法内部无法获取业务Bean。因为多个hook的执行的无序性，destroyAll方法第二次执行会立即返回，所以可能导致容器先摧毁。

No, if you are using Dubbo 2.6.3 with Spring, Dubbo's Shutdown hook should never be registered.
All the shutdown logic should be triggered by Spring's Shutdown hook.
If you found that the code is not working as expected, please file an issue and provide steps to reproduce it.

For question 3, I am not aware of that change. Looking at the history, it seems to be introduced by d290529.

@zonghaishang Do you have any idea why we are change this?

[rangtao]

@ralf0131
if you are using Dubbo 2.6.3 with Spring, Dubbo's Shutdown hook should never be registered.
--这个您说错了，ProtocolConfig中dubbo的shutdown hook依然是保留的，它会触发dubbo停机hook（这个和老版本逻辑一致）。
All the shutdown logic should be triggered by Spring's Shutdown hook.
--应该说2.6.3版本只有注册了DubboApplicationListener才能通过spring close事件触发dubbo停机。另外，spring的shutdown hook需要显示注册的，不是由dubbo框架决定的。

[ralf0131]

--这个您说错了，ProtocolConfig中dubbo的shutdown hook依然是保留的，它会触发dubbo停机hook（这个和老版本逻辑一致）。

@rangtao What framework are you using?

Expected behavior:

• Dubbo + Dubbo's built-in spring container(dubbo-container-spring): DubboApplicationListener will be registered automatically, Dubbo's Shutdown hook will never be registered.
• Dubbo + Spring Framework + Tomcat: DubboApplicationListener will be registered automatically, , Dubbo's Shutdown hook will never be registered.
• Dubbo + Spring-boot: DubboApplicationListener should be registered automatically, Dubbo's Shutdown hook should never be registered. This is not implemented yet, and I am working on enabling it.
• Dubbo without Spring: Dubbo's Shutdown hook will registered automatically.

If you have found something unexpected, please file an issue and provide steps to reproduce.

[zonghaishang]

@rangtao @ralf0131

关于第三个问题，在2.6.1中ProtocolConfig类中，destroyAll方法包含如下sleep代码。2.6.2中将此部分sleep代码删除了。

see HeaderExchangeServer#isRunning:

    private boolean isRunning() {
        Collection<Channel> channels = getChannels();
        for (Channel channel : channels) {
            if (DefaultFuture.hasFuture(channel)) {
             /**
             *  If there are any client connections,
             *  our server should be running.
             */
             if (channel.isConnected()) {
                return true;
            }
        }

If there is a client connection, it will automatically wait， there was a bug before, so will invoke sleep

[beiwei30]

one further comments on #2435 (comment):

1. When server gets shutdown, it unregisters itself from the registry center,
2. then the client will disconnects itself from the shutting-down server when it receives notification from the registry center
3. the shutting down server will continue to shutdown when all established channels are all closed by its peers, or waiting gets timed-out.