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


##########
filter/graceful_shutdown/consumer_filter.go:
##########
@@ -91,3 +112,89 @@ func (f *consumerGracefulShutdownFilter) Set(name string, 
conf any) {
                // do nothing
        }
 }
+
+// isClosingInvoker checks if invoker is in closing list
+func (f *consumerGracefulShutdownFilter) isClosingInvoker(invoker 
base.Invoker) bool {
+       key := invoker.GetURL().String()
+       if expireTime, ok := f.closingInvokers.Load(key); ok {
+               if time.Now().Before(expireTime.(time.Time)) {
+                       return true
+               }
+               f.closingInvokers.Delete(key)

Review Comment:
   `markClosingInvoker` 调用了 `bi.SetAvailable(false)`,30s 过期后 
`closingInvokers.Delete(key)` 把节点从 map 移除,但 `IsAvailable()` 仍然是 
`false`,负载均衡器会永久跳过该节点,产生永久不可用的 invoker。
   
   建议:`closingInvokers.Delete(key)` 之后同步调用 `bi.SetAvailable(true)` 恢复状态;或者干脆不调用 
`SetAvailable(false)`,只靠 `closingInvokers` 的过期机制控制路由。



##########
protocol/grpc/grpc_protocol.go:
##########
@@ -38,6 +39,28 @@ const (
 
 func init() {
        extension.SetProtocol(GRPC, GetProtocol)
+
+       // register graceful shutdown callback
+       extension.SetGracefulShutdownCallback(GRPC, func(ctx context.Context) 
error {
+               grpcProto := GetProtocol()
+               if grpcProto == nil {
+                       return nil
+               }
+
+               gp, ok := grpcProto.(*GrpcProtocol)
+               if !ok {
+                       return nil
+               }
+
+               gp.serverLock.Lock()
+               defer gp.serverLock.Unlock()
+
+               for _, server := range gp.serverMap {
+                       server.GracefulStop()

Review Comment:
   `GracefulStop()` 是阻塞调用,会等待所有活跃 RPC 完成。在持有 `serverLock` 的情况下调用,若 
`GracefulStop()` 等待的 RPC handler 内部触发了 `Export()` 等需要 `serverLock` 
的操作,会死锁。`triple.go` 的对应实现存在同样问题。
   
   建议:先拷贝 server 列表,释放锁,再逐一调用 `GracefulStop()`:
   ```go
   gp.serverLock.Lock()
   servers := make([]*Server, 0, len(gp.serverMap))
   for _, s := range gp.serverMap {
       servers = append(servers, s)
   }
   gp.serverLock.Unlock()
   for _, s := range servers {
       s.GracefulStop()
   }
   ```



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