nacos-client的HostReactor类的 processServiceJson方法中的推空防护机制在dubbo2.7.5应用环境下无法生效 #5163

zrlw · 2021-03-22T01:53:05Z

Describe the bug
dubbo服务上线、下线更新服务时，nacos-client的HostReactor处理服务列表更新的方法processServiceJson持续收到空hosts列表，导致客户端将服务列表清空，调用dubbo服务（版本2.7.5) 出现no provider，实际上dubbo服务一直运行正常。

测试环境：
nacos注册中心（集群多节点）：1.4.1和1.2.1两个版本均测过
nacos客户端版本：1.4.1和1.2.1两个版本都会出现，使用1.4.1版本时问题更严重
1.4.1版本的com.alibaba.nacos.client.naming.core.HostReactor类的processServiceJson方法相比1.2.1，默认不再判断收到的serviceInfo内容是否合法（判断条件增加了一个配置选项pushEmptyProtection，默认为false）。

Expected behavior

nacos-client处理服务列表推送Json串时，processServiceJson方法忽略CollectionUtils.isEmpty(serviceInfo.getHosts())的情况（即使服务都没有了，调用失败抛channel closed异常也好过把正常服务全清掉）。
排查nacos-server端的服务列表推送代码，不要莫名其妙地持续推送空服务列表给客户端，偶尔抽风可以忍受，持续这么搞就离谱了。

Acutally behavior
收到的服务推送列表为empty时（serviceInfo.getHosts().size()==0）时，服务列表被动清空，导致调用dubbo服务出现No provider。

KomachiSion · 2021-03-22T01:58:56Z

serviceInfo.validate中 1.4.1版本添加了空列表判断，可以读一下。

1.2.1确实存在问题，没有推空保护的功能。

服务端推空一般有2种情况，1. 服务列表确实为空；2. 服务不存在

这两种情况都应该排查为什么服务没了。

如果需要推空保护，就需要升级到至少1.4.1版本客户端，打开推空保护。

zrlw · 2021-03-22T02:41:15Z

PS： 1.2.1的代码看起来是默认开启推送保护

if (serviceInfo.getHosts() == null || !serviceInfo.validate()) {
  // empty or error push, just ignore
  return oldService;
 }

可但是，它没有考虑serviceInfo.getHosts().size()==0这种empty，这也是我们目前缓释问题的方法，采用这种缓释方法已经平稳运行了几天了。
如果这种方法有效，我们打算提一个pull，看看在最新的2.0版本加上这个补丁效果如何。

shalk · 2021-03-22T03:22:22Z

erviceInfo.getHosts().size()==0这

当 serviceInfo.getHosts().size()==0， !serviceInfo.validate() 为true，所以这个情况是考虑了的

只是和1.2.1相比，namingPushEmptyProtection 默认不是true。需要主动配置一下 namingPushEmptyProtection为true

zrlw · 2021-03-22T05:31:20Z

namingPushEmptyProtection这个值应该考虑向前兼容，除非有非常紧要的理由，升级了更容易出毛病也不合适。

shalk · 2021-03-22T05:43:31Z

erviceInfo.getHosts().size()==0这

当 serviceInfo.getHosts().size()==0， !serviceInfo.validate() 为true，所以这个情况是考虑了的
只是和1.2.1相比，namingPushEmptyProtection 默认不是true。需要主动配置一下 namingPushEmptyProtection为true
       if (hosts == null) {
            return false;
        }
serviceInfo.validate() 这样判断是不行的，hosts是个list，实际上这个对象不是null，而是它的size()等于0

看这一部分

   List<Instance> validHosts = new ArrayList<Instance>();
        for (Instance host : hosts) {
            if (!host.isHealthy()) {
                continue;
            }
            
            for (int i = 0; i < host.getWeight(); i++) {
                validHosts.add(host);
            }
        }
        //No valid hosts, return false.
        return !validHosts.isEmpty();

向前兼容

我感觉一般是要向前兼容

zrlw · 2021-03-22T05:47:23Z

erviceInfo.getHosts().size()==0这

当 serviceInfo.getHosts().size()==0， !serviceInfo.validate() 为true，所以这个情况是考虑了的
只是和1.2.1相比，namingPushEmptyProtection 默认不是true。需要主动配置一下 namingPushEmptyProtection为true
       if (hosts == null) {
            return false;
        }
