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

Doesn't save on SIGTERM #4

Closed
ericfrederich opened this issue Jul 25, 2014 · 18 comments
Closed

Doesn't save on SIGTERM #4

ericfrederich opened this issue Jul 25, 2014 · 18 comments

Comments

@ericfrederich
Copy link

According to http://redis.io/topics/signals Redis is supposed to do a save on SIGTERM:

If Redis is configured to persist on disk using RDB files, a synchronous (blocking) save is performed. Since the save is performed in a synchronous way no additional memory is used.

According to http://docs.docker.com/reference/commandline/cli/#stop Docker sends a SIGTERM followed by a SIGKILL after a period (10 second default).

The main process inside the container will receive SIGTERM, and after a grace period, SIGKILL

It seems to receive the SIGTERM and gracefully shutdown but it does so without a save.
Subsequent start of the container doesn't have the data.

$ docker run --name some-redis -d redis
4f9fd01b2ed8be449610c4210831b583c1c1f3db1e5560ed4c3921fee34f318d
$ docker run -it --link some-redis:redis --rm redis sh -c 'echo set foo bar | redis-cli -h "$REDIS_PORT_6379_TCP_ADDR" -p "$REDIS_PORT_6379_TCP_PORT"'
OK
$ docker run -it --link some-redis:redis --rm redis sh -c 'echo get foo | redis-cli -h "$REDIS_PORT_6379_TCP_ADDR" -p "$REDIS_PORT_6379_TCP_PORT"'
"bar"
$ docker stop some-redis
some-redis
$ docker logs some-redis  | tail -n6
[1] 25 Jul 15:25:58.611 # Server started, Redis version 2.8.12
[1] 25 Jul 15:25:58.611 # WARNING overcommit_memory is set to 0! Background save may fail under low memory condition. 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] 25 Jul 15:25:58.611 * The server is now ready to accept connections on port 6379
[1 | signal handler] (1406301986) Received SIGTERM, scheduling shutdown...
[1] 25 Jul 15:26:26.770 # User requested shutdown...
[1] 25 Jul 15:26:26.770 # Redis is now ready to exit, bye bye...
$ docker start some-redis 
some-redis
$ docker run -it --link some-redis:redis --rm redis sh -c 'echo get foo | redis-cli -h "$REDIS_PORT_6379_TCP_ADDR" -p "$REDIS_PORT_6379_TCP_PORT"'
(nil)
@yosifkit
Copy link
Contributor

By default redis is only an "in memory" key/value store. If you change your first run to make redis persistent: docker run --name some-redis -d redis redis-server --appendonly yes, it will work.

We forgot to add this to the documentation on the hub when we added the volume, it should be there in the next day or so.

@badboy
Copy link

badboy commented Jul 29, 2014

@yosifkit That's not the point. Redis enables RDB persistence by default. On shutdown (and on SIGTERM) it should automatically initiate a save. For some reason this is not happening here.
I'll try to find some time later to test it myself.

@badboy
Copy link

badboy commented Jul 29, 2014

Ok, it's clear now why it won't save on SIGTERM:

docker run -it --link some-redis:redis --rm redis sh -c 'redis-cli -h "$REDIS_PORT_6379_TCP_ADDR" -p "$REDIS_PORT_6379_TCP_PORT" CONFIG GET save'
1) "save"
2) ""

There are no save intervals set, for whatever reason.

@badboy
Copy link

badboy commented Jul 29, 2014

Never mind, I run this test against the wrong image. It works just as expected now:

% docker run -it --link some-redis:redis --rm redis sh -c 'redis-cli -h "$REDIS_PORT_6379_TCP_ADDR" -p "$REDIS_PORT_6379_TCP_PORT" CONFIG GET save'   
1) "save"                                                                                          
2) "3600 1 300 100 60 10000"

Log after docker stop some-redis:

[1 | signal handler] (1406638969) Received SIGTERM, scheduling shutdown...
[1] 29 Jul 13:02:49.333 # User requested shutdown...
[1] 29 Jul 13:02:49.333 * Saving the final RDB snapshot before exiting.
[1] 29 Jul 13:02:49.350 * DB saved on disk
[1] 29 Jul 13:02:49.350 # Redis is now ready to exit, bye bye...

