Alanxtl commented on code in PR #3252:
URL: https://github.com/apache/dubbo-go/pull/3252#discussion_r2938002684
##########
client/options.go:
##########
@@ -467,6 +468,14 @@ func WithParam(k, v string) ReferenceOption {
}
}
+func WithRouter(routers ...*global.RouterConfig) ReferenceOption {
+ return func(opts *ReferenceOptions) {
+ if len(routers) > 0 {
+ opts.Routers = append(opts.Routers, routers...)
+ }
+ }
+}
Review Comment:
然后静态注入和动态配置中心的关系没有讲清楚。当前主干里的 condition router / application router 依赖 config
center listener 和 Process() 机制处理动态规则；这个 PR
又额外加了静态注入入口。两者同时存在时，谁覆盖谁、启动时先加载谁、动态更新后是否替换静态规则
需要明确一下，后续写文档的时候需要写清楚
##########
client/options.go:
##########
@@ -546,6 +555,17 @@ func setRegistries(regs map[string]*global.RegistryConfig)
ReferenceOption {
}
}
+func setRouters(routers []*global.RouterConfig) ReferenceOption {
+ return func(opts *ReferenceOptions) {
+ if len(routers) > 0 {
+ if opts.Routers == nil {
+ opts.Routers = make([]*global.RouterConfig, 0)
+ }
+ opts.Routers = append(opts.Routers, routers...)
+ }
+ }
+}
Review Comment:
现在有三个地方会修改reuters：
setRouters 是 append
WithClientRouter 是 append
SetClientRouters 是直接覆盖
确认一下幂等性以及重复 key / 重复 rule 时怎么处理
##########
client/options.go:
##########
@@ -1005,6 +1037,15 @@ func SetClientProtocols(protocols
map[string]*global.ProtocolConfig) ClientOptio
}
}
+// SetClientRouters sets the routers configuration for the client.
+// This function is used by the framework to configure router settings from
global configuration.
+// It accepts a slice of router configurations.
+func SetClientRouters(routers []*global.RouterConfig) ClientOption {
+ return func(opts *ClientOptions) {
+ opts.Routers = routers
+ }
+}
Review Comment:
从 diff 看：
SetClientRouters(routers []*global.RouterConfig) 是直接赋值；
setRouters() 是 append 现有 slice；
refer() 把 refOpts.Routers 直接放进 URL attribute；
injectRouterConfig() 又把 routerCfg 原样传给实现了 StaticConfigSetter 的 router。
这意味着这里很可能是同一批 *global.RouterConfig 指针在多层共享。如果后续某个 router
实现内部会缓存、修改、补默认值，或者外部还在复用/修改这组配置，就可能出现隐式共享状态问题。更关键的是，PR 描述的流程图里写的是
routerConfigs.Store(key, cfgCopy)，但从公开 diff 可见的 injectRouterConfig() 是直接
setter.SetStaticConfig(routerCfg)，至少在这一层我没有看到“复制语义”被落实。
在 SetClientRouters / setRouters / refer 这几层至少做一次深拷贝；
或者明确约束 SetStaticConfig 的实现必须 clone，不允许持有外部指针。
--
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]