serviceInfo.validate() 这样判断是不行的，hosts是个list，实际上这个对象不是null，而是它的size()等于0
看这一部分
   List<Instance> validHosts = new ArrayList<Instance>();
        for (Instance host : hosts) {
            if (!host.isHealthy()) {
                continue;
            }
            
            for (int i = 0; i < host.getWeight(); i++) {
                validHosts.add(host);
            }
        }
        //No valid hosts, return false.
        return !validHosts.isEmpty();
向前兼容

我感觉一般是要向前兼容

绕一大圈返回一个false，有啥必要呢？

shalk · 2021-03-22T05:51:39Z

erviceInfo.getHosts().size()==0这

当 serviceInfo.getHosts().size()==0， !serviceInfo.validate() 为true，所以这个情况是考虑了的
只是和1.2.1相比，namingPushEmptyProtection 默认不是true。需要主动配置一下 namingPushEmptyProtection为true
       if (hosts == null) {
            return false;
        }
serviceInfo.validate() 这样判断是不行的，hosts是个list，实际上这个对象不是null，而是它的size()等于0
看这一部分
   List<Instance> validHosts = new ArrayList<Instance>();
        for (Instance host : hosts) {
            if (!host.isHealthy()) {
                continue;
            }
            
            for (int i = 0; i < host.getWeight(); i++) {
                validHosts.add(host);
            }
        }
        //No valid hosts, return false.
        return !validHosts.isEmpty();
向前兼容

我感觉一般是要向前兼容
绕一大圈返回一个false，有啥必要呢？

我们讨论的是 serviceInfo.getHosts().size()==0 的情况是否考虑进去了。

因此可以不用修改HostReactor，修改一下namingPushEmptyProtection 就可以。

我们只是讨论问题，你觉得实现不优雅，可以另外再讨论。

zrlw · 2021-03-22T06:09:47Z

serviceInfo的hosts成员是构造serviceInfo时刻就new出来的，不可能为null，所以在validate方法里增加一行==null判断本身就很奇怪，这个写法和代码是否优雅无关，更像是手误。
之前版本serviceInfo是否为validate和其hosts成员内容是否为空之间没有关联，所以在validate()方法里判断hosts列表为空是否合适，毕竟HostReactor里的代码注释还是empty or error push， just ignore，如果放在validate()方法，那就是说空也是error了。
还有一个不清楚的地方是，validate()方法首先判断isAllIPs，这个为true就直接返回true，此时hosts无论是否为null都不管了，有没有风险？

shalk · 2021-03-22T06:36:03Z

你这么说，我又看了一下版本和commit

