refactor: migrate graceful shutdown config to global#3386
Open
Oxidaner wants to merge 5 commits into
Open
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3386 +/- ##
===========================================
+ Coverage 46.76% 52.76% +5.99%
===========================================
Files 295 492 +197
Lines 17172 37926 +20754
===========================================
+ Hits 8031 20010 +11979
- Misses 8287 16294 +8007
- Partials 854 1622 +768 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Alanxtl
reviewed
Jun 9, 2026
Comment on lines
-474
to
-486
| cfg := &config.ShutdownConfig{ | ||
| Timeout: c.Timeout, | ||
| StepTimeout: c.StepTimeout, | ||
| NotifyTimeout: c.NotifyTimeout, | ||
| ConsumerUpdateWaitTime: c.ConsumerUpdateWaitTime, | ||
| RejectRequestHandler: c.RejectRequestHandler, | ||
| InternalSignal: c.InternalSignal, | ||
| OfflineRequestWindowTimeout: c.OfflineRequestWindowTimeout, | ||
| RejectRequest: atomic.Bool{}, | ||
| } | ||
| cfg.RejectRequest.Store(c.RejectRequest.Load()) | ||
|
|
||
| return cfg |
Comment on lines
-44
to
-154
| // ShutdownConfig is used as configuration for graceful shutdown | ||
| type ShutdownConfig struct { | ||
| /* | ||
| * Total timeout. Even though we don't release all resources, | ||
| * the applicationConfig will shutdown if the costing time is over this configuration. The unit is ms. | ||
| * default value is 60 * 1000 ms = 1 minutes | ||
| * In general, it should be bigger than 3 * StepTimeout. | ||
| */ | ||
| Timeout string `default:"60s" yaml:"timeout" json:"timeout,omitempty" property:"timeout"` | ||
| /* | ||
| * the timeout on each step. You should evaluate the response time of request | ||
| * and the time that client noticed that server shutdown. | ||
| * For example, if your client will received the notification within 10s when you start to close server, | ||
| * and the 99.9% requests will return response in 2s, so the StepTimeout will be bigger than(10+2) * 1000ms, | ||
| * maybe (10 + 2*3) * 1000ms is a good choice. | ||
| */ | ||
| 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"` | ||
|
|
||
| /* | ||
| * ConsumerUpdateWaitTime means when provider is shutting down, after the unregister, time to wait for client to | ||
| * update invokers. During this time, incoming invocation can be treated normally. | ||
| */ | ||
| ConsumerUpdateWaitTime string `default:"3s" yaml:"consumer-update-wait-time" json:"consumerUpdate.waitTIme,omitempty" property:"consumerUpdate.waitTIme"` | ||
| // when we try to shutdown the applicationConfig, we will reject the new requests. In most cases, you don't need to configure this. | ||
| RejectRequestHandler string `yaml:"reject-handler" json:"reject-handler,omitempty" property:"reject_handler"` | ||
| // internal listen kill signal，the default is true. | ||
| InternalSignal *bool `default:"true" yaml:"internal-signal" json:"internal.signal,omitempty" property:"internal.signal"` | ||
| // offline request window length | ||
| OfflineRequestWindowTimeout string `default:"3s" yaml:"offline-request-window-timeout" json:"offlineRequestWindowTimeout,omitempty" property:"offlineRequestWindowTimeout"` | ||
| // true -> new request will be rejected. | ||
| RejectRequest atomic.Bool | ||
| // active invocation | ||
| ConsumerActiveCount atomic.Int32 | ||
| ProviderActiveCount atomic.Int32 | ||
|
|
||
| // provider last received request timestamp | ||
| ProviderLastReceivedRequestTime atomic.Time | ||
| } | ||
|
|
||
| // Prefix dubbo.shutdown | ||
| func (config *ShutdownConfig) Prefix() string { | ||
| return constant.ShutdownConfigPrefix | ||
| } | ||
|
|
||
| func (config *ShutdownConfig) GetTimeout() time.Duration { | ||
| result, err := time.ParseDuration(config.Timeout) | ||
| if err != nil { | ||
| logger.Errorf("The Timeout configuration is invalid: %s, and we will use the default value: %s, err: %v", | ||
| config.Timeout, defaultTimeout.String(), err) | ||
| return defaultTimeout | ||
| } | ||
| return result | ||
| } | ||
|
|
||
| func (config *ShutdownConfig) GetStepTimeout() time.Duration { | ||
| result, err := time.ParseDuration(config.StepTimeout) | ||
| if err != nil { | ||
| logger.Errorf("The StepTimeout configuration is invalid: %s, and we will use the default value: %s, err: %v", | ||
| config.StepTimeout, defaultStepTimeout.String(), err) | ||
| return defaultStepTimeout | ||
| } | ||
| return result | ||
| } | ||
|
|
||
| func (config *ShutdownConfig) GetNotifyTimeout() time.Duration { | ||
| result, err := time.ParseDuration(config.NotifyTimeout) | ||
| if err != nil { | ||
| logger.Errorf("The NotifyTimeout configuration is invalid: %s, and we will use the default value: %s, err: %v", | ||
| config.NotifyTimeout, defaultNotifyTimeout.String(), err) | ||
| return defaultNotifyTimeout | ||
| } | ||
| return result | ||
| } | ||
|
|
||
| func (config *ShutdownConfig) GetOfflineRequestWindowTimeout() time.Duration { | ||
| result, err := time.ParseDuration(config.OfflineRequestWindowTimeout) | ||
| if err != nil { | ||
| logger.Errorf("The OfflineRequestWindowTimeout configuration is invalid: %s, and we will use the default value: %s, err: %v", | ||
| config.OfflineRequestWindowTimeout, defaultOfflineRequestWindowTimeout.String(), err) | ||
| return defaultOfflineRequestWindowTimeout | ||
| } | ||
| return result | ||
| } | ||
|
|
||
| func (config *ShutdownConfig) GetConsumerUpdateWaitTime() time.Duration { | ||
| result, err := time.ParseDuration(config.ConsumerUpdateWaitTime) | ||
| if err != nil { | ||
| logger.Errorf("The ConsumerUpdateTimeout configuration is invalid: %s, and we will use the default value: %s, err: %v", | ||
| config.ConsumerActiveCount.Load(), defaultConsumerUpdateWaitTime.String(), err) | ||
| return defaultConsumerUpdateWaitTime | ||
| } | ||
| return result | ||
| } | ||
|
|
||
| func (config *ShutdownConfig) GetInternalSignal() bool { | ||
| if config.InternalSignal == nil { | ||
| return false | ||
| } | ||
| return *config.InternalSignal | ||
| } | ||
|
|
||
| func (config *ShutdownConfig) Init() error { | ||
| return defaults.Set(config) | ||
| } |
Member
|
shutdown 默认时长现在仍有两份：global/shutdown_config.go#L36 和 graceful_shutdown/shutdown.go#L45。 这块顺便处理一下吧 都统一到 common/const |
Contributor
Author
|
@Alanxtl done |
Member
|
#3386 (comment) |
AlexStocks
approved these changes
Jun 10, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors graceful shutdown configuration to be owned by global.ShutdownConfig, continuing the effort in #3204 to remove/retire the config package while keeping compatibility.
Changes:
- Move graceful shutdown config behavior (defaults + duration parsing helpers) to
global.ShutdownConfigand centralize default duration values undercommon/constant. - Remove legacy
config.ShutdownConfigcompatibility handling from graceful shutdown filters and updategraceful_shutdowninit wiring to use a local setter interface. - Update/extend tests to assert the new defaults/constants and that filters receive
*global.ShutdownConfig.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| graceful_shutdown/shutdown.go | Drops config dependency, uses common/constant defaults, and uses a local setter interface for passing shutdown config into filters. |
| graceful_shutdown/shutdown_test.go | Updates expectations to use new default constants. |
| global/shutdown_config.go | Adds prefix + duration parsing helpers and Init()/Clone() for graceful shutdown config in global. |
| filter/graceful_shutdown/provider_filter.go | Removes compatibility support for *config.ShutdownConfig and only accepts *global.ShutdownConfig. |
| filter/graceful_shutdown/consumer_filter.go | Removes compatibility support for *config.ShutdownConfig and only accepts *global.ShutdownConfig. |
| config/graceful_shutdown_test.go | Adds a regression test ensuring config-side init passes *global.ShutdownConfig to filters. |
| config/graceful_shutdown_config.go | Replaces config.ShutdownConfig with a compatibility type-alias to global.ShutdownConfig. |
| config/graceful_shutdown_config_test.go | Updates default expectations after the type-alias refactor. |
| compat.go | Simplifies shutdown compat conversion to Clone() (now that config type is an alias). |
| common/constant/shutdown.go | Introduces centralized default shutdown durations in the constant package. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+55
to
+73
| func TestGracefulShutdownInitPassesGlobalShutdownConfigToFilters(t *testing.T) { | ||
| internalSignal := false | ||
| SetRootConfig(RootConfig{ | ||
| Shutdown: &ShutdownConfig{ | ||
| Timeout: "11s", | ||
| StepTimeout: "2s", | ||
| NotifyTimeout: "4s", | ||
| InternalSignal: &internalSignal, | ||
| }, | ||
| }) | ||
|
|
||
| consumerFilter := &captureShutdownConfigFilter{} | ||
| providerFilter := &captureShutdownConfigFilter{} | ||
| extension.SetFilter(constant.GracefulShutdownConsumerFilterKey, func() filter.Filter { | ||
| return consumerFilter | ||
| }) | ||
| extension.SetFilter(constant.GracefulShutdownProviderFilterKey, func() filter.Filter { | ||
| return providerFilter | ||
| }) |
Oxidaner
force-pushed
the
migrate-graceful-shutdown-config-global
branch
from
63b0695 to
a565a0d
Compare
…g does not support HTTP/1
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#3204