AlexStocks commented on code in PR #995:
URL: https://github.com/apache/dubbo-go-pixiu/pull/995#discussion_r4180466818


##########
admin/core/xds.go:
##########
@@ -132,12 +132,20 @@ func runXDSServer(srv envoyServer.Server, lis 
net.Listener) error {
        grpcServer := grpc.NewServer(grpcOptions...)
 
        registerServer(grpcServer, srv)
-
        logger.Infof("management server listening on %s", lis.Addr())
-       if err := grpcServer.Serve(lis); err != nil {
+
+       serveErr := make(chan error, 1)
+       go func() {
+               serveErr <- grpcServer.Serve(lis)
+       }()
+
+       select {
+       case err := <-serveErr:
                return err
+       case <-ctx.Done():
+               grpcServer.GracefulStop()
+               return <-serveErr

Review Comment:
   [P2] `GracefulStop()` 之后的返回值被当成了失败,会误报 xDS 监听错误。
   
   `ctx.Done()` 分支是 `grpcServer.GracefulStop(); return <-serveErr`。我用仓库内实际的 
gRPC 版本实测了这个返回值(存证:`evidence/probe-gracefulstop.txt`):
   
   ```
   Serve returned grpc: the server has been stopped after GracefulStop
   ```
   
   也就是 `Serve` 在优雅停止后返回的是 `grpc.ErrServerStopped`,不是 `nil`。这个值经 `return 
<-serveErr` 一路传出 `runXDSServer` → `StartxDsServer`,然后命中 
`admin/core/xds.go:107-110`:
   
   ```go
   if err := runXDSServer(ctx, srv, lis); err != nil {
       adminxds.DefaultStatusStore.RecordListenError(err)   // <- 记成"xDS 没在监听"
       return err
   }
   ```
   
   `RecordListenError` 会写 `status.Listening = false` 和 
`status.ListenError`(`admin/xds/status.go:111-113`),而这个状态是**对外可观察**的:`admin/controller/xds/status.go:71`
 用 `status.ListenError != ""` 判定 `Degraded`。也就是说,一次**完全正常**的优雅关闭(HTTP 侧先失败或收到信号 
→ cancel → xDS 优雅停止)会让 xDS 状态接口在退出瞬间报"降级 + 监听失败"。
   
   **关于退出码,这条我核实后认为不是问题**:`runCoordinated` 的 `errCh` 容量为 1 且用 `default` 
丢弃后来者,所以先到的真实错误(`http server: ...` 或 `net.Listen` 的错误)一定胜出,`ErrServerStopped` 
抢不到。`RunServer` 的返回值语义是对的,这里不需要改。
   
   建议在 `runXDSServer` 里把优雅停止和真实失败区分开:
   
   ```go
   case <-ctx.Done():
       grpcServer.GracefulStop()
       if err := <-serveErr; err != nil && !errors.Is(err, 
grpc.ErrServerStopped) {
           return err
       }
       return nil
   ```
   
   这样 `RecordListenError` 就只会记录真正的监听失败。



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