判断null 是有意的
之前版本，1.2.1 validate 的逻辑是有点问题的，只返回true，也没有推空保护，所以不存在向前兼容
1.4.1 中，是有意通过 validate是来判断，这个方法也没有其他地方引用到。 [ISSUE-#3880] NamingService Client support pushEmptyProtection. #4665

这个 isAllIPs 也不清楚有什么用，感觉可以删了

zrlw · 2021-03-22T07:03:31Z

假设服务列表推送json串反序列化出来hosts为null，1.2.1的代码也是有保护，如下：
if (serviceInfo.getHosts() == null || !serviceInfo.validate())
只是大多数情况下我们发现这个hosts反序列化出来不是null，而是size==0，
所以升到nacos-client1.4.1之后，问题更严重了，因为hosts为null时也不保护了。如果要升1.4.1，我们N多个jar都需要重新部署，设置那个新增的参数，短期内搞不来。

zrlw · 2021-03-22T07:17:18Z

从名称上看，pushEmptyProtection这个参数只和服务列表Hosts是否为空相关；
无论pushEmptyProtection是什么，感觉validate操作都应该做一下吧，万一传回来的列表内容是错的呢？

zrlw · 2021-03-22T07:32:55Z

        if (pushEmptyProtection && CollectionUtils.isEmpty(serviceInfo.getHosts())) {
            // empty push, just ignore
            return oldService;
        }
        if (!serviceInfo.validate()) {
            // error push, just ignore
            return oldService;
        }

只是建议，仅供参考。毕竟都只是缓解问题，服务下线、上线为什么会触发nacos-client收到空服务列表的根源还没有找到。
我们的测试场景：

下线、上线所有的B服务（B服务为dubbo2.7.5server，启动耗时< 15秒）；
等待50秒（确保B服务启动完，nacos注册中心完成注册及同步）；
下线、上线1台A服务，A服务为dubbo2.7.5 client，启动过程要调用m次B服务发布的provider；
等待20秒（A服务正常启动耗时<15秒），grep查看A服务的应用日志有没有No provider。
循环1~4持续2小时，打补丁之前几分钟就会遇到No provider，打补丁后迄今为止未出现。

KomachiSion · 2021-03-22T08:32:14Z

其实将validate和推空保护分开也是合理的，但是应该validate在前，我觉得。

另外就是serviceInfo.getHost == null 这点，属于一个保护性的条件，也许是很老的版本里可能返回null的列表而非空列表，为了兼容而存在的逻辑，只是一直没有删除；也可能是就是顺手写一下。我觉得留着比较好，只是可能可以移动到serviceInfo.validate()里

KomachiSion · 2021-03-22T08:34:43Z

最后我看完所有讨论后，感觉您的问题其实已经在1.4.1版本解决了。所以我认为该issue可以关闭。

大部分讨论其实是在讨论细节应该在哪里处理更优雅和可读性的问题，可以单独开issue讨论。

zrlw · 2021-03-22T08:40:48Z

namingPushEmptyProtection这个值默认应该是true吧，1.4.1版本默认是false，用dubbo2.7.5的应用根本就无法配置这个参数，使用nacos-client1.2.1至少在hosts为null时还有推空保护，升到1.4.1之后，情况更糟了。

zrlw · 2021-03-22T09:21:13Z

实际上如果hosts为null，1.4.1的nacos-client会直接抛异常，这个是另外一个问题了。

zrlw · 2021-03-22T09:57:47Z

namingPushEmptyProtection这个东西要配置也应该是在dubbo.registry下配置吧，我们用的是dubbo2.7.5，我们想配置它也配不了，dubbo 2.7.5的NacosNamingServiceUtils的buildNacosProperties还不认识这个参数。

KomachiSion · 2021-03-22T11:53:04Z

版本1.4.1 移动到validate内部了啊，只是需要打开推送保护。

默认值false是为了和旧版本保持一直吧，因为1.4.1之前validate由于实现问题一直没有推空保护能力。

zrlw · 2021-03-22T12:10:35Z

debug显示是从dubbo注解@Reference对应的url构建NacosNamingService：

@Reference（dubbo注解）-》RegistryProtocol.ref(Class<T>, URL)  -》NacosNamingServiceUtils.createNamingService(URL)

上面这些都是dubbo2.7.5的类，最后这个createNamingService负责解析URL创建properties，但是它根本就不读取namingPushEmptyProtection配置参数。
此外，dubbo2.7.5环境下，URL里也没有namingPushEmptyProtection这个参数（debug显示dubbo的ReferenceConfig这个类读dubbo.registry配置时没有读namingPushEmptyProtection，所以构造的URL也不会包含这个参数，在application.yml里配置dubbo.registry.namingPushEmptyProtection是没有任何作用的）。

zrlw · 2021-03-22T12:35:28Z

1.2.1有部分推空保护能力，因为判null在validate()之外！可以阻止hosts=null的推空，下面是1.2.1的半拉子推空保护代码：

if (serviceInfo.getHosts() == null || !serviceInfo.validate()) {
  return oldService;
}

下面是1.4.1的推空保护代码，看似完美但在dubbo2.7.5应用环境下没有半点用处，因为pushEmptyProtection恒为false，根本就设置不了。

if (pushEmptyProtection && !serviceInfo.validate()) {
  return oldService;
}

KomachiSion · 2021-03-23T01:34:50Z

1.2.1有部分推空保护能力，因为判null在validate()之外！可以阻止hosts=null的推空，下面是1.2.1的半拉子推空保护代码：
if (serviceInfo.getHosts() == null || !serviceInfo.validate()) {
  return oldService;
}
下面是1.4.1的推空保护代码，看似完美但在当前所有dubbo版本应用环境下没有半点用处，因为pushEmptyProtection恒为false，根本就设置不了。
if (pushEmptyProtection && !serviceInfo.validate()) {
  return oldService;
}

这个问题有两种方式可以处理，1是在nacos client测增加读取JVM或环境变量的方式设置该值。 2是提交给dubbo或SCA等框架，添加类似的入参或设置方式。

我觉得两边都增加都可以。

zrlw · 2021-03-23T02:22:17Z

dubbo provider注册到nacos注册中心的处理过程有两步，第1步是构建注册中心的url，这个环节是读取应用配置的注册中心参数，将这些参数作为url的parameter缓存到一个map里；第2步是从缓存的map取出url，根据url的parameter构建nacosNamingService的properties。
如果nacos-client添加了新property，需要同时修改dubbo上述两个处理过程，过于亲密不利于各自扩展。
提一个建议：
在nacos的namingservice各个构造方法上添加一个扩展入参（可省略），格式为json串；在dubbo服务注册参数配置项（如下所示）配置nacos注册中心的扩展参数，dubbo将这个配置项的内容直接传给nacosNamingService的构造方法，由nacos自己解析。

dubbo:
    registry:
        parameters: {"namingPushEmptyProtection": true, "namingRequestDomainMaxRetryCount": 3}

无论最终采取哪种方式，都需要nacos和dubbo两个团队支持。

KomachiSion · 2021-03-23T02:57:12Z

dubbo provider注册到nacos注册中心的处理过程有两步，第1步是构建注册中心的url，这个环节是读取应用配置的注册中心参数，将这些参数作为url的parameter缓存到一个map里；第2步是从缓存的map取出url，根据url的parameter构建nacosNamingService的properties。
如果nacos-client添加了新property，需要同时修改dubbo上述两个处理过程，过于亲密不利于各自扩展。
提一个建议：
在nacos的namingservice各个构造方法上添加一个扩展入参（可省略），格式为json串；dubbo增加一个注册中心参数配置项（如下所示），dubbo将这个配置项的内容直接传给nacosNamingService的构造方法，由nacos自己解析。
dubbo:
    registry:
        parameters: {"namingPushEmptyProtection": true, "namingRequestDomainMaxRetryCount": 3}
无论最终采取哪种方式，都需要nacos和dubbo两个团队支持。

可以参考SCA最新的处理方式，除了少量常用和必填的key以外，就只有一个properties，kv都是用户自己配置的，这样可以支持扩展，需要dubbo团队支持。

client这边其实也需要抽象一个properties的getter 所有变量都应该支持从入参-JVM-环境变量这样一个顺序的读取。

zrlw · 2021-03-23T03:38:20Z

又看了一遍dubbo 2.7.5的代码，已经有dubbo.registry.parameters这个配置参数了，类型就是hashmap，应用配置的格式就是json串，nacos注册中心的所有参数都可以在这个配置项里配置，构建注册中心url时dubbo直接把这个hashmap里的各个item作为了url的parameter。
只是dubbo 2.7.5构建nacos的namingservice的代码只是从url里取了指定的parameter出来，而namingPushEmptyProtection不在其列。
nacos团队的核心开发人员可否先直接和dubbo团队沟通一下这个事情，我们可以去dubbo提issue。

shalk · 2021-03-23T05:50:27Z

use nacos-client 1.4.1 with dubbo 2.7.8+

dubbo.registry.parameters.namingPushEmptyProtection=true

KomachiSion · 2021-03-23T06:57:53Z

用exclude

zrlw · 2021-03-23T07:03:28Z

明白你们的意思了，修改应用的pom文件，使用高版本dubbo在构造nacosNamingService时使用nacos-client 1.4.1定义的属性类吧，我们的大多数应用目前还不兼容dubbo 2.7.7+，所以短期内还升不了dubbo版本。

zrlw · 2021-03-23T07:28:57Z

建议直接删除namingPushEmptyProtection这个参数，看不到这个参数有什么实际用处，在彻底解决nacos-client持续收到空服务列表问题（hosts为null或者hosts.size()==0）之前，这个参数只会加重问题。

zrlw · 2021-03-23T07:33:06Z

假设服务真的全没有了，客户端还保存服务列表的副作用无非就是调用失败，出现message can not send, because channel is closed异常，这个结果可以接受吧，反正横竖都是调用失败而已。
但是服务都还在，客户端自己把服务列表清空了，没有调用直接抛出No provider异常，这样的情况是不能容忍的。

zrlw · 2021-03-23T13:21:47Z

刚才查看了一台测试环境的nacos注册中心（1.2.1版本，日志开启了DEBUG输出）的naming-server日志，发现有很多基础服务间隔几秒就会出现no instance to serve for service: DEFAULT_GROUP@@x.x.xService。
我们登录这台nacos注册中心的管理页面，查看这些服务的实例数量是完整的，刷新了几次页面，数量也都没有变化。
我们做了一下5分钟交易压测，结果正常，不过我们现在用的是修订后的nacos-client 1.2.1版本（把getHosts()==null判空改成了==null || size ==0)。

