redis / docker-library-redis

Docker Official Image packaging for Redis
http://redis.io
BSD 3-Clause "New" or "Revised" License
1.11k stars 560 forks source link

docker stop's SIGTERM is ignored so has to wait to SIGKILL to stop #386

Open sjpotter opened 7 months ago

sjpotter commented 7 months ago

With normal redis outsides a container, I can send it a SIGINT or a SIGTERM and it catches it and stops cleanly basically immeadiately. When run with this docker container, I'd then expect similar behavior if I did a docker stop (or a docker rm -f). However, that is not what I see. It delays 10 seconds or so until it just seems to get the SIGKILL.

in watching the docker container (docker run -it) in one window and then I do a docker stop in another window, I see nothing happen in the redis output until it just dies without any messages (i.e. indicating to me that it was SIGKILLed).

sjpotter commented 7 months ago

as an addendum, if I docker exec into the container, kill -SIGTERM <redis pid> works as expected and it terminates right away.

tianon commented 7 months ago

Can you give a little more detail about how you're running your container? I can't reproduce a failure. In one terminal, I ran docker run -it --rm --name foo redis, and in another, docker stop foo, and the first shut down immediately (1:signal-handler (1701297057) Received SIGTERM scheduling shutdown...).

If this is related to your example in #387, then I'd suggest changing docker run redis:latest /bin/bash -c "touch /tmp/sentinel.conf ; /usr/local/bin/redis-sentinel /tmp/sentinel.conf" into docker run redis:latest /bin/bash -c "touch /tmp/sentinel.conf && exec /usr/local/bin/redis-sentinel /tmp/sentinel.conf" (so that redis-sentinel actually ends up as PID1 for sure and receives the signals properly).

sjpotter commented 7 months ago

They aren't quite related (i.e. I see the behavior without it), but I ran into them at same time, but was able to isolate

$ docker run redis:latest
1:C 30 Nov 2023 09:37:03.977 # WARNING Memory overcommit must be enabled! Without it, a background save or replication may fail under low memory condition. Being disabled, it can also cause failures without low memory condition, see https://github.com/jemalloc/jemalloc/issues/1328. To fix this issue add 'vm.overcommit_memory = 1' to /etc/sysctl.conf and then reboot or run the command 'sysctl vm.overcommit_memory=1' for this to take effect.
1:C 30 Nov 2023 09:37:03.977 * oO0OoO0OoO0Oo Redis is starting oO0OoO0OoO0Oo
1:C 30 Nov 2023 09:37:03.977 * Redis version=7.2.3, bits=64, commit=00000000, modified=0, pid=1, just started
1:C 30 Nov 2023 09:37:03.977 # Warning: no config file specified, using the default config. In order to specify a config file use redis-server /path/to/redis.conf
1:M 30 Nov 2023 09:37:03.978 * monotonic clock: POSIX clock_gettime
1:M 30 Nov 2023 09:37:03.978 * Running mode=standalone, port=6379.
1:M 30 Nov 2023 09:37:03.978 * Server initialized
1:M 30 Nov 2023 09:37:03.978 * Ready to accept connections tcp

$ 

and

time docker stop 1745b62e0506
u1745b62e0506

real    0m12.294s
user    0m0.009s
sys 0m0.014s

notice how it doesn't print anything about getting signal and shutting down.

I'm wondering if its my docker (ubuntu provided one)

$ docker version
Client:
 Version:           24.0.5
 API version:       1.43
 Go version:        go1.20.7
 Git commit:        24.0.5-0ubuntu1
 Built:             Wed Aug 16 21:32:36 2023
 OS/Arch:           linux/amd64
 Context:           default

Server:
 Engine:
  Version:          24.0.5
  API version:      1.43 (minimum version 1.12)
  Go version:       go1.20.7
  Git commit:       24.0.5-0ubuntu1
  Built:            Wed Aug 16 21:32:36 2023
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.7.2b
  Version:          1.1.7-0ubuntu2
  GitCommit:        
 docker-init:
  Version:          0.19.0
  GitCommit:        

For my current use case, I've replaced docker stop {id} with docker exec {id} /bin/bash -c "kill -SIGTERM 1" (kill doesn't seem to be in container as a separate command), which accomplishes what I need, but I'm confused, I dont see anything that would cause that.

if I do what I said above, it behaves as I'd expect and exits gracefully

