AlexStocks commented on code in PR #892:
URL: https://github.com/apache/dubbo-go-pixiu/pull/892#discussion_r2934587858
##########
pkg/hotreload/route_reloader.go:
##########
@@ -56,16 +54,44 @@ func (r *RouteReloader) CheckUpdate(oldConfig, newConfig
*model.Bootstrap) bool
// HotReload applies the new route configuration.
func (r *RouteReloader) HotReload(oldConfig, newConfig *model.Bootstrap) error
{
- oldRoutes := extractRoutes(oldConfig)
- newRoutes := extractRoutes(newConfig)
+ logger.Info("Starting route hot reload")
+
+ srv := server.GetServer()
+ if srv == nil {
+ logger.Error("Server instance is nil")
+ return errors.New("server instance is nil")
+ }
+ logger.Info("Got server instance")
+
+ logger.Info("Reinitializing server components...")
+
+ listenerManager := srv.GetListenerManager()
+ if listenerManager == nil {
+ logger.Error("Listener manager is nil")
+ return errors.New("listener manager is nil")
+ }
+
+ refreshed := 0
+ for _, listener := range newConfig.StaticResources.Listeners {
+ logger.Infof("Refreshing listener: name=%s, protocol=%s",
listener.Name, listener.ProtocolStr)
+
+ srv.GetRouterManager().ClearRouterListeners()
Review Comment:
`ClearRouterListeners()` 在 `for` 循环体内被反复调用，每次迭代都清空所有 listener。如果有多个
listener，第一次 `UpdateListener` 新注册的 listener 在第二次迭代就被清掉了。
建议把 `ClearRouterListeners()` 移到 `for` 循环之前，只清一次：
```go
srv.GetRouterManager().ClearRouterListeners()
for _, listener := range newConfig.StaticResources.Listeners {
// ...
if err := listenerManager.UpdateListener(listener); err != nil {
// ...
}
}
```
##########
pkg/server/router_manager.go:
##########
@@ -46,6 +46,19 @@ func (rm *RouterManager) AddRouterListener(l RouterListener)
{
rm.rls = append(rm.rls, l)
}
+func (rm *RouterManager) RemoveRouterListener(l RouterListener) {
+ for i, listener := range rm.rls {
+ if listener == l {
+ rm.rls = append(rm.rls[:i], rm.rls[i+1:]...)
+ return
+ }
+ }
+}
+
+func (rm *RouterManager) ClearRouterListeners() {
+ rm.rls = nil
Review Comment:
`RouterManager` 的 `rls` 切片在
`AddRouterListener`、`RemoveRouterListener`、`ClearRouterListeners`、`AddRouter`、`DeleteRouter`
中被读写，但没有任何锁保护。
hot reload 和请求路由在不同 goroutine 中同时执行，会导致 data race。
建议给 `RouterManager` 加
`sync.RWMutex`，写操作（Add/Remove/Clear）用写锁，遍历通知（AddRouter/DeleteRouter）用读锁。
##########
pkg/config/config_load.go:
##########
@@ -58,6 +58,11 @@ func GetBootstrap() *model.Bootstrap {
return config
}
+// SetBootstrap set config global
+func SetBootstrap(cfg *model.Bootstrap) {
+ config = cfg
Review Comment:
`config` 是 package 级全局变量，`GetBootstrap()` 和 `SetBootstrap()` 在不同 goroutine
中被调用（reload handler goroutine 写，请求处理 goroutine 读），没有任何同步机制，存在 data race。
建议使用 `atomic.Pointer[model.Bootstrap]` 或 `sync.RWMutex` 保护读写。
--
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]
