Sentinel master discovery uses the wrong command

[robgolding]

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]

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]

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

[er0k]

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]

@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]

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

[github-actions]

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

[jeffwidman]

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

[kashalls]

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]

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

[github-actions]

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

[tchernomax]

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

[AdamBethke]

I'd love to understand the state of this heading into 2025. I'm patching Sentinel with er0k#3 in my environment, and in my experience it works. It also solves a sticky issue: SENTINEL masters is a more expensive command to run than SENTINEL get-master-addr-by-name, to the point that in some deployments (including mine 😉) non-privileged users aren't given privileges to enumerate all of the masters.

I'm willing to roll up my sleeves if this is stuck because er0k#3 doesn't meet some of the contributing requirements / needs some additional, but would love to understand if doing that would be helpful before investing the time.