$ docker run redis
1:C 30 Nov 2023 09:43:49.228 # WARNING Memory overcommit must be enabled! Without it, a background save or replication may fail under low memory condition. Being disabled, it can also cause failures without low memory condition, see https://github.com/jemalloc/jemalloc/issues/1328. To fix this issue add 'vm.overcommit_memory = 1' to /etc/sysctl.conf and then reboot or run the command 'sysctl vm.overcommit_memory=1' for this to take effect.
1:C 30 Nov 2023 09:43:49.228 * oO0OoO0OoO0Oo Redis is starting oO0OoO0OoO0Oo
1:C 30 Nov 2023 09:43:49.228 * Redis version=7.2.3, bits=64, commit=00000000, modified=0, pid=1, just started
1:C 30 Nov 2023 09:43:49.228 # Warning: no config file specified, using the default config. In order to specify a config file use redis-server /path/to/redis.conf
1:M 30 Nov 2023 09:43:49.228 * monotonic clock: POSIX clock_gettime
1:M 30 Nov 2023 09:43:49.228 * Running mode=standalone, port=6379.
1:M 30 Nov 2023 09:43:49.229 * Server initialized
1:M 30 Nov 2023 09:43:49.229 * Ready to accept connections tcp
1:signal-handler (1701337444) Received SIGTERM scheduling shutdown...
1:M 30 Nov 2023 09:44:04.576 * User requested shutdown...
1:M 30 Nov 2023 09:44:04.576 * Saving the final RDB snapshot before exiting.
1:M 30 Nov 2023 09:44:04.588 * DB saved on disk
1:M 30 Nov 2023 09:44:04.588 # Redis is now ready to exit, bye bye...
$ time docker exec 28801c8c2e06 /bin/bash -c "kill -SIGTERM 1"

real    0m0.105s
user    0m0.014s
sys 0m0.010s
LaurentGoderre commented 6 months ago

@sjpotter can you confirm that the redis process is PID1 when it doesn't get the SIGTERM?

sjpotter commented 6 months ago
$ docker run -d redis:latest
53f27d71dc05cf8537e3b7f88bbb20c16d063409abb76feea2aaa207c54427e8

then ls -l /proc/

$ docker exec -it 53f27d71dc05cf8537e3b7f88bbb20c16d063409abb76feea2aaa207c54427e8 /bin/bash -c 'ls -l /proc'
total 0
dr-xr-xr-x  9 redis redis    0 Dec 13 16:57 1
dr-xr-xr-x  9 root  root     0 Dec 13 16:58 19
drwxrwxrwt  2 root  root    40 Dec 13 16:57 acpi
<snip>

and again

$ docker exec -it 53f27d71dc05cf8537e3b7f88bbb20c16d063409abb76feea2aaa207c54427e8 /bin/bash -c 'ls -l /proc/'
total 0
dr-xr-xr-x  9 redis redis    0 Dec 13 16:57 1
dr-xr-xr-x  9 root  root     0 Dec 13 16:59 43
<snip>

so it seems there's only one process as pid1 as each bash got the only other pid.

$ time docker stop 53f27d71dc05cf8537e3b7f88bbb20c16d063409abb76feea2aaa207c54427e8
53f27d71dc05cf8537e3b7f88bbb20c16d063409abb76feea2aaa207c54427e8

real    0m12.262s
user    0m0.023s
sys 0m0.016s
tianon commented 6 months ago

The biggest difference between your reproducer and mine is the use of -it (stdin and TTY), so I tried without those and still wasn't able to reproduce: 🤔

$ docker run -d --name redis redis
Unable to find image 'redis:latest' locally
latest: Pulling from library/redis
1f7ce2fa46ab: Already exists 
4827e9d1e197: Pull complete 
5845062cfda9: Pull complete 
44d659adcf8b: Pull complete 
b6962d83313d: Pull complete 
5d29cf86ecab: Pull complete 
4f4fb700ef54: Pull complete 
3a2d9f90268c: Pull complete 
Digest: sha256:396b0f027ba2f33bf385771a621b58c1fd834fd2c522c35c98fd24fc17863c2f
Status: Downloaded newer image for redis:latest
abddc0c0d336cab9b736bdcc7447596d273c1f201499fc1cee91fbc0ba00f1b1
$ time docker stop redis
redis

real    0m0.343s
user    0m0.010s
sys 0m0.010s
$ docker logs redis --tail=6
1:M 13 Dec 2023 19:32:27.356 * Ready to accept connections tcp
1:signal-handler (1702495952) Received SIGTERM scheduling shutdown...
1:M 13 Dec 2023 19:32:32.474 * User requested shutdown...
1:M 13 Dec 2023 19:32:32.474 * Saving the final RDB snapshot before exiting.
1:M 13 Dec 2023 19:32:32.482 * DB saved on disk
1:M 13 Dec 2023 19:32:32.482 # Redis is now ready to exit, bye bye...
sjpotter commented 6 months ago

perhaps a problem with ubuntu's provided docker. I should test with upstream directly, but hard to do that right now without possibly breaking my ability to work.