[GitHub] [dubbo-go] LaurenceLiZhixin commented on a change in pull request #1592: Config update issue

[GitBox <gi...@apache.org>]

LaurenceLiZhixin commented on a change in pull request #1592:
URL: https://github.com/apache/dubbo-go/pull/1592#discussion_r757464117



##########
File path: config/root_config.go
##########
@@ -358,3 +360,29 @@ func publishMapping(sc exporter.MetadataServiceExporter) error {
 	}
 	return nil
 }
+
+// Process receive changing listener's event, dynamic update config
+func (rc *RootConfig) Process(event *config_center.ConfigChangeEvent) {
+	logger.Info("CenterConfig process event:\n%+v", event)

Review comment:
       I think change to logger.Infof is better

##########
File path: config/root_config.go
##########
@@ -358,3 +360,29 @@ func publishMapping(sc exporter.MetadataServiceExporter) error {
 	}
 	return nil
 }
+
+// Process receive changing listener's event, dynamic update config
+func (rc *RootConfig) Process(event *config_center.ConfigChangeEvent) {
+	logger.Info("CenterConfig process event:\n%+v", event)
+	koan := koanf.New(".")
+	if err := koan.Load(rawbytes.Provider([]byte(event.Value.(string))), yaml.Parser()); err != nil {
+		logger.Errorf("CenterConfig process load failed, got error %#v", err)
+		return
+	}
+	tempRootConfig := &RootConfig{}
+	if err := koan.UnmarshalWithConf(rc.Prefix(),
+		tempRootConfig, koanf.UnmarshalConf{Tag: "yaml"}); err != nil {
+		logger.Errorf("CenterConfig process unmarshalConf failed, got error %#v", err)
+		return
+	}
+	// update registries timeout, rang each registryId(eg: demoZK) ,
+	// if nacos's registry timeout not equal local root config's registry timeout , update.
+	for _, registryId := range tempRootConfig.getRegistryIds() {
+		tempRegistry := tempRootConfig.Registries[registryId]
+		rcRegistry := rc.Registries[registryId]
+		if tempRegistry != nil && rcRegistry != nil && tempRegistry.Timeout != rcRegistry.Timeout {
+			rc.Registries[registryId].Timeout = tempRegistry.Timeout
+			logger.Infof("CenterConfig process update %s timeout, new value:%s", registryId, rc.Registries[registryId].Timeout)
+		}

Review comment:
       1. 这块代码，你是直接在当前rootConfig 的process函数里改的。这样不太好。
   每一层级只关心自己这一层级的配置。rootConfig函数的process方法应当只负责查看是否当前事件需要被更新，如果需要，则将事件里面的更新的信息下传至子模块中更新，而不是在这一层做。
   因此这一块代码判断和修改的逻辑，应当放到registryConfig 的process方法内做，传入的参数应当为新的registryConfig。
   
   2. registryConfig 的 timeout 修改了，不代表真正应用到了registry 的客户端上，因此需要在registryConfig这一层的process方法来进一步处理。
   @sunrui1225




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org