KomachiSion · 2021-03-24T01:45:07Z

假设服务真的全没有了，客户端还保存服务列表的副作用无非就是调用失败，出现message can not send, because channel is closed异常，这个结果可以接受吧，反正横竖都是调用失败而已。
但是服务都还在，客户端自己把服务列表清空了，没有调用直接抛出No provider异常，这样的情况是不能容忍的。

是这样，所以才有推空保护功能。但是推空保护和保护阈值一样，是一个修改了实际结果的操作，那么需要用户明知开启了这个操作。

还有一个问题是，客户端不应该在服务端正常及服务列表正常的情况下获取到非法或空列表，如果出现应该排查哪里出了问题，并修复这个问题。

zrlw · 2021-03-24T05:22:15Z

nacos注册中心naming-server.log出现no instance我们已经初步定位到问题代码了，nacos-server的InstanceController收到list请求的serviceName名称里面没有providers:或consumers:前缀，但是nacos注册中心serviceMap里面的entrykey要么是有consumers:前缀，要么是有providers:前缀，所以找不到。
目前看有时注册中心收到的list请求参数带有前缀，有时没有，下一步我们要查一下客户端的代码，看看为啥要这么搞。

zrlw · 2021-03-24T09:41:05Z

nacos注册中心naming-server.log出现no instance我们已经初步定位到问题代码了，nacos-server的InstanceController收到list请求的serviceName名称里面没有providers:或consumers:前缀，但是nacos注册中心serviceMap里面的entrykey要么是有consumers:前缀，要么是有providers:前缀，所以找不到。
目前看有时注册中心收到的list请求参数带有前缀，有时没有，下一步我们要查一下客户端的代码，看看为啥要这么搞。

