Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature-graceful-shutdwon #1675

Merged
merged 17 commits into from
Jan 17, 2022
Merged

Conversation

XiaoWeiKIN
Copy link
Contributor

@XiaoWeiKIN XiaoWeiKIN commented Dec 19, 2021

Fix #1664
of 3.0.1 milestone #1685

@XiaoWeiKIN
Copy link
Contributor Author

XiaoWeiKIN commented Dec 22, 2021

todo:

  1. config支持shutdwon配置获取(完成)
  2. kill型号可配置处理(完成)
  3. 补充单元测试(完成)
  4. 完善文档

@LaurenceLiZhixin
Copy link
Contributor

run
$ import-formatter .
in project root

@XiaoWeiKIN XiaoWeiKIN changed the title [WIP] feature-graceful-shutdwon feature-graceful-shutdwon Jan 5, 2022
@LaurenceLiZhixin
Copy link
Contributor

license, imports block 问题已经解决,ut问题需要再看下,位置 registry/protocol/protocol_test.go:141
@XiaoWeiKIN

wangxiaowei14227 added 2 commits January 5, 2022 17:28
@LaurenceLiZhixin LaurenceLiZhixin added this to the 3.0.1 milestone Jan 7, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jan 15, 2022

Codecov Report

Merging #1675 (39916bb) into 3.0 (0f93d1c) will increase coverage by 0.08%.
The diff coverage is 39.80%.

Impacted file tree graph

@@            Coverage Diff             @@
##              3.0    #1675      +/-   ##
==========================================
+ Coverage   41.31%   41.39%   +0.08%     
==========================================
  Files         255      256       +1     
  Lines       14705    14778      +73     
==========================================
+ Hits         6076     6118      +42     
- Misses       7930     7960      +30     
- Partials      699      700       +1     
Impacted Files Coverage Δ
cluster/cluster/zoneaware/cluster_interceptor.go 40.00% <0.00%> (ø)
common/extension/filter.go 0.00% <0.00%> (ø)
common/proxy/proxy_factory/pass_through.go 31.91% <0.00%> (ø)
config/service_config.go 10.75% <0.00%> (-0.04%) ⬇️
protocol/dubbo/dubbo_codec.go 1.87% <0.00%> (ø)
protocol/dubbo/dubbo_invoker.go 0.00% <0.00%> (ø)
protocol/result.go 0.00% <ø> (ø)
registry/base_registry.go 1.42% <0.00%> (+0.15%) ⬆️
config/graceful_shutdown.go 4.00% <8.16%> (+4.00%) ⬆️
config/root_config.go 44.38% <15.38%> (-2.07%) ⬇️
... and 23 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f271fc1...39916bb. Read the comment docs.

@LaurenceLiZhixin
Copy link
Contributor

我在调试完善,先不要合并

@LaurenceLiZhixin LaurenceLiZhixin merged commit 7168e8e into apache:3.0 Jan 17, 2022
@@ -33,11 +33,11 @@ func SetFilter(name string, v func() filter.Filter) {
}

// GetFilter finds the filter extension with @name
func GetFilter(name string) filter.Filter {
func GetFilter(name string) (filter.Filter, bool) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

原来返回一个参数感觉更舒服,panic去掉就可以了

timeout := shutdownConfig.GetStepTimeout()
if timeout <= 0 {
return
}
deadline := time.Now().Add(timeout)

for time.Now().Before(deadline) && !shutdownConfig.RequestsFinished {
for time.Now().Before(deadline) && shutdownConfig.ConsumerActiveCount > 0 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shutdownConfig.ConsumerActiveCount 需要用atomic.Load吧

@@ -124,6 +124,16 @@ func GetApplicationConfig() *ApplicationConfig {
return rootConfig.Application
}

func GetShutDown() *ShutdownConfig {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这样写也许更整洁点
if err := check(); err == nil && rootConfig.Shutdown != nil {
return rootConfig.Shutdown
}
return NewShutDownConfigBuilder().Build()

}
deadline := time.Now().Add(timeout)

for time.Now().Before(deadline) && shutdownConfig.ProviderActiveCount > 0 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

atomic.Load?

reg, err = extension.GetRegistry(registryUrl.Protocol, registryUrl)
if err != nil {
logger.Errorf("Registry can not connect success, program is going to panic.Error message is %s", err.Error())
panic(err.Error())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

panic 直接panic(err) 就可以吧

This was referenced Jan 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants