AllConnectConnectionHolder心跳重试线程优化讨论

[shawnliang1124]

Your question

Consumer中，ConnectConnectionHolder负责管理所有provider的长连接。
其中，com.alipay.sofa.rpc.client.AllConnectConnectionHolder#startReconnectThread 函数，会启动一个异步线程，每隔一段时间去检查的长连接是否健康。

如果AllConnectConnectionHolder中的aliveConnections（存活的客户端列表）无法连接，就会将该长连接从aliveConnections中移除，加入到retryConnections（失败待重试的客户端列表）中，假如retryConnections中的连接恢复正常，会把该长连接从retryConnections中移除，加回至 aliveConnections中。
这段逻辑见:com.alipay.sofa.rpc.client.AllConnectConnectionHolder#doReconnect 函数。

private void doReconnect() {
 // 省略代码 

        // 检查可用连接 
        for (Map.Entry<ProviderInfo, ClientTransport> alive : aliveConnections.entrySet()) {
            ClientTransport connection = alive.getValue();
            if (connection != null && !connection.isAvailable()) {
                // 加入至retryConnections
                aliveToRetry(alive.getKey(), connection);
            }
        }

//  检查失败待重试的客户端列表
        for (Map.Entry<ProviderInfo, ClientTransport> entry : getRetryConnections()
                .entrySet()) {

            // 省略代码。。
            transport.connect();
        }

    }

如果retryConnection一直没连上， transport.connect();函数底层会调用至com/alipay/sofa/rpc/transport/bolt/ReuseBoltClientConnectionManager#getConnection方法，该方法会打印warn日志：
get connection failed in url

正常情况，很难去执行到AllConnectConnectionHolder的重试线程，因为provider从注册中心下线，注册中心会向consumer主动推送，consumer会去摘除该长连接。

但我们在实战中，有两个场景碰到了此类问题，会导致以上get connection failed in url的warn日志疯狂打印，造成困扰。

Your scenes

目前，我们服务都是以容器化部署，若宿主机的资源不足，在重启provider服务的时候，原provider的容器ip可能会发生变化。

情况1：

假如consumer没收到注册中心老provider ip下线节点的通知，就会复现get connection failed in url 疯狂打印的场景。
补充说明：因为宿主机ip分配问题，老的provider ip已经永远消失，provider是以一个新的ip注册到注册中心，但老的provider ip仍一直保留在AllConnectConnectionHolder中的retryConnections中，所以会导致warn日志疯狂打印。

情况2：

注册中心摘除provider节点代码设计不合理。
背景：灰度环境和测试环境集群只有一台provider
通过分析源码，定位到consumer摘除provider下线节点的代码在：
com.alipay.sofa.rpc.client.AbstractCluster#updateProviders 函数。

但是该函数做了一个判断：

 public void updateProviders(ProviderGroup providerGroup) {
        // 代码省略。。。


        if (ProviderHelper.isEmpty(providerGroup)) {
            addressHolder.updateProviders(providerGroup);
            if (!ProviderHelper.isEmpty(oldProviderGroup)) {
                if (LOGGER.isWarnEnabled(consumerConfig.getAppName())) {
                    LOGGER.warnWithApp(consumerConfig.getAppName(), "Provider list is emptied, may be all " +
                        "providers has been closed, or this consumer has been add to blacklist");
                    closeTransports();
                }
            }
        } else {
            addressHolder.updateProviders(providerGroup);
            connectionHolder.updateProviders(providerGroup);
        }
         // 代码省略。。。
        }
    }

如果provider中一个节点都没有，只做了 addressHolder.updateProviders(providerGroup);并没有去做 connectionHolder.updateProviders(providerGroup); 而connectionHolder.updateProviders(providerGroup); 底层是会把下线节点从aliveConnections或者retryConnections中删除。 删除的代码在：AllConnectConnectionHolder#remove函数中。

这样会导致，即使consumer收到了provider下线的通知，因为provider在灰度和测试环境只有一台机器，provider重启过程中，就命中了ProviderHelper.isEmpty(providerGroup) 分支，并不会去执行connectionHolder.updateProviders(providerGroup);函数。 该下线的IP一直还在 AllConnectConnectionHolder中的retryConnections属性中。

假如灰度环境或者测试环境的provider ip发生了变化，但老的provider ip仍一直保留在AllConnectConnectionHolder中的retryConnections中，也会导致warn日志疯狂打印。

Your advice

1、建议com.alipay.sofa.rpc.client.AllConnectConnectionHolder#startReconnectThread 机制加上最大重试次数，若失败次数超过x.则将该连接彻底移除。
2、没懂为何provider一台机器没有的情况下，不去执行connectionHolder.updateProviders(providerGroup)函数，希望作者可以解释下如此设计的原因。

Environment

• SOFARPC version:5.6
• jdk 8
  -windows/mac
• Maven version:3.9
• IDE version:2024/01

[Lo1nt]

关于2 。看了下应该是说如果一台都没有，他会做一个关闭动作， closeTransports()，这个动作应该会触发 all/alive/retry的clear

这里确实有个bug，closeTransports()被放在了 warnEnable 里面

也就是说这里update完，你们的empty情况应该会做alive和retry的清理的。

所以没有做 update是因为做了close。然后参考这个bug，有没有可能因为日志级别被设置成 warn disable 导致没有做 close transport？

[shawnliang1124]

是的，我们这个日志是info级别，所以这个函数没进去