原因是nacos-sync忘了删掉一些配错了的同步任务，这些任务都漏了provider前缀，清掉这些任务之后nacos注册中心的naming-server日志暂时就没有再出现no instance了，还需要进一步排查。因为我们当时出现问题时，naming-sever日志开启了debug输出，里面并没有和客户端丢失服务相关的no instance日志内容。

zrlw · 2021-03-24T09:45:52Z

你们也可以测试一下，写一个dubbo服务B，部署3~6台实例，一个dubbo客户端A，按下列步骤测试可以稳定复现问题：

ssh远程kill -9杀掉所有的B实例、远程启动所有的B实例（B服务为dubbo2.7.5server，启动耗时< N秒）；
等待N+10秒（确保B服务实例启动完，nacos注册中心完成注册及同步）；
ssh远程kill -9杀掉1台A实例、远程启动1台A实例，A为dubbo2.7.5 client，启动过程要调用3~6次B服务发布的provider；
等待M+10秒（A实例正常启动耗时<M秒），grep查看A实例的应用日志有没有No provider。
循环1~4用不了几次循环就能看到A启动过程的No provider异常。

KomachiSion · 2021-03-26T01:46:19Z

如果是服务名错误，那就不是nacos的问题了，是注册和订阅的使用问题。

如果是sync配错了，那还是使用问题，如果是配对了，同步的时候丢了信息，那可能是sync的bug，需要再看看。

关于最后一个问题，需要分清是一直No provider还是出现一次然后再启动时又没有，如果是，那应该只是获取地址列表可能有一定的延迟，启动时最好有一定的重试保护。

这个issue已经偏离了主题，既然找到了主要问题是服务名配错导致服务找不到，然后推空保护问题其实1.4.1已经支持，我认为issue可以关闭了。

