AlexStocks commented on code in PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#discussion_r2902465158


##########
common/extension/graceful_shutdown.go:
##########
@@ -19,35 +19,41 @@ package extension
 
 import (
        "container/list"
+       "context"
 )
 
-var customShutdownCallbacks = list.New()
-
-/**
- * AddCustomShutdownCallback
- * you should not make any assumption about the order.
- * For example, if you have more than one callbacks, and you wish the order is:
- * callback1()
- * callback2()
- * ...
- * callbackN()
- * Then you should put then together:
- * func callback() {
- *     callback1()
- *     callback2()
- *     ...
- *     callbackN()
- * }
- * I think the order of custom callbacks should be decided by the users.
- * Even though I can design a mechanism to support the ordered custom 
callbacks,
- * the benefit of that mechanism is low.
- * And it may introduce much complication for another users.
- */
+// GracefulShutdownCallback is the callback for graceful shutdown
+// name: protocol name such as "grpc", "tri", "dubbo"
+// returns error if notify failed
+type GracefulShutdownCallback func(ctx context.Context) error
+
+var (
+       customShutdownCallbacks   = list.New()
+       gracefulShutdownCallbacks = make(map[string]GracefulShutdownCallback)
+)
+
+// AddCustomShutdownCallback adds custom shutdown callback
 func AddCustomShutdownCallback(callback func()) {
        customShutdownCallbacks.PushBack(callback)
 }
 
-// GetAllCustomShutdownCallbacks gets all custom shutdown callbacks
+// GetAllCustomShutdownCallbacks returns all custom shutdown callbacks
 func GetAllCustomShutdownCallbacks() *list.List {
        return customShutdownCallbacks
 }
+
+// SetGracefulShutdownCallback sets protocol-level graceful shutdown callback
+func SetGracefulShutdownCallback(name string, f GracefulShutdownCallback) {
+       gracefulShutdownCallbacks[name] = f
+}
+
+// GetGracefulShutdownCallback returns protocol's graceful shutdown callback
+func GetGracefulShutdownCallback(name string) (GracefulShutdownCallback, bool) 
{
+       f, ok := gracefulShutdownCallbacks[name]
+       return f, ok
+}
+
+// GetAllGracefulShutdownCallbacks returns all protocol's graceful shutdown 
callbacks
+func GetAllGracefulShutdownCallbacks() map[string]GracefulShutdownCallback {
+       return gracefulShutdownCallbacks

Review Comment:
   `gracefulShutdownCallbacks` 是全局 map,`SetGracefulShutdownCallback` 和 
`GetAllGracefulShutdownCallbacks` 没有任何锁保护,Go race detector 会报警。
   
   `GetAllGracefulShutdownCallbacks` 直接返回内部 map 引用,调用方若并发修改则 crash。
   
   建议:加 `sync.RWMutex` 保护;`GetAllGracefulShutdownCallbacks` 返回副本而不是原始 map。



##########
graceful_shutdown/shutdown.go:
##########
@@ -150,6 +167,71 @@ func destroyRegistries() {
        registryProtocol.Destroy()
 }
 
+// notifyLongConnectionConsumers notifies all connected consumers via long 
connections
+func notifyLongConnectionConsumers() {
+       logger.Info("Graceful shutdown --- Notify long connection consumers.")
+
+       notifyTimeout := 3 * time.Second

Review Comment:
   `notifyTimeout` 硬编码 3s,与 `ShutdownConfig` 中 
`StepTimeout`、`ConsumerUpdateWaitTime` 等配置体系完全割裂,无法通过配置控制。
   
   建议:函数签名改为接收 `shutdown *global.ShutdownConfig`,复用已有的 timeout 配置字段,或新增 
`NotifyTimeout` 字段。



-- 
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