if (LOGGER.isWarnEnabled(consumerConfig.getAppName())) {
LOGGER.warnWithApp(consumerConfig.getAppName(), "Provider list is emptied, may be all " +
"providers has been closed, or this consumer has been add to blacklist");
closeTransports();
}

那么，是否会考虑将该BUG解决

[Lo1nt]

是的，我们这个日志是info级别，所以这个函数没进去

if (LOGGER.isWarnEnabled(consumerConfig.getAppName())) { LOGGER.warnWithApp(consumerConfig.getAppName(), "Provider list is emptied, may be all " + "providers has been closed, or this consumer has been add to blacklist"); closeTransports(); }

那么，是否会考虑将该BUG解决

会的，下个版本就修一下。不过Info级别理论上应该是 warn enable 的🤔

[shawnliang1124]

updateProviders

我刚debug了源码，发现在
if (LOGGER.isWarnEnabled(consumerConfig.getAppName())) { LOGGER.warnWithApp(consumerConfig.getAppName(), "Provider list is emptied, may be all " + "providers has been closed, or this consumer has been add to blacklist"); closeTransports(); }

之前还有一个!ProviderHelper.isEmpty(oldProviderGroup)的判断，这个if，在provider只有一台，且重启的情况下，是不会命中上述的代码的，所以，这个还是一个bug。
if (!ProviderHelper.isEmpty(oldProviderGroup)) { if (LOGGER.isWarnEnabled(consumerConfig.getAppName())) { LOGGER.warnWithApp(consumerConfig.getAppName(), "Provider list is emptied, may be all " + "providers has been closed, or this consumer has been add to blacklist"); closeTransports(); } }

[Lo1nt]

updateProviders

我刚debug了源码，发现在 if (LOGGER.isWarnEnabled(consumerConfig.getAppName())) { LOGGER.warnWithApp(consumerConfig.getAppName(), "Provider list is emptied, may be all " + "providers has been closed, or this consumer has been add to blacklist"); closeTransports(); }

之前还有一个!ProviderHelper.isEmpty(oldProviderGroup)的判断，这个if，在provider只有一台，且重启的情况下，是不会命中上述的代码的，所以，这个还是一个bug。 if (!ProviderHelper.isEmpty(oldProviderGroup)) { if (LOGGER.isWarnEnabled(consumerConfig.getAppName())) { LOGGER.warnWithApp(consumerConfig.getAppName(), "Provider list is emptied, may be all " + "providers has been closed, or this consumer has been add to blacklist"); closeTransports(); } }

这个场景下， oldProviderGroup 应该是不为空的？有一个 provider的数据？

他可能会进来多次，第一次应该是provider重启导致 client需要更新为空，本次更新应该是 oldProviderGroup.len == 1 -> providerGroupToUpdate.len == 0, 并在本次做了 closeTransports？ 按理说这样才是符合预期的

[shawnliang1124]

oldProviderGroup

我debug之后，流程如下：
1、在命中 if (ProviderHelper.isEmpty(providerGroup)) 分支后
2、会执行 addressHolder.updateProviders(providerGroup);函数
3、这个函数里面，执行的是SingleGroupAddressHolder#updateProviders函数
4、这时候已经会将providerGroup，里面的providerInfos设置成空数组（ providerGroup = protected ProviderGroup registryGroup; //注册中心来的地址列表 ）
5、而这个oldProviderGroup对象引用正是第四步的registryGroup
6、所以就!ProviderHelper.isEmpty(oldProviderGroup)就无法命中

[Lo1nt]

嗯，看了下这里 oldProviderGroup是个浅拷贝，只是个对SingleGroupAddressHolder.providerGroup的引用，所以SingleGroupAddressHolder.providerGroup 更新时 oldProviderGroup 指向的内容也更新了。

这里应该是 oldProviderGroup 失去了 作为 old 的意义。从设计初衷来看这里可能要考虑做个深拷贝。

[shawnliang1124]

嗯，看了下这里 oldProviderGroup是个浅拷贝，只是个对SingleGroupAddressHolder.providerGroup的引用，所以SingleGroupAddressHolder.providerGroup 更新时 oldProviderGroup 指向的内容也更新了。

这里应该是 oldProviderGroup 失去了 作为 old 的意义。从设计初衷来看这里可能要考虑做个深拷贝。

另外我提到的第一点，【建议com.alipay.sofa.rpc.client.AllConnectConnectionHolder#startReconnectThread 机制加上最大重试次数，若失败次数超过x.则将该连接彻底移除。】，是否会考虑呢

[Lo1nt]

另外我提到的第一点，【建议com.alipay.sofa.rpc.client.AllConnectConnectionHolder#startReconnectThread 机制加上最大重试次数，若失败次数超过x.则将该连接彻底移除。】，是否会考虑呢

这个还需要详细评估一下。provideGroup数据是来自注册中心的，一般情况下我们应该是去信任注册中心传递来的元数据。没有收到注册中心数据的情况下移除连接，可能会在其他一些情况下引发问题。

[shawnliang1124]

另外我提到的第一点，【建议com.alipay.sofa.rpc.client.AllConnectConnectionHolder#startReconnectThread 机制加上最大重试次数，若失败次数超过x.则将该连接彻底移除。】，是否会考虑呢

这个还需要详细评估一下。provideGroup数据是来自注册中心的，一般情况下我们应该是去信任注册中心传递来的元数据。没有收到注册中心数据的情况下移除连接，可能会在其他一些情况下引发问题。

明白，不过其实只要解决了oldProviderGroup 被覆盖的问题，就不存在这个最大重试次数的必要性了。