This is not a bug then.
@ericfrederich: Check that you're using the right Docker image, also check the config of the running instance.

@tianon
Copy link
Contributor

tianon commented Jul 29, 2014

I hit just docker run -it --rm --name some-redis redis (as documented), and then hit it up with redis-cli:

$ docker run -it --link some-redis:redis --rm redis sh -c 'exec redis-cli -h "$REDIS_PORT_6379_TCP_ADDR" -p "$REDIS_PORT_6379_TCP_PORT" CONFIG GET save'
1) "save"
2) ""

However, when I run it with the configuration file provided with the source code (which is not the same as the compiled-in default configuration) via docker run -it --rm --name some-redis redis redis-server /usr/src/redis/redis.conf, I get:

$ docker run -it --link some-redis:redis --rm redis sh -c 'exec redis-cli -h "$REDIS_PORT_6379_TCP_ADDR" -p "$REDIS_PORT_6379_TCP_PORT" CONFIG GET save'
1) "save"
2) "900 1 300 10 60 10000"

@badboy
Copy link

badboy commented Jul 29, 2014

Hm, afaik the included redis.conf should have the same values as the built-in defaults. I'm still not sure why it doesn't work for you ("900 1 300 10 60 10000" is definitely the default compiled in this source as well as in the redis.conf).

I can't reproduce it here anymore (and the one time I could reproduce it was against another Redis image, probably older, not sure).

Can you try removing the complete redis image from docker (docker rmi redis), re-pull this repo and again build it using docker run -t redis .?
(I'm no Docker expert, but that's how you do it, right? That worked for me)

@ericfrederich
Copy link
Author

Okay... I did a docker build from this repo and the issue does not exist any more.
The issue I guess seems to be with the hosted Docker image.... needs an update.

I didn't see any redis.conf other than in /usr/src/redis within either of the images

@yosifkit
Copy link
Contributor

I built a new image from the repo. The only difference between this and the current release is the removal of , "--bind", "0.0.0.0" from the CMD. We assumed this was just a cosmetic change, because the default config was the same. It seems that if you specify any argument to redis-server, they assume you are going to specify everything.

The first line only shows up when specifying no arguments to redis-server:

