search

redis / redis-py

Redis Python client
MIT License
12.53k stars 2.5k forks source link

Sentinel master discovery uses the wrong command #626

Open robgolding opened 9 years ago

robgolding commented 9 years ago

SENTINEL masters is used to discover the master for a given cluster, whereas SENTINEL get-master-addr-by-name should be used instead.

See https://github.com/antirez/redis/issues/2615 for the issues that this causes.

pfreixes commented 8 years ago

IMHO you are right, current implementation is not right. Also I can see a variable called min_other_sentinels that is not necessary, the quorum to elect a new master is determined by a group of sentinels and configured in the sentinel config file rather than give it as a param.

Sentinels by default run listening for connections to TCP port 26379, so for Sentinels to work, port 26379 of your servers must be open to receive connections from the IP addresses of the other Sentinel instances. Otherwise Sentinels can't talk and can't agree about what to do, so failover will never be performed.

This must be fixed, but Im concern that we are going to broke the compatibility. Thoughts @andymccurdy ?

github-actions[bot] commented 4 years ago

This issue is marked stale. It will be closed in 30 days if it is not updated.

er0k commented 4 years ago

This bug is still present and IMO this issue should not be closed. The current master should be determined by SENTINEL get-master-addr-by-name and not by SENTINEL masters

gnat commented 3 years ago

@abrookins Would highly appreciate you taking a look at this one. I feel like the correct change was done here: https://github.com/er0k/redis-py/pull/3/files#diff-9d65b3446d551c0ed9e7bb0a16cf7b31f8b94c1024b9bf05afaa63ce70950019R195

Thanks!

dpaluch-rp commented 2 years ago

Bump! I'm face the same issue, after 6 years bug still present

github-actions[bot] commented 1 year ago

This issue is marked stale. It will be closed in 30 days if it is not updated.

jeffwidman commented 1 year ago

This is not stale, it's needs maintainer attention... there's a fix suggested ☝️

kashalls commented 10 months ago

https://github.com/bitnami/charts/tree/main/bitnami/redis For read-only operations, access the service using port 6379. For write operations, it's necessary to access the Redis® Sentinel cluster and query the current master using the command below (using redis-cli or similar): SENTINEL get-master-addr-by-name

https://redis.io/docs/reference/sentinel-clients/

This should be looked into still. I currently pass a loadbalancer to redis-py and it always picks a replica instead of the master. I loose my high availability if the master crashes/goes down if I specifically only select the master.

kashalls commented 10 months ago

Looks like support for this was introduced 2 years ago in #1636 by @chayim

Realistically @PKizzle we should be able to change sentinel.py to use sentinel_get_master_addr_by_name