-
Notifications
You must be signed in to change notification settings - Fork 917
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 #1254
Conversation
Codecov Report
@@ Coverage Diff @@
## 3.0 #1254 +/- ##
==========================================
- Coverage 59.53% 55.75% -3.78%
==========================================
Files 259 272 +13
Lines 12737 12899 +162
==========================================
- Hits 7583 7192 -391
- Misses 4199 4808 +609
+ Partials 955 899 -56
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approved these changes
@@ -60,6 +60,9 @@ func (gf *gracefulShutdownFilter) Invoke(ctx context.Context, invoker protocol.I | |||
return gf.getRejectHandler().RejectedExecution(invoker.GetURL(), invocation) | |||
} | |||
atomic.AddInt32(&gf.activeCount, 1) | |||
if gf.shutdownConfig != nil && gf.activeCount > 0 { | |||
gf.shutdownConfig.RequestsFinished = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
没有锁,会导致 data race。
@@ -163,6 +163,7 @@ func waitForReceivingRequests() { | |||
// ignore this step | |||
return | |||
} | |||
providerConfig.ShutdownConfig.RejectRequest = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
graceful shutdown 时拒绝新请求不合适,会导致请求失败。
参考 grpc,先是关闭 listener,拒绝新连接,而不是请求。
ShutdownConfig should be not nil if it is configured in |
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
* 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>
* 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>
…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>
What this PR does:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: