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: Graceful shutdown bugs(supplement #1254) #1257

Merged
merged 10 commits into from
Jun 24, 2021

Conversation

justxuewei
Copy link
Member

@justxuewei justxuewei commented Jun 13, 2021

What this PR does:

  • Fix a bug that RejectRequest is not working
  • Fix a bug that shutdownConfig always be nil, because init() is called before config.Load(). The changes are listed at below:
    • To avoid the import circle that happens when filter_impl package is imported in config package, a new interface, called config.Setter, is introduced.
    • gracefulShutdownFilters for both provider and consumer are initialized in init() as usual, they, however, are not to get shutdownConfig immediately, instead, the shutdownConfig is set to filters in config.GracefulShutdownInit() via config.Setter.

see also #1254

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

@justxuewei justxuewei changed the title Fix: Graceful Shutdown bugs(supplement #1254) Fix: Graceful shutdown bugs(supplement #1254) Jun 13, 2021
remove unused comments

fix import cycle

append apache license header

fix gracefulShutdownFilter unittest bug

go fmt

fix gracefulShutdownConfig unittest bug

fix gracefulShutdownConfig unittest bug

go fmt
@codecov-commenter
Copy link

codecov-commenter commented Jun 13, 2021

Codecov Report

Merging #1257 (f12379a) into 3.0 (968650f) will decrease coverage by 3.65%.
The diff coverage is 50.78%.

Impacted file tree graph

@@            Coverage Diff             @@
##              3.0    #1257      +/-   ##
==========================================
- Coverage   59.53%   55.87%   -3.66%     
==========================================
  Files         259      272      +13     
  Lines       12737    12815      +78     
==========================================
- Hits         7583     7161     -422     
- Misses       4199     4760     +561     
+ Partials      955      894      -61     
Impacted Files Coverage Δ
cluster/cluster_impl/available_cluster_invoker.go 66.66% <ø> (ø)
cluster/cluster_impl/broadcast_cluster_invoker.go 76.47% <ø> (ø)
cluster/cluster_impl/failfast_cluster_invoker.go 66.66% <ø> (ø)
cluster/cluster_impl/mock_cluster.go 0.00% <0.00%> (ø)
cluster/loadbalance/random.go 100.00% <ø> (ø)
...router/v3router/judger/list_string_match_judger.go 0.00% <0.00%> (ø)
...ster/router/v3router/judger/method_match_judger.go 0.00% <0.00%> (ø)
...er/router/v3router/judger/url_label_match_judge.go 0.00% <0.00%> (ø)
common/extension/auth.go 0.00% <ø> (ø)
common/extension/cluster.go 0.00% <ø> (ø)
... and 284 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 99bcbee...f12379a. Read the comment docs.

@justxuewei justxuewei force-pushed the fix/graceful-shutdown-bug-2 branch 2 times, most recently from ef69b41 to 93809ea Compare June 14, 2021 12:15
config/graceful_shutdown_config.go Outdated Show resolved Hide resolved
filter/filter_impl/graceful_shutdown_filter.go Outdated Show resolved Hide resolved
@LaurenceLiZhixin LaurenceLiZhixin added this to the v3.0.0-rc2 milestone Jun 16, 2021
@justxuewei justxuewei marked this pull request as draft June 16, 2021 02:27
@justxuewei
Copy link
Member Author

justxuewei commented Jun 16, 2021

Currently, it may have performance issue. So, this pr is converted to draft temporarily.

@LaurenceLiZhixin LaurenceLiZhixin removed this from the v3.0.0-rc2 milestone Jun 16, 2021
@justxuewei justxuewei marked this pull request as ready for review June 19, 2021 14:58
@LaurenceLiZhixin LaurenceLiZhixin added this to the v3.0.0 milestone Jun 20, 2021
@AlexStocks AlexStocks merged commit 0f0e1f6 into apache:3.0 Jun 24, 2021
GracefulShutdownFilterShutdownConfig = "GracefulShutdownFilterShutdownConfig"
)

type Setter interface {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe it is bertter to add some comment for Setting

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, some comments will be added lately.

mark4z pushed a commit to mark4z/dubbo-go that referenced this pull request Jul 5, 2021
* supplementary fix apache#1254

remove unused comments

fix import cycle

append apache license header

fix gracefulShutdownFilter unittest bug

go fmt

fix gracefulShutdownConfig unittest bug

fix gracefulShutdownConfig unittest bug

go fmt

* improve formatting based on code style

* go fmt

* set RequestsFinished explicitly

* use mutex to protect variables of ShutdownConfig

* ftr: add config (apache#1258)

* recover gracefulShutdownFilter logic

* remove unused mutex

Co-authored-by: Laurence <45508533+LaurenceLiZhixin@users.noreply.github.com>
AlexStocks added a commit that referenced this pull request Jul 5, 2021
…1300)

* build(deps): bump actions/cache from v2.1.4 to v2.1.5

Bumps [actions/cache](https://github.com/actions/cache) from v2.1.4 to v2.1.5.
- [Release notes](https://github.com/actions/cache/releases)
- [Commits](actions/cache@v2.1.4...1a9e213)

Signed-off-by: dependabot[bot] <support@github.com>

* improve etcd version and change create to put (#1203)

* make the package v3router/judger test coverage rate reach 80% (#1260)

* make the package v3router/judger test coverage rate reach 80%

* add router_chain unit test

* refactor imports and some code

* remove blank lines

Co-authored-by: dongjianhui <dongjianhui@yuanfudao.com>

* Fix: Graceful shutdown bugs(supplement #1254) (#1257)

* supplementary fix #1254

remove unused comments

fix import cycle

append apache license header

fix gracefulShutdownFilter unittest bug

go fmt

fix gracefulShutdownConfig unittest bug

fix gracefulShutdownConfig unittest bug

go fmt

* improve formatting based on code style

* go fmt

* set RequestsFinished explicitly

* use mutex to protect variables of ShutdownConfig

* ftr: add config (#1258)

* recover gracefulShutdownFilter logic

* remove unused mutex

Co-authored-by: Laurence <45508533+LaurenceLiZhixin@users.noreply.github.com>

* refine grpc test code (#1266)

* refine grpc test code

* fix test

* remove useless code

* config test grpc server

* registry 默认值问题 (#1275)

Co-authored-by: wangxiaowei14227 <wangxiaowei14227@autohome.com.cn>

* config center for more parameters (#1277)

* nacos config center optimize

* up remote config

* add protocol chick

* up:代码优化

* go fmt

* fix: add arch picture in readme and delete unused router field. (#1279)

* fix

* fix: delete notify

* performance optimization: change time.After => time.NewTimer

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Xin.Zh <dragoncharlie@foxmail.com>
Co-authored-by: AlexStocks <alexstocks@foxmail.com>
Co-authored-by: randy <ztelur@gmail.com>
Co-authored-by: LaurenceLiZhixin <382673304@qq.com>
Co-authored-by: Mulavar <978007503@qq.com>
Co-authored-by: dongjianhui <dongjianhui@yuanfudao.com>
Co-authored-by: XavierNiu <a@nxw.name>
Co-authored-by: Laurence <45508533+LaurenceLiZhixin@users.noreply.github.com>
Co-authored-by: gaoxinge <xg.gao@tianrang-inc.com>
Co-authored-by: wangxw666 <2484713618@qq.com>
Co-authored-by: wangxiaowei14227 <wangxiaowei14227@autohome.com.cn>
Co-authored-by: 赵云兴 <2385585770@qq.com>
@justxuewei justxuewei deleted the fix/graceful-shutdown-bug-2 branch January 27, 2022 04:34
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.

7 participants