mhart / kinesalite

An implementation of Amazon's Kinesis built on LevelDB
MIT License
808 stars 86 forks source link

Handle properly SINGINT and SIGTERM signals #43

Closed nhuray closed 7 years ago

nhuray commented 7 years ago

Hi @mhart,

This is a pull request to manage properly the SINGINT and SIGTERM signals when Kinesalite is bundle in a Docker container.

Context

The current behavior is due to a change introduced by c61b0e9 in Node.

With this commit, when a user sends SIGINT by pressing CTRL-C in a terminal where node is the foreground process, the node process doesn't exit explicitly and instead forward the signal.

Please read this issue for more information: https://github.com/nodejs/node-v0.x-archive/issues/9131

Impact

When Kinesalite run in a Docker container (as PID 1) and we send a SIGINT or SIGTERM signal with CTRL+C the container does not exit properly:

CONTAINER ID        IMAGE                        STATUS               NAMES
51b1f016686b        saikocat/kinesalite:1.11.5   Exited (137)   kinesalite
mhart commented 7 years ago

This is really an issue with Docker – it runs processes with PID 1 and provides no way to have normal (non-PID-1) SIGINT behaviour. Node.js acts correctly here – other containers like mysql and redis suffer from the same problem.

However, that said, I'm happy to add this with some checks for process.pid – I'll merge this in a sec

mhart commented 7 years ago

Merged and released as 1.11.6 – thanks for the contribution!

I limited it to just SIGINT and only if the pid is 1, as it is in Docker – all other behaviour should act as normal

Let me know if you have any further issues

nhuray commented 7 years ago

It's just perfect:

~/D/P/B/W/docker-kinesalite ❯❯❯ docker run -ti  bandsintown/kinesalite                                                                                                                                                ⏎ 1.11.6 ✚ ✱
Kinesalite started on port 4567
^C
caught SIGINT, exiting
~/D/P/B/W/docker-kinesalite ❯❯❯ docker ps                                                                                                                                                                               
CONTAINER ID        IMAGE               COMMAND             CREATED             STATUS              PORTS               NAMES
~/D/P/B/W/docker-kinesalite ❯❯❯ docker ps -a                                                                                                                                                                            
CONTAINER ID        IMAGE                      COMMAND                  CREATED             STATUS                      PORTS               NAMES
880eaaea34b0        bandsintown/kinesalite     "node kinesalite.j..."   21 seconds ago      Exited (0) 17 seconds ago                       epic_visvesvaraya

Thanks 👍