zrlw · 2021-03-26T02:18:38Z

最后一个问题是一直No provider，重启nacos注册服务中心才能恢复。（没有开启nacos sync）

KomachiSion added the status/need feedback label Mar 22, 2021

zrlw mentioned this issue Mar 22, 2021

缓解nacos-client不停地清空服务列表的问题，客户端获取本地IP过滤loopback、anylocal和linklocal地址 #5164

Closed

zrlw changed the title ~~nacos-client的HostReactor类的 processServiceJson方法判断serviceInfo的hosts列表是否为空的逻辑有误~~ nacos-client的HostReactor类的 processServiceJson方法中的推空防护机制在当前所有dubbo版本应用环境下均无法生效 Mar 22, 2021

zrlw mentioned this issue Mar 23, 2021

可否将dubbo.registry.parameters配置的所有参数全部传给NacosNamingService的构造方法 apache/dubbo#7431

Closed

2 tasks

zrlw changed the title ~~nacos-client的HostReactor类的 processServiceJson方法中的推空防护机制在当前所有dubbo版本应用环境下均无法生效~~ nacos-client的HostReactor类的 processServiceJson方法中的推空防护机制在dubbo2.7.5应用环境下无法生效 Mar 23, 2021

KomachiSion closed this as completed Mar 26, 2021

KomachiSion added kind/question Category issues related to questions or problems and removed status/need feedback labels Mar 26, 2021

nacos-client的HostReactor类的 processServiceJson方法中的推空防护机制在dubbo2.7.5应用环境下无法生效 #5163

nacos-client的HostReactor类的 processServiceJson方法中的推空防护机制在dubbo2.7.5应用环境下无法生效 #5163

Comments

zrlw commented Mar 22, 2021 • edited

KomachiSion commented Mar 22, 2021

zrlw commented Mar 22, 2021

shalk commented Mar 22, 2021 • edited

zrlw commented Mar 22, 2021

shalk commented Mar 22, 2021

zrlw commented Mar 22, 2021

shalk commented Mar 22, 2021

zrlw commented Mar 22, 2021 • edited

shalk commented Mar 22, 2021 • edited

zrlw commented Mar 22, 2021

zrlw commented Mar 22, 2021

zrlw commented Mar 22, 2021

KomachiSion commented Mar 22, 2021

KomachiSion commented Mar 22, 2021

zrlw commented Mar 22, 2021 • edited

zrlw commented Mar 22, 2021

zrlw commented Mar 22, 2021 • edited

KomachiSion commented Mar 22, 2021

zrlw commented Mar 22, 2021 • edited

zrlw commented Mar 22, 2021 • edited

KomachiSion commented Mar 23, 2021 • edited

zrlw commented Mar 23, 2021 • edited

KomachiSion commented Mar 23, 2021

zrlw commented Mar 23, 2021 • edited

shalk commented Mar 23, 2021

KomachiSion commented Mar 23, 2021

zrlw commented Mar 23, 2021 • edited

zrlw commented Mar 23, 2021

zrlw commented Mar 23, 2021 • edited

zrlw commented Mar 23, 2021

KomachiSion commented Mar 24, 2021

zrlw commented Mar 24, 2021

zrlw commented Mar 24, 2021

zrlw commented Mar 24, 2021 • edited

KomachiSion commented Mar 26, 2021

zrlw commented Mar 26, 2021 • edited

zrlw commented Mar 22, 2021 •

edited

shalk commented Mar 22, 2021 •

edited

zrlw commented Mar 22, 2021 •

edited

shalk commented Mar 22, 2021 •

edited

zrlw commented Mar 22, 2021 •

edited

zrlw commented Mar 22, 2021 •

edited

zrlw commented Mar 22, 2021 •

edited

zrlw commented Mar 22, 2021 •

edited

KomachiSion commented Mar 23, 2021 •

edited

zrlw commented Mar 23, 2021 •

edited

zrlw commented Mar 23, 2021 •

edited

zrlw commented Mar 23, 2021 •

edited

zrlw commented Mar 23, 2021 •

edited

zrlw commented Mar 24, 2021 •

edited

zrlw commented Mar 26, 2021 •

edited