-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
simplify storage SIGINT signal handler #3437
simplify storage SIGINT signal handler #3437
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3437 +/- ##
==========================================
+ Coverage 85.23% 85.30% +0.06%
==========================================
Files 1277 1278 +1
Lines 119088 119287 +199
==========================================
+ Hits 101502 101754 +252
+ Misses 17586 17533 -53
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.
LGTM
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.
LGTM
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.
Good job
#### What type of PR is this? - [x] bug - [ ] feature - [ ] enhancement #### What does this PR do? closes vesoft-inc#3441 simplify graphd signal handler:Set stop condition instead of waiting all workers stop #### Which issue(s)/PR(s) this PR relates to? In vesoft-inc#3437 , it simplify storage SIGINT signal handler #### Special notes for your reviewer, ex. impact of this fix, etc: #### Additional context/ Design document: #### Checklist: - [ ] Documentation affected (Please add the label if documentation needs to be modified.) - [ ] Incompatibility (If it breaks the compatibility, please describe it and add the corresponding label.) - [ ] If it's needed to cherry-pick (If cherry-pick to some branches is required, please label the destination version(s).) - [ ] Performance impacted: Consumes more CPU/Memory #### Release notes: Please confirm whether to be reflected in release notes and how to describe: > ` Migrated from vesoft-inc#3542 Co-authored-by: endy.li <25311962+heroicNeZha@users.noreply.github.com>
What type of PR is this?
What does this PR do?
In short:
let's say we(storage server) have a thread running some task.
user send kill SIGINT signal, linux schedule the signal handler at the thread we talked about.
now, we interrupt the previous task, executing the signal handler,
it calls storage server->stop()
trying to clear everything storage server has(also include this thread!!!!).
Then we stuck.
Which issue(s)/PR(s) this PR relates to?
Storage hang at exit #3434