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

Gracefully shutdown when receiving SIGTERM #13369

Closed
aylei opened this issue Nov 11, 2019 · 2 comments · Fixed by #37441
Closed

Gracefully shutdown when receiving SIGTERM #13369

aylei opened this issue Nov 11, 2019 · 2 comments · Fixed by #37441
Labels
component/server type/enhancement The issue or PR belongs to an enhancement.

Comments

@aylei
Copy link

aylei commented Nov 11, 2019

Feature Request

Is your feature request related to a problem? Please describe:

tidb-server listen for SIGQUIT to perform gracefully shutdown, however, most of the process managers send SIGTERM by default and expect the process exits gracefully, e.g. systemd, kubernetes.

#8711 introduce a gracefully shutdown with timeout for all signals, however, that did not guarantee all the ongoing requests could get finished.

Describe the feature you'd like:

Gracefully shutdown when tidb-server receiving SIGTERM.

Describe alternatives you've considered:

Making tryGracefullyShutdownTimeout configurable, which could satisfy most of the scenarios. However, a better default behavior is waiting for shutting down gracefully forever if request keeps processing and leave the decision to operators or automation systems to shutdown the server forcefully, e.g. sending SIGKILL. So I propose to change the default behavior of receiving SIGTERM, which should wait forever to be graceful by default. A configurable timeout is useful but is non goal of this issue.

Sending SIGQUIT in process manager, which is possible but requires customized work on different platforms, e.g. ansible and kubernetes.

Teachability, Documentation, Adoption, Migration Strategy:

This changes a long standing behavior, so though the fix itself is tiny, we should target it in the next major release.

ref: pingcap/tidb-ansible#506 #8711

@aylei aylei added the type/enhancement The issue or PR belongs to an enhancement. label Nov 11, 2019
@ekalinin
Copy link
Contributor

BTW, it seems like it's very easy to implement.
Just add a new signal in two places:

@aylei
Copy link
Author

aylei commented Nov 13, 2019

@ekalinin Exactly.

And I am still investigating whether tidb-ansible and tidb-operator need to be modified accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/server type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants