You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "Oxidaner (via GitHub)" <gi...@apache.org> on 2026/03/08 06:09:11 UTC

[PR] Feat/graceful shutdown [dubbo-go]

Oxidaner opened a new pull request, #3235:
URL: https://github.com/apache/dubbo-go/pull/3235

    ## What kind of change does this PR introduce?
   
     - [x] Feature
   
     ## What is the current behavior?
   
     Related Issue: #1977
   
     ## What is the new behavior?
   
     This PR adds graceful shutdown enhancement with the following features:
   
   1. **Exponential backoff retry for active notification**
   
     - When sending notification to consumers fails, the server will retry with exponential backoff
     - Default: 500ms → 1s → 2s, max 3 retries
   
   2. **Active awareness via connection error detection**
   
     - Client detects connection errors (EOF, broken pipe, gRPC closing, http2 closing)
     - Marks the invoker as closing to avoid routing requests to unavailable instances
   
   3. **Closing flag in response**
   
     - Server adds closing flag in response attachment
     - Client checks this flag to detect closing state
   
   4. **Protocol-level graceful shutdown callback**
   
     - Added `SetGracefulShutdownCallback` for protocol-specific shutdown logic
     - gRPC protocol uses this for `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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#discussion_r2902464903


##########
protocol/triple/triple.go:
##########
@@ -47,6 +47,30 @@ var (
 
 func init() {
 	extension.SetProtocol(TRIPLE, GetProtocol)
+
+	// register graceful shutdown callback
+	extension.SetGracefulShutdownCallback(TRIPLE, func(ctx context.Context) error {
+		tp := GetProtocol()
+		if tp == nil {
+			return nil
+		}
+
+		var ok bool
+		tripleProtocol = tp.(*TripleProtocol)
+		if !ok {

Review Comment:
   类型断言写错了:`tripleProtocol = tp.(*TripleProtocol)` 没有捕获 `ok`,而 `var ok bool` 声明后从未赋值,始终是 `false`。导致 `if !ok { return nil }` 永远成立,回调永远提前返回,`grpc.GracefulStop()` 从不执行。triple 的 graceful shutdown 完全失效。
   
   建议改为:
   ```go
   tripleProtocol, ok := tp.(*TripleProtocol)
   if !ok {
       return nil
   }
   ```



-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#issuecomment-4106617618

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3235) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [10 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3235&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3235&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3235&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3235&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3235&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3235)
   
   


-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#issuecomment-4018486642

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3235) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3235&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3235&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3235&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3235&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3235&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3235)
   
   


-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "Alanxtl (via GitHub)" <gi...@apache.org>.
Alanxtl commented on code in PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#discussion_r2902921542


##########
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)
+	}
+	return false
+}
+
+// isClosingResponse checks if response contains closing flag
+func (f *consumerGracefulShutdownFilter) isClosingResponse(result result.Result) bool {
+	if result != nil && result.Attachments() != nil {
+		if v, ok := result.Attachments()[constant.GracefulShutdownClosingKey]; ok {
+			if v == "true" {
+				return true
+			}
+		}
+	}
+	return false
+}
+
+// markClosingInvoker marks invoker as closing and sets available=false
+func (f *consumerGracefulShutdownFilter) markClosingInvoker(invoker base.Invoker) {
+	key := invoker.GetURL().String()
+	expireTime := time.Now().Add(f.getClosingInvokerExpireTime())
+	f.closingInvokers.Store(key, expireTime)
+
+	logger.Infof("Graceful shutdown: marked invoker as closing: %s, will expire at %v, IsAvailable=%v",
+		key, expireTime, invoker.IsAvailable())
+
+	if bi, ok := invoker.(*base.BaseInvoker); ok {
+		bi.SetAvailable(false)
+		logger.Infof("Graceful shutdown: set invoker unavailable: %s, IsAvailable now=%v",
+			key, invoker.IsAvailable())
+	}
+}
+
+func (f *consumerGracefulShutdownFilter) getClosingInvokerExpireTime() time.Duration {
+	if f.shutdownConfig != nil && f.shutdownConfig.ClosingInvokerExpireTime != "" {
+		if duration, err := time.ParseDuration(f.shutdownConfig.ClosingInvokerExpireTime); err == nil && duration > 0 {
+			return duration
+		}
+	}
+	// default 30s, also try parsing numeric string as milliseconds
+	if f.shutdownConfig != nil && f.shutdownConfig.ClosingInvokerExpireTime != "" {
+		if ms, err := strconv.ParseInt(f.shutdownConfig.ClosingInvokerExpireTime, 10, 64); err == nil && ms > 0 {
+			return time.Duration(ms) * time.Millisecond
+		}
+	}
+	return 30 * time.Second
+}
+
+// handleRequestError handles request errors and marks invoker as unavailable for connection errors
+func (f *consumerGracefulShutdownFilter) handleRequestError(invoker base.Invoker, err error) {
+	if err == nil {
+		return
+	}
+
+	// check for connection-related errors
+	errMsg := err.Error()
+	isConnectionError := strings.Contains(errMsg, "client has closed") ||
+		strings.Contains(errMsg, "connection") ||
+		strings.Contains(errMsg, "EOF") ||
+		strings.Contains(errMsg, "broken pipe") ||
+		strings.Contains(errMsg, "gRPC") && strings.Contains(errMsg, "closing") ||
+		strings.Contains(errMsg, "http2") && strings.Contains(errMsg, "close")
+
+	if isConnectionError {
+		key := invoker.GetURL().String()
+		expireTime := time.Now().Add(f.getClosingInvokerExpireTime())
+		f.closingInvokers.Store(key, expireTime)
+
+		logger.Infof("Graceful shutdown: connection error detected for invoker: %s, marking as closing, will expire at %v, IsAvailable=%v",

Review Comment:
   其他的logger.Infof都改一下



-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#discussion_r2902466633


##########
filter/graceful_shutdown/provider_filter.go:
##########
@@ -85,6 +84,12 @@ func (f *providerGracefulShutdownFilter) Invoke(ctx context.Context, invoker bas
 // OnResponse reduces the number of active processes then return the process result
 func (f *providerGracefulShutdownFilter) OnResponse(ctx context.Context, result result.Result, invoker base.Invoker, invocation base.Invocation) result.Result {
 	f.shutdownConfig.ProviderActiveCount.Dec()
+
+	// add closing flag to response
+	if f.isClosing() {
+		result.AddAttachment(constant.GracefulShutdownClosingKey, "true")

Review Comment:
   `result` 可能为 nil(invoker 返回 nil result 时),直接调用 `result.AddAttachment` 会 panic。
   
   建议:加 nil 检查:
   ```go
   if f.isClosing() && result != nil {
       result.AddAttachment(constant.GracefulShutdownClosingKey, "true")
   }
   ```



-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#discussion_r2969086748


##########
filter/graceful_shutdown/consumer_filter.go:
##########
@@ -50,27 +54,61 @@ func init() {
 }
 
 type consumerGracefulShutdownFilter struct {
-	shutdownConfig *global.ShutdownConfig
+	shutdownConfig      *global.ShutdownConfig
+	closingEventHandler gracefulshutdown.ClosingEventHandler
+	closingInvokers     sync.Map // map[string]time.Time (url key -> expire time)
 }
 
+const consumerCountMarkedKey = "dubbo-go-graceful-shutdown-consumer-counted"
+
 func newConsumerGracefulShutdownFilter() filter.Filter {
 	if csf == nil {
 		csfOnce.Do(func() {

Review Comment:
   ## consumer_filter.go:33 - 常量定义位置不一致
   
   `consumerCountMarkedKey` 在 consumer_filter.go 第 33 行定义,但类似的 `providerCountMarkedKey` 在 provider_filter.go 第 16 行定义。
   
   **建议**: 将这些常量统一放到 compat.go 或单独的 constants.go 文件中,便于维护。



##########
graceful_shutdown/shutdown.go:
##########
@@ -125,31 +140,125 @@ func totalTimeout(shutdown *global.ShutdownConfig) time.Duration {
 }
 
 func beforeShutdown(shutdown *global.ShutdownConfig) {
-	destroyRegistries()
+	// 1. mark closing state
+	logger.Info("Graceful shutdown --- Mark closing state.")
+	shutdown.Closing.Store(true)
+
+	// 2. unregister services from registries
+	unregisterRegistries()
+
+	// 3. notify long connection consumers
+	notifyLongConnectionConsumers(shutdown)
+
+	// 4. wait and accept new requests
 	// waiting for a short time so that the clients have enough time to get the notification that server shutdowns
 	// The value of configuration depends on how long the clients will get notification.
 	waitAndAcceptNewRequests(shutdown)
 
+	// 5. reject new requests and wait for in-flight requests
 	// reject sending/receiving the new request but keeping waiting for accepting requests
 	waitForSendingAndReceivingRequests(shutdown)
 
-	// destroy all protocols
+	// 6. destroy protocols
 	destroyProtocols()
 
-	logger.Info("Graceful shutdown --- Execute the custom callbacks.")
-	customCallbacks := extension.GetAllCustomShutdownCallbacks()
-	for callback := customCallbacks.Front(); callback != nil; callback = callback.Next() {
-		callback.Value.(func())()
-	}
+	// 7. execute custom callbacks
+	executeCustomShutdownCallbacks(shutdown)
 }
 
-// destroyRegistries destroys RegistryProtocol directly.
-func destroyRegistries() {
-	logger.Info("Graceful shutdown --- Destroy all registriesConfig. ")
-	registryProtocol := extension.GetProtocol(constant.RegistryProtocol)
+// unregisterRegistries unregisters exported services from registries during graceful shutdown.
+// If the registry protocol does not expose a narrower unregister capability, it falls back to Destroy.
+func unregisterRegistries() {
+	logger.Info("Graceful shutdown --- Unregister exported services from registries.")
+	registryProtocol, ok := getProtocolSafely(constant.RegistryProtocol)
+	if !ok {
+		logger.Warnf("Graceful shutdown --- Registry protocol %s is not registered, skip unregistering registries.", constant.RegistryProtocol)
+		return
+	}
+
+	if unregisterer, ok := registryProtocol.(protocolbase.RegistryUnregisterer); ok {
+		unregisterer.UnregisterRegistries()
+		return
+	}
+
+	logger.Warnf("Graceful shutdown --- Registry protocol %s does not support unregister-only shutdown, falling back to Destroy().", constant.RegistryProtocol)
 	registryProtocol.Destroy()
 }
 
+// notifyLongConnectionConsumers notifies all connected consumers via long connections
+func notifyLongConnectionConsumers(shutdown *global.ShutdownConfig) {
+	logger.Info("Graceful shutdown --- Notify long connection consumers.")
+
+	notifyTimeout := parseDuration(shutdown.NotifyTimeout, notifyTimeoutDesc, defaultNotifyTimeout)
+	callbacks := extension.GracefulShutdownCallbacks()
+	var wg sync.WaitGroup
+	for name, callback := range callbacks {
+		wg.Add(1)
+		go func(name string, callback extension.GracefulShutdownCallback) {
+			defer wg.Done()
+			ctx, cancel := context.WithTimeout(context.Background(), notifyTimeout)
+			defer cancel()
+			notifyWithRetry(ctx, name, callback)
+		}(name, callback)
+	}
+	wg.Wait()
+}

Review Comment:
   ## shutdown.go:140 - notifyWithRetry 中 ctx 超时后重试可能继续
   
   当 context 超时(notifyTimeout)后,backoff.RetryNotify 可能会继续执行正在等待的重试。虽然代码在第 172-174 行检查了 ctx.Err(),但如果在重试等待期间 context 取消,goroutine 可能无法及时退出。
   
   **建议**: 在 notifyWithRetry 返回前再次检查 ctx.Err(),确保超时后立即退出。



-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#discussion_r2902465650


##########
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
+	ctx, cancel := context.WithTimeout(context.Background(), notifyTimeout)
+	defer cancel()
+
+	callbacks := extension.GetAllGracefulShutdownCallbacks()
+
+	for name, callback := range callbacks {
+		notifyWithRetry(ctx, name, callback)
+	}
+}
+
+// notifyWithRetry notifies with exponential backoff retry
+func notifyWithRetry(ctx context.Context, name string, callback extension.GracefulShutdownCallback) {
+	maxRetries := defaultMaxRetries
+	baseDelay := defaultRetryBaseDelay
+	maxDelay := defaultRetryMaxDelay
+
+	var lastErr error
+	for attempt := 0; attempt <= maxRetries; attempt++ {
+		// check timeout
+		select {
+		case <-ctx.Done():
+			logger.Warnf("Graceful shutdown --- Notify %s timeout after %d attempts, continuing...", name, attempt)
+			return
+		default:
+		}
+
+		err := callback(ctx)
+		if err == nil {
+			logger.Infof("Graceful shutdown --- Notify %s completed", name)
+			return
+		}
+
+		lastErr = err
+		logger.Warnf("Graceful shutdown --- Notify %s attempt %d failed: %v", name, attempt+1, err)
+
+		// retry with exponential backoff
+		if attempt < maxRetries {
+			delay := baseDelay
+			for i := 0; i < attempt; i++ {
+				delay *= 2

Review Comment:
   三次重试的总等待时间:500ms + 1s + 2s = 3.5s,超过了外层 context timeout 3s,实际最多只能完成 1-2 次重试,配置自相矛盾。
   
   另外 `go.mod` 里本就有 `github.com/cenkalti/backoff/v4` 依赖,不需要手写指数退避逻辑,直接用更可靠。
   
   建议:要么把 `notifyTimeout` 调大(至少 10s),要么减少重试次数/延迟;退避逻辑改用 `cenkalti/backoff/v4`。



##########
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)
+	}
+	return false
+}
+
+// isClosingResponse checks if response contains closing flag
+func (f *consumerGracefulShutdownFilter) isClosingResponse(result result.Result) bool {
+	if result != nil && result.Attachments() != nil {
+		if v, ok := result.Attachments()[constant.GracefulShutdownClosingKey]; ok {
+			if v == "true" {
+				return true
+			}
+		}
+	}
+	return false
+}
+
+// markClosingInvoker marks invoker as closing and sets available=false
+func (f *consumerGracefulShutdownFilter) markClosingInvoker(invoker base.Invoker) {
+	key := invoker.GetURL().String()
+	expireTime := time.Now().Add(f.getClosingInvokerExpireTime())
+	f.closingInvokers.Store(key, expireTime)
+
+	logger.Infof("Graceful shutdown: marked invoker as closing: %s, will expire at %v, IsAvailable=%v",
+		key, expireTime, invoker.IsAvailable())
+
+	if bi, ok := invoker.(*base.BaseInvoker); ok {
+		bi.SetAvailable(false)
+		logger.Infof("Graceful shutdown: set invoker unavailable: %s, IsAvailable now=%v",
+			key, invoker.IsAvailable())
+	}
+}
+
+func (f *consumerGracefulShutdownFilter) getClosingInvokerExpireTime() time.Duration {
+	if f.shutdownConfig != nil && f.shutdownConfig.ClosingInvokerExpireTime != "" {
+		if duration, err := time.ParseDuration(f.shutdownConfig.ClosingInvokerExpireTime); err == nil && duration > 0 {
+			return duration
+		}
+	}
+	// default 30s, also try parsing numeric string as milliseconds
+	if f.shutdownConfig != nil && f.shutdownConfig.ClosingInvokerExpireTime != "" {
+		if ms, err := strconv.ParseInt(f.shutdownConfig.ClosingInvokerExpireTime, 10, 64); err == nil && ms > 0 {
+			return time.Duration(ms) * time.Millisecond
+		}
+	}
+	return 30 * time.Second
+}
+
+// handleRequestError handles request errors and marks invoker as unavailable for connection errors
+func (f *consumerGracefulShutdownFilter) handleRequestError(invoker base.Invoker, err error) {
+	if err == nil {
+		return
+	}
+
+	// check for connection-related errors
+	errMsg := err.Error()
+	isConnectionError := strings.Contains(errMsg, "client has closed") ||
+		strings.Contains(errMsg, "connection") ||

Review Comment:
   `strings.Contains(errMsg, "connection")` 过于宽泛,任何包含 "connection" 字样的业务错误(如 "database connection pool exhausted"、"connection refused by business logic")都会被误判为连接关闭错误,将正常 invoker 标记为 closing 并 `SetAvailable(false)`,导致请求被永久拒绝直到 30s 过期。
   
   建议:改用 `errors.Is` 对已知错误类型匹配,或缩窄字符串匹配为 `"connection reset"`、`"use of closed network connection"` 等具体错误。



-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#issuecomment-4097719644

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3235) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [10 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3235&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3235&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3235&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3235&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3235&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3235)
   
   


-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "Oxidaner (via GitHub)" <gi...@apache.org>.
Oxidaner commented on code in PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#discussion_r2903446648


##########
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)
+	}
+	return false
+}
+
+// isClosingResponse checks if response contains closing flag
+func (f *consumerGracefulShutdownFilter) isClosingResponse(result result.Result) bool {
+	if result != nil && result.Attachments() != nil {
+		if v, ok := result.Attachments()[constant.GracefulShutdownClosingKey]; ok {
+			if v == "true" {
+				return true
+			}
+		}
+	}
+	return false
+}
+
+// markClosingInvoker marks invoker as closing and sets available=false
+func (f *consumerGracefulShutdownFilter) markClosingInvoker(invoker base.Invoker) {
+	key := invoker.GetURL().String()
+	expireTime := time.Now().Add(f.getClosingInvokerExpireTime())
+	f.closingInvokers.Store(key, expireTime)
+
+	logger.Infof("Graceful shutdown: marked invoker as closing: %s, will expire at %v, IsAvailable=%v",
+		key, expireTime, invoker.IsAvailable())
+
+	if bi, ok := invoker.(*base.BaseInvoker); ok {
+		bi.SetAvailable(false)
+		logger.Infof("Graceful shutdown: set invoker unavailable: %s, IsAvailable now=%v",
+			key, invoker.IsAvailable())
+	}
+}
+
+func (f *consumerGracefulShutdownFilter) getClosingInvokerExpireTime() time.Duration {
+	if f.shutdownConfig != nil && f.shutdownConfig.ClosingInvokerExpireTime != "" {
+		if duration, err := time.ParseDuration(f.shutdownConfig.ClosingInvokerExpireTime); err == nil && duration > 0 {
+			return duration
+		}
+	}
+	// default 30s, also try parsing numeric string as milliseconds
+	if f.shutdownConfig != nil && f.shutdownConfig.ClosingInvokerExpireTime != "" {
+		if ms, err := strconv.ParseInt(f.shutdownConfig.ClosingInvokerExpireTime, 10, 64); err == nil && ms > 0 {
+			return time.Duration(ms) * time.Millisecond
+		}
+	}
+	return 30 * time.Second
+}
+
+// handleRequestError handles request errors and marks invoker as unavailable for connection errors
+func (f *consumerGracefulShutdownFilter) handleRequestError(invoker base.Invoker, err error) {
+	if err == nil {
+		return
+	}
+
+	// check for connection-related errors
+	errMsg := err.Error()
+	isConnectionError := strings.Contains(errMsg, "client has closed") ||
+		strings.Contains(errMsg, "connection") ||

Review Comment:
   Sorry, I'll correct it



-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#discussion_r2969087027


##########
graceful_shutdown/closing_ack.go:
##########
@@ -0,0 +1,95 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package graceful_shutdown
+
+import (
+	"strings"
+	"sync"
+	"sync/atomic"
+)
+
+// ClosingAckStats is a lightweight, consumer-side statistic for active closing notices.
+// Received means the consumer observed an active notice; Removed means directory removal
+// succeeded; Missed means the notice was observed but did not match a local directory entry.
+type ClosingAckStats struct {
+	Received uint64
+	Removed  uint64
+	Missed   uint64
+}
+
+type closingAckCounter struct {
+	received atomic.Uint64
+	removed  atomic.Uint64
+	missed   atomic.Uint64
+}
+
+type closingAckTracker struct {
+	counters sync.Map // map[string]*closingAckCounter
+}
+
+var defaultClosingAckTracker = &closingAckTracker{}
+
+func isActiveClosingSource(source string) bool {
+	return strings.HasSuffix(source, "-health-watch")
+}
+
+func (t *closingAckTracker) record(event ClosingEvent, removed bool) {
+	if !isActiveClosingSource(event.Source) {
+		return
+	}
+
+	counter := t.getOrCreateCounter(event.Source)
+	counter.received.Add(1)
+	if removed {
+		counter.removed.Add(1)
+		return
+	}
+	counter.missed.Add(1)
+}
+
+func (t *closingAckTracker) getOrCreateCounter(source string) *closingAckCounter {
+	if value, ok := t.counters.Load(source); ok {
+		return value.(*closingAckCounter)
+	}
+	counter := &closingAckCounter{}
+	actual, _ := t.counters.LoadOrStore(source, counter)
+	return actual.(*closingAckCounter)
+}
+
+func (t *closingAckTracker) snapshot() map[string]ClosingAckStats {
+	stats := make(map[string]ClosingAckStats)
+	t.counters.Range(func(key, value any) bool {
+		counter := value.(*closingAckCounter)
+		stats[key.(string)] = ClosingAckStats{
+			Received: counter.received.Load(),
+			Removed:  counter.removed.Load(),
+			Missed:   counter.missed.Load(),
+		}
+		return true
+	})
+	return stats
+}
+
+func (t *closingAckTracker) reset() {
+	t.counters = sync.Map{}
+}
+
+// DefaultClosingAckStats returns the process-wide active-notice ack statistics.
+func DefaultClosingAckStats() map[string]ClosingAckStats {

Review Comment:
   ## closing_ack.go:93 - reset() 存在竞态条件
   
   `t.counters = sync.Map{}` 直接赋值替换不是原子操作。如果其他 goroutine 同时调用 record() 或 snapshot(),可能导致数据不一致。
   
   **建议**: 使用 sync.Mutex 保护 reset() 操作,或者提供一个带锁的 Clear() 方法。



##########
graceful_shutdown/shutdown.go:
##########
@@ -125,31 +140,125 @@ func totalTimeout(shutdown *global.ShutdownConfig) time.Duration {
 }
 
 func beforeShutdown(shutdown *global.ShutdownConfig) {
-	destroyRegistries()
+	// 1. mark closing state
+	logger.Info("Graceful shutdown --- Mark closing state.")
+	shutdown.Closing.Store(true)
+
+	// 2. unregister services from registries
+	unregisterRegistries()
+
+	// 3. notify long connection consumers
+	notifyLongConnectionConsumers(shutdown)
+
+	// 4. wait and accept new requests
 	// waiting for a short time so that the clients have enough time to get the notification that server shutdowns
 	// The value of configuration depends on how long the clients will get notification.
 	waitAndAcceptNewRequests(shutdown)
 
+	// 5. reject new requests and wait for in-flight requests
 	// reject sending/receiving the new request but keeping waiting for accepting requests
 	waitForSendingAndReceivingRequests(shutdown)
 
-	// destroy all protocols
+	// 6. destroy protocols
 	destroyProtocols()
 
-	logger.Info("Graceful shutdown --- Execute the custom callbacks.")
-	customCallbacks := extension.GetAllCustomShutdownCallbacks()
-	for callback := customCallbacks.Front(); callback != nil; callback = callback.Next() {
-		callback.Value.(func())()
-	}
+	// 7. execute custom callbacks
+	executeCustomShutdownCallbacks(shutdown)
 }
 
-// destroyRegistries destroys RegistryProtocol directly.
-func destroyRegistries() {
-	logger.Info("Graceful shutdown --- Destroy all registriesConfig. ")
-	registryProtocol := extension.GetProtocol(constant.RegistryProtocol)
+// unregisterRegistries unregisters exported services from registries during graceful shutdown.
+// If the registry protocol does not expose a narrower unregister capability, it falls back to Destroy.
+func unregisterRegistries() {
+	logger.Info("Graceful shutdown --- Unregister exported services from registries.")
+	registryProtocol, ok := getProtocolSafely(constant.RegistryProtocol)
+	if !ok {
+		logger.Warnf("Graceful shutdown --- Registry protocol %s is not registered, skip unregistering registries.", constant.RegistryProtocol)
+		return
+	}
+
+	if unregisterer, ok := registryProtocol.(protocolbase.RegistryUnregisterer); ok {
+		unregisterer.UnregisterRegistries()
+		return
+	}
+
+	logger.Warnf("Graceful shutdown --- Registry protocol %s does not support unregister-only shutdown, falling back to Destroy().", constant.RegistryProtocol)
 	registryProtocol.Destroy()
 }
 
+// notifyLongConnectionConsumers notifies all connected consumers via long connections
+func notifyLongConnectionConsumers(shutdown *global.ShutdownConfig) {
+	logger.Info("Graceful shutdown --- Notify long connection consumers.")
+
+	notifyTimeout := parseDuration(shutdown.NotifyTimeout, notifyTimeoutDesc, defaultNotifyTimeout)
+	callbacks := extension.GracefulShutdownCallbacks()
+	var wg sync.WaitGroup
+	for name, callback := range callbacks {
+		wg.Add(1)
+		go func(name string, callback extension.GracefulShutdownCallback) {
+			defer wg.Done()
+			ctx, cancel := context.WithTimeout(context.Background(), notifyTimeout)
+			defer cancel()
+			notifyWithRetry(ctx, name, callback)
+		}(name, callback)
+	}
+	wg.Wait()
+}
+
+// notifyWithRetry notifies with exponential backoff retry
+func notifyWithRetry(ctx context.Context, name string, callback extension.GracefulShutdownCallback) {
+	backOff := backoff.NewExponentialBackOff()
+	backOff.InitialInterval = defaultRetryBaseDelay
+	backOff.MaxInterval = defaultRetryMaxDelay
+	backOff.MaxElapsedTime = 0
+
+	var attempts int
+	operation := func() error {
+		attempts++
+		err := invokeGracefulShutdownCallback(ctx, name, callback)
+		if err == nil {
+			logger.Infof("Graceful shutdown --- Notify %s completed", name)
+			return nil
+		}
+
+		logger.Warnf("Graceful shutdown --- Notify %s attempt %d failed --- %v", name, attempts, err)
+		return err
+	}
+
+	notify := func(err error, delay time.Duration) {
+		logger.Infof("Graceful shutdown --- Notify %s retrying in %v (attempt %d/%d)", name, delay, attempts, defaultMaxRetries)
+	}
+
+	retryPolicy := backoff.WithContext(backoff.WithMaxRetries(backOff, uint64(defaultMaxRetries)), ctx)
+	if err := backoff.RetryNotify(operation, retryPolicy, notify); err != nil {
+		if ctx.Err() != nil {
+			logger.Warnf("Graceful shutdown --- Notify %s timeout after %d attempts, continuing...", name, attempts)
+			return
+		}
+
+		logger.Warnf("Graceful shutdown --- Notify %s failed after %d attempts --- %v", name, attempts, err)
+	}
+}
+
+func invokeGracefulShutdownCallback(ctx context.Context, name string, callback extension.GracefulShutdownCallback) error {
+	done := make(chan error, 1)
+	go func() {
+		defer func() {
+			if recovered := recover(); recovered != nil {

Review Comment:
   ## shutdown.go:181-198 - invokeGracefulShutdownCallback panic recovery 可能导致死锁
   
   如果 callback(ctx) 执行时 panic,done 不会被发送,调用方会一直等待 ctx.Done()。虽然有 recover,但 panic 会导致 channel 永远阻塞。
   
   **建议**: 在 recover 中确保 done 被发送,或者使用 defer 保证执行。



-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#issuecomment-4150109110

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3235) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [10 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3235&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3235&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3235&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3235&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3235&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3235)
   
   


-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks merged PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235


-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "Alanxtl (via GitHub)" <gi...@apache.org>.
Alanxtl commented on code in PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#discussion_r2902930215


##########
protocol/base/base_invoker.go:
##########
@@ -89,6 +89,11 @@ func (bi *BaseInvoker) IsAvailable() bool {
 	return bi.available.Load()
 }
 
+// SetAvailable sets available flag
+func (bi *BaseInvoker) SetAvailable(available bool) {
+	bi.available.Store(available)
+}

Review Comment:
   这个没必要封装一个函数,之前都是直接store的



##########
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)
+	}
+	return false
+}
+
+// isClosingResponse checks if response contains closing flag
+func (f *consumerGracefulShutdownFilter) isClosingResponse(result result.Result) bool {
+	if result != nil && result.Attachments() != nil {
+		if v, ok := result.Attachments()[constant.GracefulShutdownClosingKey]; ok {
+			if v == "true" {
+				return true
+			}
+		}
+	}
+	return false
+}
+
+// markClosingInvoker marks invoker as closing and sets available=false
+func (f *consumerGracefulShutdownFilter) markClosingInvoker(invoker base.Invoker) {
+	key := invoker.GetURL().String()
+	expireTime := time.Now().Add(f.getClosingInvokerExpireTime())
+	f.closingInvokers.Store(key, expireTime)
+
+	logger.Infof("Graceful shutdown: marked invoker as closing: %s, will expire at %v, IsAvailable=%v",
+		key, expireTime, invoker.IsAvailable())
+
+	if bi, ok := invoker.(*base.BaseInvoker); ok {
+		bi.SetAvailable(false)
+		logger.Infof("Graceful shutdown: set invoker unavailable: %s, IsAvailable now=%v",
+			key, invoker.IsAvailable())
+	}
+}
+
+func (f *consumerGracefulShutdownFilter) getClosingInvokerExpireTime() time.Duration {
+	if f.shutdownConfig != nil && f.shutdownConfig.ClosingInvokerExpireTime != "" {
+		if duration, err := time.ParseDuration(f.shutdownConfig.ClosingInvokerExpireTime); err == nil && duration > 0 {
+			return duration
+		}
+	}
+	// default 30s, also try parsing numeric string as milliseconds
+	if f.shutdownConfig != nil && f.shutdownConfig.ClosingInvokerExpireTime != "" {
+		if ms, err := strconv.ParseInt(f.shutdownConfig.ClosingInvokerExpireTime, 10, 64); err == nil && ms > 0 {
+			return time.Duration(ms) * time.Millisecond
+		}
+	}
+	return 30 * time.Second
+}
+
+// handleRequestError handles request errors and marks invoker as unavailable for connection errors
+func (f *consumerGracefulShutdownFilter) handleRequestError(invoker base.Invoker, err error) {
+	if err == nil {
+		return
+	}
+
+	// check for connection-related errors
+	errMsg := err.Error()
+	isConnectionError := strings.Contains(errMsg, "client has closed") ||
+		strings.Contains(errMsg, "connection") ||
+		strings.Contains(errMsg, "EOF") ||
+		strings.Contains(errMsg, "broken pipe") ||
+		strings.Contains(errMsg, "gRPC") && strings.Contains(errMsg, "closing") ||
+		strings.Contains(errMsg, "http2") && strings.Contains(errMsg, "close")
+
+	if isConnectionError {
+		key := invoker.GetURL().String()
+		expireTime := time.Now().Add(f.getClosingInvokerExpireTime())
+		f.closingInvokers.Store(key, expireTime)
+
+		logger.Infof("Graceful shutdown: connection error detected for invoker: %s, marking as closing, will expire at %v, IsAvailable=%v",

Review Comment:
   其他的都改一下



##########
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)
+	}
+	return false
+}
+
+// isClosingResponse checks if response contains closing flag
+func (f *consumerGracefulShutdownFilter) isClosingResponse(result result.Result) bool {
+	if result != nil && result.Attachments() != nil {
+		if v, ok := result.Attachments()[constant.GracefulShutdownClosingKey]; ok {
+			if v == "true" {
+				return true
+			}
+		}
+	}
+	return false
+}
+
+// markClosingInvoker marks invoker as closing and sets available=false
+func (f *consumerGracefulShutdownFilter) markClosingInvoker(invoker base.Invoker) {
+	key := invoker.GetURL().String()
+	expireTime := time.Now().Add(f.getClosingInvokerExpireTime())
+	f.closingInvokers.Store(key, expireTime)
+
+	logger.Infof("Graceful shutdown: marked invoker as closing: %s, will expire at %v, IsAvailable=%v",
+		key, expireTime, invoker.IsAvailable())
+
+	if bi, ok := invoker.(*base.BaseInvoker); ok {
+		bi.SetAvailable(false)
+		logger.Infof("Graceful shutdown: set invoker unavailable: %s, IsAvailable now=%v",

Review Comment:
   ```suggestion
   		logger.Infof("Graceful shutdown --- Set invoker unavailable: %s, IsAvailable now=%v",
   ```



##########
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.
- */

Review Comment:
   这个注释也别删



##########
graceful_shutdown/shutdown.go:
##########
@@ -42,6 +43,10 @@ const (
 	defaultStepTimeout                 = 3 * time.Second
 	defaultConsumerUpdateWaitTime      = 3 * time.Second
 	defaultOfflineRequestWindowTimeout = 3 * time.Second
+	// retry config

Review Comment:
   ```suggestion
   
   	// retry config
   ```



##########
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:
   1. getter和setter风格的命名不是go的习惯,可以考虑改成
   ```go
   func RegisterGracefulShutdownCallback(name string, f GracefulShutdownCallback)
   func LookupGracefulShutdownCallback(name string) (GracefulShutdownCallback, bool)
   func GracefulShutdownCallbacks() map[string]GracefulShutdownCallback
   ```
   
   2. 并发访问问题需要再考虑一下
   3. 这个SetGracefulShutdownCallback是否允许重复注册,如:
   ```go
   extension.SetGracefulShutdownCallback(GRPC, cb1)
   extension.SetGracefulShutdownCallback(GRPC, cb2)
   ```
   如果不允许需要加一下判断
   
   4. 不要直接把内部 map 返回出去,如果需要的话可以返回一个拷贝



-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "Oxidaner (via GitHub)" <gi...@apache.org>.
Oxidaner commented on code in PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#discussion_r2903025616


##########
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:
   Sorry, I'll correct it



-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#discussion_r2969087457


##########
filter/graceful_shutdown/consumer_filter.go:
##########
@@ -91,3 +129,107 @@ 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)
+		if setter, ok := invoker.(base.AvailabilitySetter); ok {
+			setter.SetAvailable(true)
+			logger.Infof("Graceful shutdown --- Recovered invoker availability after closing TTL --- %s", key)
+		}
+	}
+	return false
+}
+
+// isClosingResponse checks if response contains closing flag
+func (f *consumerGracefulShutdownFilter) isClosingResponse(result result.Result) bool {
+	if result != nil && result.Attachments() != nil {
+		if v, ok := result.Attachments()[constant.GracefulShutdownClosingKey]; ok {
+			if v == "true" {
+				return true
+			}
+		}
+	}
+	return false
+}
+
+// markClosingInvoker marks invoker as closing and sets available=false
+func (f *consumerGracefulShutdownFilter) markClosingInvoker(invoker base.Invoker) {
+	key := invoker.GetURL().String()
+	expireTime := time.Now().Add(f.getClosingInvokerExpireTime())
+	f.closingInvokers.Store(key, expireTime)
+
+	logger.Infof("Graceful shutdown --- Marked invoker as closing --- %s, will expire at %v, IsAvailable=%v",
+		key, expireTime, invoker.IsAvailable())
+
+	if setter, ok := invoker.(base.AvailabilitySetter); ok {
+		setter.SetAvailable(false)
+		logger.Infof("Graceful shutdown --- Set invoker unavailable --- %s, IsAvailable now=%v",
+			key, invoker.IsAvailable())
+	}
+}
+
+func (f *consumerGracefulShutdownFilter) handleClosingEvent(invoker base.Invoker, source string) {
+	if f.closingEventHandler == nil || invoker == nil || invoker.GetURL() == nil {
+		return
+	}
+
+	f.closingEventHandler.HandleClosingEvent(gracefulshutdown.ClosingEvent{
+		Source:      source,
+		InstanceKey: invoker.GetURL().GetCacheInvokerMapKey(),
+		ServiceKey:  invoker.GetURL().ServiceKey(),
+		Address:     invoker.GetURL().Location,
+	})
+}
+
+func (f *consumerGracefulShutdownFilter) getClosingInvokerExpireTime() time.Duration {
+	if f.shutdownConfig != nil && f.shutdownConfig.ClosingInvokerExpireTime != "" {
+		if duration, err := time.ParseDuration(f.shutdownConfig.ClosingInvokerExpireTime); err == nil && duration > 0 {
+			return duration
+		}
+	}
+	return 30 * time.Second
+}
+
+// handleRequestError handles request errors and marks invoker as unavailable for connection errors
+func (f *consumerGracefulShutdownFilter) handleRequestError(invoker base.Invoker, err error) {
+	if err == nil {
+		return
+	}
+
+	if isClosingError(err) {
+		f.markClosingInvoker(invoker)
+		f.handleClosingEvent(invoker, "connection-closing-error")
+	}
+}
+
+func isClosingError(err error) bool {
+	if errors.Is(err, base.ErrClientClosed) || errors.Is(err, base.ErrDestroyedInvoker) {
+		return true
+	}
+

Review Comment:
   ## consumer_filter.go:174-181 - 错误检测依赖字符串匹配过于脆弱
   
   strings.Contains(errMsg, ...) 使用字符串匹配检测关闭错误是脆弱的,不同 gRPC 版本可能返回不同的错误信息。
   
   **建议**: 考虑使用 google.golang.org/grpc/status 的 FromError() 解析 error,然后检查 status.Code() 是否为 Unavailable 或 Canceled。



-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "Oxidaner (via GitHub)" <gi...@apache.org>.
Oxidaner commented on code in PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#discussion_r2969108953


##########
protocol/grpc/grpc_protocol.go:
##########
@@ -38,10 +41,36 @@ const (
 
 func init() {
 	extension.SetProtocol(GRPC, GetProtocol)
+
+	// register graceful shutdown callback
+	extension.RegisterGracefulShutdownCallback(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.SetAllServicesNotServing()
+		}
+
+		return nil
+	})
 }
 
 var grpcProtocol *GrpcProtocol
 
+var grpcServerGracefulStop = func(server *Server) {
+	server.GracefulStop()
+}
+
 // GrpcProtocol is gRPC protocol

Review Comment:
   yes, Some are in the grpc_protocol_test.go 



-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#issuecomment-4097683757

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3235) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3235&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3235&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3235&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3235&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3235&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3235)
   
   


-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#issuecomment-4097816352

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3235) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [10 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3235&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3235&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3235&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3235&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3235&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3235)
   
   


-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "robocanic (via GitHub)" <gi...@apache.org>.
robocanic commented on PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#issuecomment-4062938696

   @sirui


-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#issuecomment-4019010947

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3235) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3235&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3235&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3235&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3235&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3235&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3235)
   
   


-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "Oxidaner (via GitHub)" <gi...@apache.org>.
Oxidaner commented on code in PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#discussion_r2901795201


##########
config/graceful_shutdown.go:
##########
@@ -99,14 +98,8 @@ func gracefulShutdownInit() {
 // BeforeShutdown provides processing flow before shutdown
 func BeforeShutdown() {
 	destroyAllRegistries()
-	// waiting for a short time so that the clients have enough time to get the notification that server shutdowns
-	// The value of configuration depends on how long the clients will get notification.
 	waitAndAcceptNewRequests()
-
-	// reject sending/receiving the new request, but keeping waiting for accepting requests
 	waitForSendingAndReceivingRequests()
-
-	// destroy all protocols

Review Comment:
   Sorry, I will restore annotations



-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown [dubbo-go]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#issuecomment-4018451154

   ## [Codecov](https://app.codecov.io/gh/apache/dubbo-go/pull/3235?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :x: Patch coverage is `3.52113%` with `137 lines` in your changes missing coverage. Please review.
   :white_check_mark: Project coverage is 47.74%. Comparing base ([`60d1c2a`](https://app.codecov.io/gh/apache/dubbo-go/commit/60d1c2a949f0ee0da4be3fe09fb79d295491e040?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`da642c7`](https://app.codecov.io/gh/apache/dubbo-go/commit/da642c72e4702daa15aa3649367912f71f2dd0c7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 750 commits behind head on develop.
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/dubbo-go/pull/3235?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [filter/graceful\_shutdown/consumer\_filter.go](https://app.codecov.io/gh/apache/dubbo-go/pull/3235?src=pr&el=tree&filepath=filter%2Fgraceful_shutdown%2Fconsumer_filter.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZmlsdGVyL2dyYWNlZnVsX3NodXRkb3duL2NvbnN1bWVyX2ZpbHRlci5nbw==) | 5.17% | [53 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/dubbo-go/pull/3235?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [graceful\_shutdown/shutdown.go](https://app.codecov.io/gh/apache/dubbo-go/pull/3235?src=pr&el=tree&filepath=graceful_shutdown%2Fshutdown.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhY2VmdWxfc2h1dGRvd24vc2h1dGRvd24uZ28=) | 0.00% | [42 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo-go/pull/3235?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [protocol/triple/triple.go](https://app.codecov.io/gh/apache/dubbo-go/pull/3235?src=pr&el=tree&filepath=protocol%2Ftriple%2Ftriple.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHJvdG9jb2wvdHJpcGxlL3RyaXBsZS5nbw==) | 0.00% | [12 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/dubbo-go/pull/3235?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [protocol/grpc/grpc\_protocol.go](https://app.codecov.io/gh/apache/dubbo-go/pull/3235?src=pr&el=tree&filepath=protocol%2Fgrpc%2Fgrpc_protocol.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHJvdG9jb2wvZ3JwYy9ncnBjX3Byb3RvY29sLmdv) | 0.00% | [11 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/dubbo-go/pull/3235?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [common/extension/graceful\_shutdown.go](https://app.codecov.io/gh/apache/dubbo-go/pull/3235?src=pr&el=tree&filepath=common%2Fextension%2Fgraceful_shutdown.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29tbW9uL2V4dGVuc2lvbi9ncmFjZWZ1bF9zaHV0ZG93bi5nbw==) | 0.00% | [7 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo-go/pull/3235?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [filter/graceful\_shutdown/provider\_filter.go](https://app.codecov.io/gh/apache/dubbo-go/pull/3235?src=pr&el=tree&filepath=filter%2Fgraceful_shutdown%2Fprovider_filter.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZmlsdGVyL2dyYWNlZnVsX3NodXRkb3duL3Byb3ZpZGVyX2ZpbHRlci5nbw==) | 0.00% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo-go/pull/3235?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [protocol/base/base\_invoker.go](https://app.codecov.io/gh/apache/dubbo-go/pull/3235?src=pr&el=tree&filepath=protocol%2Fbase%2Fbase_invoker.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHJvdG9jb2wvYmFzZS9iYXNlX2ludm9rZXIuZ28=) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo-go/pull/3235?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@             Coverage Diff             @@
   ##           develop    #3235      +/-   ##
   ===========================================
   + Coverage    46.76%   47.74%   +0.97%     
   ===========================================
     Files          295      463     +168     
     Lines        17172    33979   +16807     
   ===========================================
   + Hits          8031    16223    +8192     
   - Misses        8287    16439    +8152     
   - Partials       854     1317     +463     
   ```
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/dubbo-go/pull/3235?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   </details>


-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] Feat/graceful shutdown [dubbo-go]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#issuecomment-4018444080

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3235) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3235&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3235&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3235&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3235&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3235&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3235)
   
   


