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

If the Nginx process dies I'm not sure what will happen #51

Open
JayH5 opened this issue May 24, 2017 · 4 comments
Open

If the Nginx process dies I'm not sure what will happen #51

JayH5 opened this issue May 24, 2017 · 4 comments

Comments

@JayH5
Copy link
Contributor

JayH5 commented May 24, 2017

The Nginx process is just forked from the main one during startup, nothing manages it. I'm not sure exactly what will happen if something goes wrong with it.

Thankfully, Nginx is pretty reliable.

@JayH5 JayH5 changed the title If the Nginx dies I'm not sure what will happen If the Nginx process dies I'm not sure what will happen May 24, 2017
@alexmuller
Copy link
Contributor

I was curious about this so I did a little investigating. If the nginx worker process dies it will get respawned by the nginx master (as you'd probably expect). If the nginx master process dies the worker will keep serving requests until it dies, but then won't be respawned.

It's relatively straightforward to add a Docker HEALTHCHECK to this Dockerfile but I don't know if our container scheduler will do anything about health status - like restart unhealthy containers? Is it worth doing a pull request for?

diff --git a/Dockerfile b/Dockerfile
index a6f8ec6..ce16cce 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -24,10 +24,14 @@ RUN apt-key adv --keyserver hkp://pgp.mit.edu:80 --recv-keys 573BFD6B3D8FBC64107
     && adduser nginx django
 COPY nginx/ /etc/nginx/
 
+RUN apt-get-install.sh curl
+
 # Install gunicorn
 COPY requirements.txt /requirements.txt
 RUN pip install -r /requirements.txt
 
+HEALTHCHECK --interval=5m --timeout=3s \
+    CMD ["curl", "--fail", "http://localhost:8000/", "||", "exit", "1"]
 EXPOSE 8000
 WORKDIR /app
 

@JayH5
Copy link
Contributor Author

JayH5 commented Aug 14, 2017

@alexmuller thanks for looking into this. I did have a branch locally where our tests sent a SIGKILL to various processes and checked what happened, but we've been working on simpler tests before that.

The issue is that the container starts up like this:

  1. The tini init process starts:
tini(1)
  1. The entrypoint script starts:
tini(1)--/bin/sh
  1. Nginx is started as a background job (&):
tini(1)--/bin/sh
          \--nginx master
              \-- nginx worker
  1. The gunicorn process replaces the entrypoint script (exec):
tini(1)--gunicorn master
         |  \- gunicorn worker
          \--nginx master
              \-- nginx worker

So the Gunicorn master process ends up being the parent process of the Nginx master process. When the Nginx process quits the Gunicorn process doesn't care. I think the SIGCHILD goes through to tini but it doesn't care either, it only cares about SIGCHILD from its direct child. So things carry on running happily.

The proper solution to this is probably to not run more than one thing at a time in a container, and instead run Nginx in a separate container. But we're not really able to do that currently with MC2.

So there should be something like this:

tini(1)--"thing"
            |
            +--gunicorn master
            |   \- gunicorn worker
            \--nginx master
                \-- nginx worker

... that dies when any of its direct children dies and also proxies signals to those children.

I haven't looked much into Docker healthchecks.. I always assumed that they were a Docker swarm thing. Additionally, these images don't have curl inside them.

@alexmuller
Copy link
Contributor

That makes sense, thanks for the detailed reply!

I think you're right about the actual solution being to make the container simpler.

@JayH5
Copy link
Contributor Author

JayH5 commented Aug 16, 2017

We now have tests for this process structure.

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

No branches or pull requests

2 participants