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.

Sign up for GitHub

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

Jump to bottom

RedisSentinelBackend doesn't work with "db" number other than 0 #201

Open
konstruktor227 opened this issue Apr 9, 2021 · 6 comments
Open

RedisSentinelBackend doesn't work with "db" number other than 0 #201

konstruktor227 opened this issue Apr 9, 2021 · 6 comments

Comments

@konstruktor227
Copy link

konstruktor227 commented Apr 9, 2021 •
edited
Loading

Hello,

we are trying to setup RedisSentinelBackend with a non-default db parameter to specify the database of the Redis server where the cache keys should be stored.

However when specifying db parameter other than 0, following error is raised by the Redis client:

redis.exceptions.ResponseError: unknown command `SELECT`, with args beginning with: `2`

Here is our configuration in a simple local setup (we use YAML format, however it shouldn't really matter here):

  dogpile_cache:
    regions: short_term,jwt_duration,oauth_access_token_duration,hourly
    backend: dogpile.cache.redis_sentinel
    arguments:
      sentinels:
        - [127.0.0.1, 26379]
      db: 2
      distributed_lock: True
      thread_local_lock: False
      service_name: mymaster

Of course everything works fine when parameter db is removed or set to 0.

I presume what is happening is that Redis client is told to use command SELECT 2 when working with a Sentinel node, while this command is not known by Sentinel.

I can reproduce the issue in redis-cli when connected to a Sentinel node:

127.0.0.1:26379> select 2
(error) ERR unknown command `select`, with args beginning with: `2`,

This seems to be an issue in RedisSentinelBackend of dogpile.cache, because when using e.g. Celery, we are able to specify the db numbers with Sentinel connection strings and keys are then properly created in the db of our choice.

I was also wondering if there could be some workaround for this issue, e.g. using connection_kwargs parameter? I tried passing the db argument to it, however it did not work.

Any help is greatly appreciated, because in our current setup we can't store the cache keys in db 0 of Redis, which currently blocks us from switching to Redis Sentinel setup.
@zzzeek
Copy link
Member

zzzeek commented Apr 9, 2021

hi there -

unfortunately I didn't write this backend and I know very little about it. dogpile doesn't emit those statements so perhaps this issue is actually in the redis.sentinel library? i dont know anything about how Celery interacts with redis.

the backend here is quite simple so feel free to send a PR if you know what it's not doing correctly: https://github.com/sqlalchemy/dogpile.cache/blob/master/dogpile/cache/backends/redis.py#L273

@zzzeek
Copy link
Member

zzzeek commented Apr 9, 2021

CC @sbrunner , the creator of this backend

@konstruktor227
Copy link
Author

@zzzeek Thanks a lot for the quick reply.

In this case I will definitely look deeper into this issue next week and at the backend implementation you shared.

Wish you a great weekend!

@jvanasco
Copy link
Member

jvanasco commented Apr 9, 2021

I was also wondering if there could be some workaround for this issue, e.g. using connection_kwargs parameter? I tried passing the db argument to it, however it did not work.

Looking at the code, I THINK you may be able to do a workaround by sending in a fully qualified url kwarg that has the host/port/database

@sbrunner
Copy link
Contributor

Effectively the db should always be 0, he is used to internal communication.
To change the used db you should set connection_kwargs to{'db': <db>}...

@konstruktor227
Copy link
Author

Thanks @sbrunner , this works nicely! I've tried that before but initially it didn't work due to how other library (pyramid_dogpile_cache2) is parsing the settings. It seems they do not really support connection_kwargs, so I will have to work around this now but it's no longer an issue in dogpile.cache.

One thing I would suggest is to extend the documentation for the sentinel backend to mention that db should always be set to 0 in this scenario. It would somehow also make sense in my mind to ignore the db parameter in the sentinel backend, since any value other than 0 will lead to a crash. I may find some time later to send a PR if you don't mind.

Thanks again everybody, I think now I will be able to work around this issue somehow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@zzzeek @jvanasco @sbrunner @konstruktor227