apache / dubbo

The java implementation of Apache Dubbo. An RPC and microservice framework.
https://dubbo.apache.org/
Apache License 2.0
40.51k stars 26.43k forks source link

关于将AbstractRegistry#doSaveProperties()方法中properties.setProperty()做异步处理的建议 #9660

Open fsx379 opened 2 years ago

fsx379 commented 2 years ago

AbstractRegistry#doSaveProperties()用于在notify通知时,将推送内容存在内存properties 中,然后再持久化磁盘。 通过测试,发现如果推送量比较大,doSaveProperties()会比较耗时,影响整个notify处理流程。 具体原因如下:

private void saveProperties(URL url) {
  .....
    try {
        StringBuilder buf = new StringBuilder();
        Map<String, List<URL>> categoryNotified = notified.get(url);
        if (categoryNotified != null) {
           ......
        }
        //[1] 保存内存
        properties.setProperty(url.getServiceKey(), buf.toString());
        long version = lastCacheChanged.incrementAndGet();
        //[2]默认异步持久化
        if (syncSaveFile) {
            doSaveProperties(version);
        } else {
            registryCacheExecutor.execute(new SaveProperties(version));
        }
    } catch (Throwable t) {
        logger.warn(t.getMessage(), t);
    }
}

1.在主流程中 调用 properties.setProperty() 更新 properties,此方法中带有 synchronized 修饰符; 2.默认持久化逻辑 doSaveProperties() 虽然是异步的,但是底层 properties.store0() 方法中,有synchronized(this){...} 代码块; 上述两处的 properties 是同一对象,故会相互影响。 3.如果推送量比较大, properties.setProperty() 被卡住的几率就比较大,影响整体处理速度。(特别是dubbo-admin 底层也使用了dubbo 的逻辑,受影响更大)

此时是否可以将properties.setProperty() 处理,也放入 doSaveProperties()异步逻辑中,减少对主流程notify处理的影响?

BurningCN commented 2 years ago

的确存在这个问题。

我的另一种想法是在异步存储到文件的时候,加锁(比如显示声明一个Object obj对象,然后sync(obj)),然后将properties的数据读到另一个容器中,再将此容器的数据写入到文件中。

这个思路来源于MetaCacheManager#CacheRefreshTask#run,这里可以看到cache就相当于前面提到的properties,是一个内存随时变更的容器。

fsx379 commented 2 years ago

的确存在这个问题。

我的另一种想法是在异步存储到文件的时候,加锁(比如显示声明一个Object obj对象,然后sync(obj)),然后将properties的数据读到另一个容器中,再将此容器的数据写入到文件中。

这个思路来源于MetaCacheManager#CacheRefreshTask#run,这里可以看到cache就相当于前面提到的properties,是一个内存随时变更的容器。

现在代码中的锁是 properties 自身的,改起来比较费劲,是要重新实现一个 properties 类? 顺便求MetaCacheManager#CacheRefreshTask#run 出处,学习下.....

BurningCN commented 2 years ago

@fsx379

不需要实现一个properties类,比如说可以用一个新的临时的properties对象,将AbstractRegistry的原有properties的数据读出来写到这个临时的properties中。(就像下面代码将原有cache的数据写到新的properties一样)

MetaCacheManager#CacheRefreshTask#run 3.0分支的代码 如下 https://github.com/apache/dubbo/blob/adea101f93555101a5a0eaec6951c055bfe1ede8/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/metadata/store/MetaCacheManager.java#L153-L167

fsx379 commented 2 years ago

@BurningCN 了解了,是个不错的思路。

fsx379 commented 2 years ago

@fsx379

不需要实现一个properties类,比如说可以用一个新的临时的properties对象,将AbstractRegistry的原有properties的数据读出来写到这个临时的properties中。(就像下面代码将原有cache的数据写到新的properties一样)

MetaCacheManager#CacheRefreshTask#run 3.0分支的代码 如下

https://github.com/apache/dubbo/blob/adea101f93555101a5a0eaec6951c055bfe1ede8/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/metadata/store/MetaCacheManager.java#L153-L167

同步下我这边根据测试,对这个问题的最新看法: 1.通过加日志观察,如果把这个问题优化了,notify总体处理过程会快很多,但是却引发出一个新问题,即 “notify处理了更多的通知” 2.之前工行那边提出了一个“zk客户端延时订阅方案” 的思路,解决同一服务如果部署多个机器,机器在分批重启过程中,造成了大量重复订阅(/dubbo/**serice/provides 下任何节点变化都会推送)。利用延时订阅的方法,减少了频繁变化下的重复请求,降低了带宽和CPU负载。 3.所以目前的properties中的锁,从效果看也是一种 “延时处理方案”,解决了反而引出新问题。。

以上就是我最近的结论,不知道你怎么看? @BurningCN

BurningCN commented 2 years ago

第二点提到的利用延时订阅的方法,减少了频繁变化下的重复请求 这个优化逻辑已经dubbo代码里实现了(ZookeeperRegistry#RegistryChildListenerImpl)。

第一点提到的 “notify处理了更多的通知” 指的是notify方法被调用频次高吗?这个主要还是因为第二点吧,在优化了异步写文件的线程和原执行notify方法的线程不共用properties的锁 ,从而加速了notify方法的执行流程,但应该也不会因为这个带来notify被调用次数高很多吧。

fsx379 commented 2 years ago

第一点提到的 “notify处理了更多的通知” 指的是notify方法被调用频次高吗?这个主要还是因为第二点吧,在优化了异步写文件的线程和原执行notify方法的线程不共用properties的锁 ,从而加速了notify方法的执行流程,但应该也不会因为这个带来notify被调用次数高很多吧。

对,是由于优化后加速了notify方法的执行流程导致的(老版本 2.6* 中)。特别是应用节点多,且一次批量发布多个机器,会频繁操作zk(删除+重新注册节点)。zk客户端 notify 处理慢,同一个节点watch频率低,会忽略很多中间的变化;处理快,同一个节点watch频率高,会更敏感的感知中间的变化。