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

Fix: consul destory #788

Merged
merged 8 commits into from
Oct 13, 2020
Merged

Conversation

LaurenceLiZhixin
Copy link
Contributor

What this PR does:
修复consul Registry 在interrupt后不会立刻从注册中心消失的bug

Which issue(s) this PR fixes:
Fixes #749

Special notes for your reviewer:
修复consul Registry 在interrupt后不会立刻从注册中心消失的bug,在consulRegistry的Destory函数中增加对Unregister函数的调用。从而从consul反注册。增加超时unregister超时的处理。

Does this PR introduce a user-facing change?:
NONE

@AlexStocks AlexStocks changed the title Fix/consul destory Fix: consul destory Oct 12, 2020
select {
case <-done:
logger.Infof("consulRegistry unregister done")
case <-ticker.C:
Copy link
Contributor

Choose a reason for hiding this comment

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

a time.After is enough in this simple case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I fixed it.

@codecov-io
Copy link

codecov-io commented Oct 13, 2020

Codecov Report

Merging #788 into develop will decrease coverage by 3.87%.
The diff coverage is 52.74%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #788      +/-   ##
===========================================
- Coverage    63.75%   59.87%   -3.88%     
===========================================
  Files          239      259      +20     
  Lines        12796    12753      -43     
===========================================
- Hits          8158     7636     -522     
- Misses        3849     4163     +314     
- Partials       789      954     +165     
Impacted Files Coverage Δ
cluster/router/condition/listenable_router.go 54.71% <0.00%> (-0.13%) ⬇️
cluster/router/tag/file.go 76.19% <0.00%> (-0.74%) ⬇️
common/extension/health_checker.go 50.00% <ø> (-16.67%) ⬇️
common/proxy/proxy_factory/default.go 13.33% <0.00%> (-7.31%) ⬇️
common/rpc_service.go 87.94% <ø> (+1.35%) ⬆️
config/consumer_config.go 51.21% <ø> (-5.04%) ⬇️
config/provider_config.go 50.00% <ø> (-8.07%) ⬇️
config/reference_config.go 80.00% <ø> (+0.17%) ⬆️
config/remote_config.go 75.00% <ø> (+1.31%) ⬆️
config_center/apollo/impl.go 83.63% <ø> (+2.81%) ⬆️
... and 324 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 811f4f1...cdf7405. Read the comment docs.

@cityiron
Copy link
Contributor

LGTM

Copy link
Contributor

@hxmhlt hxmhlt left a comment

Choose a reason for hiding this comment

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

LGTM

@hxmhlt hxmhlt merged commit ab44015 into apache:develop Oct 13, 2020
@zouyx zouyx added this to the v1.5.3 milestone Oct 15, 2020
@zouyx zouyx modified the milestones: v1.5.5, v1.5.4 Oct 30, 2020
zouyx pushed a commit to zouyx/dubbo-go that referenced this pull request Oct 31, 2020
AlexStocks pushed a commit that referenced this pull request Apr 14, 2021
AlexStocks pushed a commit that referenced this pull request Apr 14, 2021
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