-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#issuecomment-4102655531

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3235) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [10 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3235&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3235&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3235&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3235&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3235&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3235)
   
   


-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#discussion_r2969086523


##########
protocol/grpc/grpc_protocol.go:
##########
@@ -38,10 +41,36 @@ const (
 
 func init() {
 	extension.SetProtocol(GRPC, GetProtocol)
+
+	// register graceful shutdown callback
+	extension.RegisterGracefulShutdownCallback(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.SetAllServicesNotServing()
+		}
+
+		return nil
+	})
 }
 
 var grpcProtocol *GrpcProtocol
 
+var grpcServerGracefulStop = func(server *Server) {
+	server.GracefulStop()
+}
+
 // GrpcProtocol is gRPC protocol

Review Comment:
   ## grpc_protocol.go:49 - 未使用的变量
   
   `grpcServerGracefulStop` 定义后从未直接使用(实际调用的是 `drainServers()` 返回的 server 的 `GracefulStop()` 方法)。设计意图是方便测试时 mock,但如果测试没有用到,应删除。
   
   **建议**: 确认是否有测试用到,如果没有则删除。



-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#discussion_r2969088005


##########
go.mod:
##########
@@ -10,7 +10,6 @@ require (
 	github.com/apache/dubbo-getty v1.4.10
 	github.com/apache/dubbo-go-hessian2 v1.12.5

Review Comment:
   ## go.mod - 依赖变更需要确认
   
   cenkalti/backoff/v4 从 require 段移动到了 indirect。这可能意味着这个依赖现在是通过其他模块间接引入的,而不是直接被新代码导入。
   
   **建议**: 确认这个依赖确实只是间接依赖,或者将新代码中对 backoff 的导入声明为直接依赖。



##########
protocol/triple/triple.go:
##########
@@ -139,14 +167,23 @@ func (tp *TripleProtocol) Refer(url *common.URL) base.Invoker {
 func (tp *TripleProtocol) Destroy() {
 	logger.Infof("TripleProtocol destroy.")
 
+	for _, server := range tp.drainServers() {
+		tripleServerGracefulStop(server)
+	}
+
+	tp.BaseProtocol.Destroy()
+}
+
+func (tp *TripleProtocol) drainServers() []*Server {
 	tp.serverLock.Lock()
 	defer tp.serverLock.Unlock()
+

Review Comment:
   ## Triple 协议支持缺失
   
   从 diff 看,gRPC 协议实现了 GracefulShutdownCallback 并使用 health check 进行关闭通知,但 Triple 协议没有相应的实现。这可能导致在使用 Triple 协议时优雅关闭功能不完整。
   
   **建议**: 确认 Triple 协议是否需要相同的实现,或在文档中说明只支持 gRPC。



-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#discussion_r2969089068


##########
global/shutdown_config.go:
##########
@@ -41,6 +41,13 @@ type ShutdownConfig struct {
 	 */
 	StepTimeout string `default:"3s" yaml:"step-timeout" json:"step.timeout,omitempty" property:"step.timeout"`
 
+	/*
+	 * NotifyTimeout means the timeout budget for actively notifying long-connection consumers
+	 * during graceful shutdown. It only controls the notify step and should not be coupled to
+	 * request draining timeouts.
+	 */
+	NotifyTimeout string `default:"5s" yaml:"notify-timeout" json:"notify.timeout,omitempty" property:"notify.timeout"`
+

Review Comment:
   ## shutdown_config.go:174 - ClosingInvokerExpireTime 缺少文档注释
   
   ClosingInvokerExpireTime 没有注释说明其用途,与同文件中其他字段的文档风格不一致。
   
   **建议**: 添加类似其他字段的注释,说明这个配置的作用(如:consumer 端标记 invoker 为 closing 后的过期时间)。



-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "Oxidaner (via GitHub)" <gi...@apache.org>.
Oxidaner commented on code in PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#discussion_r2969114079


##########
filter/graceful_shutdown/consumer_filter.go:
##########
@@ -50,27 +54,61 @@ func init() {
 }
 
 type consumerGracefulShutdownFilter struct {
-	shutdownConfig *global.ShutdownConfig
+	shutdownConfig      *global.ShutdownConfig
+	closingEventHandler gracefulshutdown.ClosingEventHandler
+	closingInvokers     sync.Map // map[string]time.Time (url key -> expire time)
 }
 
+const consumerCountMarkedKey = "dubbo-go-graceful-shutdown-consumer-counted"
+
 func newConsumerGracefulShutdownFilter() filter.Filter {
 	if csf == nil {
 		csfOnce.Do(func() {

Review Comment:
   这些是常量是包内私有,单文件内使用,确定要上提吗
   



-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#issuecomment-4106709552

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3235) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [10 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3235&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3235&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3235&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3235&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3235&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3235)
   
   


-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#issuecomment-4097768179

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3235) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [10 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3235&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3235&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3235&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3235&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3235&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3235)
   
   


-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "Oxidaner (via GitHub)" <gi...@apache.org>.
Oxidaner commented on code in PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#discussion_r2903025123


##########
protocol/triple/triple.go:
##########
@@ -47,6 +47,30 @@ var (
 
 func init() {
 	extension.SetProtocol(TRIPLE, GetProtocol)
+
+	// register graceful shutdown callback
+	extension.SetGracefulShutdownCallback(TRIPLE, func(ctx context.Context) error {
+		tp := GetProtocol()
+		if tp == nil {
+			return nil
+		}
+
+		var ok bool
+		tripleProtocol = tp.(*TripleProtocol)
+		if !ok {

Review Comment:
   Sorry, I'll correct it



-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


Re: [PR] feat: graceful shutdown #1977 [dubbo-go]

Posted by "Alanxtl (via GitHub)" <gi...@apache.org>.
Alanxtl commented on code in PR #3235:
URL: https://github.com/apache/dubbo-go/pull/3235#discussion_r2901558551


##########
config/graceful_shutdown.go:
##########
@@ -99,14 +98,8 @@ func gracefulShutdownInit() {
 // BeforeShutdown provides processing flow before shutdown
 func BeforeShutdown() {
 	destroyAllRegistries()
-	// waiting for a short time so that the clients have enough time to get the notification that server shutdowns
-	// The value of configuration depends on how long the clients will get notification.
 	waitAndAcceptNewRequests()
-
-	// reject sending/receiving the new request, but keeping waiting for accepting requests
 	waitForSendingAndReceivingRequests()
-
-	// destroy all protocols

Review Comment:
   do not delete these comments



-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org