$ docker run -it --rm --name some-redis redis redis-server
[1] 31 Jul 15:55:23.775 # Warning: no config file specified, using the default config. In order to specify a config file use redis-server /path/to/redis.conf
                _._                                                  
           _.-``__ ''-._                                             
      _.-``    `.  `_.  ''-._           Redis 2.8.12 (00000000/0) 64 bit
  .-`` .-```.  ```\/    _.,_ ''-._                                   
 (    '      ,       .-`  | `,    )     Running in stand alone mode
 |`-._`-...-` __...-.``-._|'` _.-'|     Port: 6379
 |    `-._   `._    /     _.-'    |     PID: 1
  `-._    `-._  `-./  _.-'    _.-'                                   
 |`-._`-._    `-.__.-'    _.-'_.-'|                                  
 |    `-._`-._        _.-'_.-'    |           http://redis.io        
  `-._    `-._`-.__.-'_.-'    _.-'                                   
 |`-._`-._    `-.__.-'    _.-'_.-'|                                  
 |    `-._`-._        _.-'_.-'    |                                  
  `-._    `-._`-.__.-'_.-'    _.-'                                   
      `-._    `-.__.-'    _.-'                                       
          `-._        _.-'                                           
              `-.__.-'                                               

... (and the rest of the server start)

I will get the no args version up to stackbrew (the official image builder). For now you can run it with docker run -it --rm --name some-redis redis redis-server and it will work.

@yosifkit
Copy link
Contributor

This has been fixed in the latest version of redis and is now much smaller. (no need for --appendonly)
2.8.13, 5cd5c0d9ebfc, 175 MB

@miraage
Copy link

miraage commented Jul 4, 2016

Next solution worked for me (latest redis, 3.2):

# docker-compose.yml
services:
  foo_redis:
    image: redis
    command: redis-server --appendonly yes

@jpetazzo
Copy link

The problem is back (caused by the "protected mode" workaround).

@tianon
Copy link
Contributor

tianon commented Aug 23, 2016

Poo 😞

@tianon tianon reopened this Aug 23, 2016
@tianon
Copy link
Contributor

tianon commented Aug 23, 2016

This might mean we have to patch the source to change the "protected mode" default value. 😭

@tianon
Copy link
Contributor

tianon commented Aug 23, 2016

Seems like we could just modify https://github.com/antirez/redis/blob/a81a92ca2ceba364f4bb51efde9284d939e7ff47/src/server.h#L118 (CONFIG_DEFAULT_PROTECTED_MODE) after extracting and before compiling. It's a pretty minor change, and since we build from source, it should be reasonably simple (and less fragile than our entrypoint hacking too).

TimWolla added a commit to TimWolla/official-images that referenced this issue Aug 27, 2016
On SIGTERM a dump.rdb should be created when no special
parameters are passed.

see redis/docker-library-redis#4
TimWolla added a commit to TimWolla/official-images that referenced this issue Sep 1, 2016
On SIGTERM a dump.rdb should be created when no special
parameters are passed.

see redis/docker-library-redis#4
TimWolla added a commit to TimWolla/official-images that referenced this issue Sep 1, 2016
On SIGTERM a dump.rdb should be created when no special
parameters are passed.

see redis/docker-library-redis#4
TimWolla added a commit to TimWolla/official-images that referenced this issue Sep 1, 2016
On SIGTERM a dump.rdb should be created when no special
parameters are passed.

see redis/docker-library-redis#4
@x-yuri
Copy link

x-yuri commented Jan 20, 2021

For those interested in why

It seems that if you specify any argument to redis-server, they assume you are going to specify everything.

welcome to the adjacent issue.

@ebuildy
Copy link

ebuildy commented Feb 16, 2023

could we reopen it?

Our use case

On kubernetes, we use argo-workflow to run tests. Redis is deployed as a daemon:

  - name: redis-start
    daemon: true
    container:
      image: redis:6-alpine
      resources:
        requests:
          memory: "128Mi"
          cpu: "100m"
      readinessProbe:
        exec:
          command:
          - redis-cli
          - ping
        initialDelaySeconds: 5
        periodSeconds: 2
        timeoutSeconds: 1

Run fine, when the workflow stop, redis stay because the SIGTERM stuff failed.

How run redis 100% on memory without the snapshot stuff please?

@yosifkit
Copy link
Contributor

@ebuildy, I think that if you don't want persistence then either start redis with an empty save value, or set it via your testing setup (e.g. redis-cli config set save '').

$ # regular run
$ docker run -it --rm --name redi redis
...
^C1:signal-handler (1677110921) Received SIGINT scheduling shutdown...
1:M 23 Feb 2023 00:08:41.487 # User requested shutdown...
1:M 23 Feb 2023 00:08:41.487 * Saving the final RDB snapshot before exiting.
1:M 23 Feb 2023 00:08:41.499 * DB saved on disk
1:M 23 Feb 2023 00:08:41.499 # Redis is now ready to exit, bye bye...
$ docker exec -it redi redis-cli config get save
1) "save"
2) "3600 1 300 100 60 10000"

$ # no save run
$ docker run -it --rm --name redi redis --save
...
1:M 23 Feb 2023 00:08:49.650 * Ready to accept connections
^C1:signal-handler (1677110940) Received SIGINT scheduling shutdown...
1:M 23 Feb 2023 00:09:00.132 # User requested shutdown...
1:M 23 Feb 2023 00:09:00.132 # Redis is now ready to exit, bye bye...
$ .. $ docker exec -it redi redis-cli config get save
1) "save"
2) ""

@ebuildy
Copy link

ebuildy commented Feb 23, 2023

thanks you!

adamiBs pushed a commit to adamiBs/docker-library-redis that referenced this issue Sep 19, 2024
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

8 participants