Skip to content

using dns address for the configuration of "sentinel announce-ip" , "replica-announce-ip" and "replicaof"#7758

Closed
myl1024 wants to merge 2 commits into
redis:unstablefrom
myl1024:sentinel-dns-bug
Closed

using dns address for the configuration of "sentinel announce-ip" , "replica-announce-ip" and "replicaof"#7758
myl1024 wants to merge 2 commits into
redis:unstablefrom
myl1024:sentinel-dns-bug

Conversation

@myl1024

@myl1024 myl1024 commented Sep 8, 2020

Copy link
Copy Markdown
Contributor

When we deploy redis in kubernetes, each pod corresponds to a redis instance or sentinel instance, using the dns address as the pod's fixed external address, we found:

  1. When sentinel is configured with "sentinel announce-ip sentinel-0-svc", sentinel-0-svc is this sentinel`s dns address, sentinel will always report the "+sentinel-address-switch" log when it is started.
  2. When slave is configured with "replicaof redis-0-svc", redis-0-svc is the master redis instance`s dns address, sentinel will report the “+slave” log first, followed by the “+fix-slave-config“ log,this will cause the slave to re-initiate replication to the master.
  3. When redis slave is configured with "replica-announce-ip redis-0-svc" (redis-0-svc is the dns address of the redis instance) and a failover occurs, sentinel will list the slave instance twice.

So, in these scenarios,

  1. When storing redis slave instances in sentinel, ip:port is used as the key uniformly (modify this in line 1198), even if the redis slave uses the dns address to configure replica-announce-ip.
  2. It should first convert the dns address (or hostname) to the ip address when search an instance (modify this in line 1397 and line 2372).

This PR is related to #7393

…ntinel (ie, sentinel announce-ip sentinel-0-svc) or redis (ie, replica-announce-ip redis-0-svc & replicaof redis-0-svc)
@myl1024 myl1024 changed the title using dns address for the configuration of "sentinel announce-ip" , "sentinel announce-ip"and"replicaof" using dns address for the configuration of "sentinel announce-ip" , "replica-announce-ip" and "replicaof" Sep 8, 2020
- Add sentinelAddrEqualsHostname() to simplify comparision of hostname
  with a sentinelAddr.
- Some parameter name updates to reflect that we're now expecting
  hostnames and IPs and not just IPs.
- Fix logic of getSentinelRedisInstanceByAddrAndRunID() which failed the
  tests.
@yossigo

yossigo commented Sep 8, 2020

Copy link
Copy Markdown
Collaborator

Thanks @myl1024, I've reviewed your work and pushed another commit to suggest a few cleanups and fix the Sentinel failing tests. My comments are in the commit, please let me know if this makes sense and works for you.

@yossigo

yossigo commented Sep 8, 2020

Copy link
Copy Markdown
Collaborator

Related to #2118, and we'll need to consider the issue of timing discussed in the original ticket before progressing with this.

@myl1024

myl1024 commented Sep 9, 2020

Copy link
Copy Markdown
Contributor Author

@yossigo Thanks for your review. Your commit is more concise and robust.

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

Successfully merging this pull request may close these issues.

2 participants