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

Use STOPSIGNAL SIGQUIT instead of SIGTERM #377

Closed
nottrobin opened this issue Nov 15, 2019 · 6 comments
Closed

Use STOPSIGNAL SIGQUIT instead of SIGTERM #377

nottrobin opened this issue Nov 15, 2019 · 6 comments

Comments

@nottrobin
Copy link

nottrobin commented Nov 15, 2019

Docker (and, therefore, Kubernetes) send SIGTERM as the default signal to stop containers. Docker will then wait 10 seconds before sending SIGKILL, Kubernetes will wait 30 seconds. They expect containers to exit gracefully within this grace period:

When you issue a docker stop command Docker will first ask nicely for the process to stop and if it doesn't comply within 10 seconds it will forcibly kill it. If you've ever issued a docker stop and had to wait 10 seconds for the command to return you've seen this in action

From: https://www.ctl.io/developers/blog/post/gracefully-stopping-docker-containers/

It’s important that your application handle termination gracefully so that there is minimal impact on the end user and the time-to-recovery is as fast as possible!

In practice, this means your application needs to handle the SIGTERM message and begin shutting down when it receives it. This means saving all data that needs to be saved, closing down network connections, finishing any work that is left, and other similar tasks.

From: https://cloud.google.com/blog/products/gcp/kubernetes-best-practices-terminating-with-grace

However, this is not what nginx does:

TERM, INT fast shutdown
QUIT graceful shutdown

From: http://nginx.org/en/docs/control.html

The Dockerfiles in this project use STOPSIGNAL SIGTERM (which actually isn't necessary since this is the default).

What this means in practice is that when Docker or Kubernetes decide to stop a container, nginx will die immediately, closing any connections straight away:

$ curl localhost
curl: (52) Empty reply from server

It would be much healthier, and comply with the ethos of both Docker and Kubernetes if instead the nginx image made an effort to close all connections before shutting down. This can be achieved nicely with:

STOPSIGNAL SIGQUIT

I've verified this behaviour locally with this Dockerfile.

@thresheek
Copy link
Collaborator

We discussed that in #167 - can you check if dangling sockets are still an issue with your approach?

@nottrobin
Copy link
Author

Okay, interesting. We don't use any unix sockets, so it's not an issue for us.

But more generally, this SIGTERM behaviour seems definitely undesirable to me. Given how prevalent nginx is, and how prevalent its usage in e.g. Kubernetes is, the default docker image should really be able to shut down gracefully without causing broken connections.

It seems to me that the correct signal to send is SIGQUIT and the issue with unix signals is a bug that should be fixed. Is the nginx project aware of the issue?

@nottrobin
Copy link
Author

nottrobin commented Nov 15, 2019

(Either that or nginx should change its behaviour on SIGTERM to be more graceful)

@thresheek
Copy link
Collaborator

nginx is aware of the issue, see https://trac.nginx.org/nginx/ticket/753 for more details.

@thresheek
Copy link
Collaborator

Now that https://trac.nginx.org/nginx/ticket/753 is fixed, we can try moving to SIGQUIT.

@nesl247
Copy link

nesl247 commented Jul 15, 2020

The fix was released in 1.19.1. Can this be implemented then?

johejo added a commit to johejo/nginx-opentracing that referenced this issue Jul 6, 2022
miry pushed a commit to opentracing-contrib/nginx-opentracing that referenced this issue Jul 7, 2022
zmiklank added a commit to zmiklank/nginx-container that referenced this issue Mar 29, 2023
Nginx upstream [1] uses SIGQUIT for graceful shutdown even in their
container images [2].

We switch to SIGQUIT too, as current default SIGTERM immediately
closes opened connections.

[1] http://nginx.org/en/docs/control.html
[2] nginxinc/docker-nginx#377
zmiklank added a commit to sclorg/nginx-container that referenced this issue Apr 4, 2023
Nginx upstream [1] uses SIGQUIT for graceful shutdown even in their
container images [2].

We switch to SIGQUIT too, as current default SIGTERM immediately
closes opened connections.

[1] http://nginx.org/en/docs/control.html
[2] nginxinc/docker-nginx#377
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 a pull request may close this issue.

3 participants