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

Support gracefully offline without registry #1973

Merged

Conversation

TheR1sing3un
Copy link
Member

What this PR does:
image-20220607195015283
这一块是不基于注册中心的优雅下线的第一个小功能点。原本的逻辑是以10ms为间隔轮询counter,在等待未超时的情况下,一旦发现counter为0,就结束该步骤了。那么这样可能会有问题,在那一轮询counter的一瞬间,当前的请求刚好都返回,但是在这一瞬间之前一直都有请求过来,在这瞬间之后也仍有请求继续。那么这时候结束该步骤是不妥的,因此可以维护一个滑动窗口,假设窗口期为x ms,在x ms之内都没有新的请求到来,那么这时候就可以认为该步骤可以结束了。
算法思路就是,在counter计数的基础上,维护一个最近一个请求的时间戳。

该改动在当前版本不会体现出作用,因为目前的优雅下线在该步骤之前已经拒绝了请求,因此滑动窗口现在还不会起作用,后续开发的下线逻辑在该处就不会直接拒绝这些请求,而是先放进来,然后使用到该处的滑动窗口逻辑来继续接收请求并正确响应,并且在响应中携带标识使客户端感知到服务端处于下线状态,以此做出响应的逻辑处理。
目前仍使用基于注册中心的下线逻辑,因此这一块更改的作用不会立马体现在当前版本中,同样也不会影响到当前的下线功能,属于是平滑升级。

Which issue(s) this PR fixes:
Fixes #

You should pay attention to items below to ensure your pr passes our ci test
We do not merge pr with ci tests failed

  • All ut passed (run 'go test ./...' in project root)
  • After go-fmt ed , run 'go fmt project' using goland.
  • Golangci-lint passed, run 'sudo golangci-lint run' in project root.
  • After import formatted, (using imports-formatter to run 'imports-formatter .' in project root, to format your import blocks, mentioned in CONTRIBUTING.md above)
  • Your new-created file needs to have apache license at the top, like other existed file does.
  • All integration test passed. You can run integration test locally (with docker env). Clone our dubbo-go-samples project and replace the go.mod to your dubbo-go, and run 'sudo sh start_integration_test.sh' at root of samples project root. (M1 Slice is not Support)

TheR1sing3un and others added 4 commits July 15, 2022 16:38
…fully shutdown

1. using sliding window for better logic to support gracefully shutdown,

Signed-off-by: TheR1sing3un <[email protected]>
…fully shutdown

1. using sliding window for better logic to support gracefully shutdown,

Signed-off-by: TheR1sing3un <[email protected]>
@TheR1sing3un
Copy link
Member Author

@LaurenceLiZhixin Please check!

@codecov-commenter
Copy link

codecov-commenter commented Jul 16, 2022

Codecov Report

Merging #1973 (8a37ced) into 3.0 (de6fcfb) will increase coverage by 0.18%.
The diff coverage is 68.75%.

❗ Current head 8a37ced differs from pull request most recent head d0894b8. Consider uploading reports for the commit d0894b8 to get more accurate results

@@            Coverage Diff             @@
##              3.0    #1973      +/-   ##
==========================================
+ Coverage   44.32%   44.50%   +0.18%     
==========================================
  Files         283      283              
  Lines       17067    17037      -30     
==========================================
+ Hits         7565     7583      +18     
+ Misses       8696     8650      -46     
+ Partials      806      804       -2     
Impacted Files Coverage Δ
config/graceful_shutdown.go 3.88% <0.00%> (-0.12%) ⬇️
config/graceful_shutdown_config.go 92.72% <100.00%> (+19.39%) ⬆️
filter/graceful_shutdown/provider_filter.go 74.35% <100.00%> (+0.67%) ⬆️
...nstance/random/random_service_instance_selector.go 66.66% <0.00%> (ø)
cluster/loadbalance/p2c/loadbalance.go 80.00% <0.00%> (+3.63%) ⬆️
metrics/prometheus/reporter.go 33.33% <0.00%> (+5.27%) ⬆️

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 de6fcfb...d0894b8. Read the comment docs.

@AlexStocks
Copy link
Contributor

@LaurenceLiZhixin pls review this pr. thx.

…onfig#OfflineRequestWindowTimeout

1. delete the "default" of
config.graceful_shutdown_config#OfflineRequestWindowTimeout

Signed-off-by: TheR1sing3un <[email protected]>
@Mulavar
Copy link
Member

Mulavar commented Jul 19, 2022

maybe we can add an issue for the pr? just personal advice.

@TheR1sing3un
Copy link
Member Author

maybe we can add an issue for the pr? just personal advice.

yep! I will do it!

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