AlexStocks commented on code in PR #993:
URL: https://github.com/apache/dubbo-go-pixiu/pull/993#discussion_r3563646788
##########
pkg/server/listener_manager.go:
##########
@@ -98,33 +102,124 @@ func (lm *ListenerManager) gracefulShutdownInit() {
sig := <-signals
logger.Infof("get signal %s, dubbo-go-pixiu will start
shutdown.", sig)
- time.AfterFunc(timeout, func() {
- logger.Warn("Shutdown gracefully timeout, listeners
will shutdown immediately. ")
- os.Exit(0)
- })
+ // Handle shutdown signal (extracted for testability)
+ shutdownErrors, timedOut := lm.handleShutdownSignal(sig,
timeout)
+
+ // Exit with appropriate code based on shutdown errors
+ if len(shutdownErrors) > 0 {
+ logger.Errorf("Shutdown completed with %d errors",
len(shutdownErrors))
+ os.Exit(1)
+ }
+ if timedOut {
+ logger.Warn("Shutdown gracefully timeout, some
listeners may not have shut down cleanly")
+ }
+ os.Exit(0)
+ }()
+}
- for _, listener := range lm.activeListenerService {
+// handleShutdownSignal processes a shutdown signal by coordinating listener
shutdowns.
+// It returns a slice of errors from failed shutdowns and a boolean indicating
timeout.
+// This function is extracted from gracefulShutdownInit for testability.
+func (lm *ListenerManager) handleShutdownSignal(sig os.Signal, timeout
time.Duration) ([]error, bool) {
+ // Build shutdown functions for all listeners
+ shutdownFuncs := make([]ShutdownFunc, 0, len(lm.activeListenerService))
+ for _, listener := range lm.activeListenerService {
+ shutdownFuncs = append(shutdownFuncs, func() error {
lm.shutdownWG.Add(1)
- go func(listener *wrapListenerService) {
- err := listener.ShutDown(lm.shutdownWG)
- if err != nil {
- logger.Errorf("Shutdown Error: %+v",
err)
- os.Exit(0)
- }
- }(listener)
+ // Note: listener.ShutDown() internally calls wg.Done()
+ return listener.ShutDown(lm.shutdownWG)
+ })
+ }
+
+ // Execute shutdown coordination
+ shutdownErrors, timedOut := shutdownListeners(shutdownFuncs, timeout,
defaultPerListenerTimeout)
+
+ // those signals' original behavior is exit with dump ths stack, so we
try to keep the behavior
+ for _, dumpSignal := range shutdown.DumpHeapShutdownSignals {
+ if sig == dumpSignal {
+ debug.WriteHeapDump(os.Stdout.Fd())
}
- lm.shutdownWG.Wait()
+ }
+
+ return shutdownErrors, timedOut
+}
- // those signals' original behavior is exit with dump ths
stack, so we try to keep the behavior
- for _, dumpSignal := range shutdown.DumpHeapShutdownSignals {
- if sig == dumpSignal {
- debug.WriteHeapDump(os.Stdout.Fd())
+// shutdownListeners coordinates the shutdown of multiple listeners.
+// It returns a slice of errors from failed shutdowns and a boolean indicating
+// whether the shutdown timed out before all listeners completed.
+// This function is extracted from gracefulShutdownInit for testability.
+// The perListenerTimeout parameter controls how long we wait for each
individual
+// listener goroutine before considering it stuck (default 5 seconds in
production).
+func shutdownListeners(shutdownFuncs []ShutdownFunc, timeout time.Duration,
perListenerTimeout time.Duration) ([]error, bool) {
+ if len(shutdownFuncs) == 0 {
+ return nil, false
+ }
+
+ // Create error collection channel with capacity for all listeners
+ errCh := make(chan error, len(shutdownFuncs))
+ // Create done channel to track completion of each listener goroutine
+ doneCh := make(chan struct{}, len(shutdownFuncs))
+
+ // Start shutdown for all listeners
+ for _, shutdownFunc := range shutdownFuncs {
+ go func(fn ShutdownFunc) {
+ err := fn()
+ if err != nil {
+ logger.Errorf("Shutdown Error: %+v", err)
+ errCh <- err
+ }
+ // Signal that this goroutine has completed (after
potential error send)
+ doneCh <- struct{}{}
+ }(shutdownFunc)
+ }
+
+ // Wait for all listener goroutines to complete or timeout
+ // We use doneCh instead of relying solely on WaitGroup because:
+ // - listener.ShutDown() calls wg.Done() before returning
+ // - this ensures we wait until error is sent to errCh
+ allDone := make(chan struct{})
+ go func() {
+ for i := 0; i < len(shutdownFuncs); i++ {
+ select {
+ case <-doneCh:
+ // listener goroutine completed
+ case <-time.After(perListenerTimeout):
Review Comment:
[P0] 这里额外引入固定 5 秒的 `perListenerTimeout`，会覆盖用户配置的整体优雅关闭语义。现有
TCP/HTTP2/Triple/gRPC listener 都可能按 bootstrap 中的 shutdown timeout 等待活跃请求；例如配置
30 秒且只有一个 listener 时，本循环 5 秒后就把该 listener 当作已处理并关闭 `allDone`，随后调用方以
`timedOut=false` 执行 `os.Exit(0)`，直接中断本应继续 25 秒的优雅关闭。不要用无身份的固定超时消费“完成名额”；应只由整体配置
timeout 控制退出，或为每个 listener 使用不短于剩余整体期限的独立上下文并明确记录未完成项。
--
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]
