xiaobaicai66695 commented on code in PR #3725:
URL: https://github.com/apache/dubbo-go/pull/3725#discussion_r3999272440


##########
config_center/nacos/listener.go:
##########
@@ -81,9 +81,13 @@ func (s *keyListenerSet) snapshot() 
[]config_center.ConfigurationListener {
 }
 
 func callback(set *keyListenerSet, _, group, dataId, data string) {
+       eventType := remoting.EventTypeUpdate
+       if data == "" {
+               eventType = remoting.EventTypeDel
+       }

Review Comment:
   > 直接回答这个问题,因为结论会影响注释怎么写:
   > 
   > * **「删除会通知监听器」是 Nacos 文档写明的**(也就是你链接的那一页)。
   > * **「用空 data 表示删除」不是文档规定**,是 `nacos-sdk-go` 
配置缓存层的实现细节。也就是说,把链接贴上去并不能支撑这句注释。
   > 
   > 在 v2.2.5 上的实际路径:`clients/config_client/config_client.go:488 
refreshContentAndCheck` → `cacheData.content = 
configQueryResponse.Content`(配置被删时服务端返回的 Content 为空)→ `cacheData.md5 = 
util.Md5(cacheData.content)` → MD5 与 `lastMd5` 不同 → `:85 executeListener()` 把空 
Content 交给监听器。空值来自「MD5 变了」这个判定,不是回调字段本身的语义。
   > 
   > 建议把注释改成陈述 SDK 行为,并且指出这个仓库里已有的先例:`config_center/zookeeper/listener.go` 的 
`DataChange` 里同样有 `if event.Content == "" { changeType = remoting.EventTypeDel 
}`。所以这个改动是让两个配置中心行为一致,而不是新引入一条约定 —— 这句话比文档链接更有说服力。
   > 
   > 另外有一个当前注释没有覆盖的限制,建议一并写清楚:这个回调签名区分不了「删除」和「把配置值设成空串」,而下游是按删除处理的 —— 
`cluster/router/condition/dynamic_router.go:183` 收到 Del 会把 `d.conditionRouter` 
置为 nil(条件路由被清空),`registry/base_configuration_listener.go:82` 会 
`setConfigurators(nil)`。也就是说,一次把配置清空的更新会让这些监听器认为规则被删除。如果这是有意接受的取舍(与 zookeeper 
侧保持一致、且空规则本就等价于无规则),那注释里明说就好;如果想要区分,就需要在回调之外拿到「配置是否存在」的信息,单靠这个回调参数做不到。
   
   感谢指出。Nacos 文档只说明删除会触发监听通知,并未规定空回调内容代表删除;此前的文档链接不足以支撑这一判断,已移除。  
   当前判断基于 nacos-sdk-go v2.2.5 的缓存刷新实现:删除配置后,SDK 会以空内容触发该回调。由于回调本身不携带操作类型,我将其与现有 
ZooKeeper cache listener 的处理保持一致,空内容按 Del 传递给下游。  
   这也意味着“更新为空字符串”和“删除配置”会等价处理;这是当前有意接受的兼容取舍。相关注释已补充说明。



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to