Add hostname support in Sentinel. by yossigo · Pull Request #8282 · redis/redis

yossigo · 2021-01-03T09:22:45Z

This is both a bugfix and an enhancement.

Internally, Sentinel relies entirely on IP addressess to identify
instances. When configured with a new master, it also requires users to
specify and IP and not hostname.

However, replicas may use the replica-announce-ip configuration to
announce a hostname. When that happens, Sentinel fails to match the
announced hostname with the expected IP and considers that a different
instance, trigerring reconfiguration, etc.

Another use case is where TLS is used and clients are expected to match
the hostname to connect to with the certificate's SAN attribute. To
properly implement this configuration, it is necessary for Sentinel to
redirect clients to a hostname rather than an IP address.

The new 'resolve-hostnames' configuration parameter determines if
Sentinel is willing to accept hostnames. It is set by default to no,
which maintains backwards compatibility and avoids unexpected DNS
resolution delays on systems with DNS configuration issues.

Internally, Sentinel contineus to identify instances by their resolved
IP address and will also report the IP by default. The new
'announce-hostnames' parameter determines if Sentinel should prefer to
announce a hostname, when available, rather than an IP address. This
applies to addresses returned to clients, as well as their
representation in the configuration file, REPLICAOF configuration
commands, etc.

This commit also introduces SENTINEL CONFIG GET and SENTINEL CONFIG SET
which can be used to introspect or configure global Sentinel
configuration that was previously was only possible by directly
accessing the configuration file and possibly restarting the instance.

Fixes #7758, #7393, #2118, #7928

Co-authored-by: myl1024 myl92916@qq.com

This is both a bugfix and an enhancement. Internally, Sentinel relies entirely on IP addressess to identify instances. When configured with a new master, it also requires users to specify and IP and not hostname. However, replicas may use the replica-announce-ip configuration to announce a hostname. When that happens, Sentinel fails to match the announced hostname with the expected IP and considers that a different instance, trigerring reconfiguration, etc. Another use case is where TLS is used and clients are expected to match the hostname to connect to with the certificate's SAN attribute. To properly implement this configuration, it is necessary for Sentinel to redirect clients to a hostname rather than an IP address. The new 'resolve-hostnames' configuration parameter determines if Sentinel is willing to accept hostnames. It is set by default to no, which maintains backwards compatibility and avoids unexpected DNS resolution delays on systems with DNS configuration issues. Internally, Sentinel contineus to identify instances by their resolved IP address and will also report the IP by default. The new 'announce-hostnames' parameter determines if Sentinel should prefer to announce a hostname, when available, rather than an IP address. This applies to addresses returned to clients, as well as their representation in the configuration file, REPLICAOF configuration commands, etc. This commit also introduces SENTINEL CONFIG GET and SENTINEL CONFIG SET which can be used to introspect or configure global Sentinel configuration that was previously was only possible by directly accessing the configuration file and possibly restarting the instance. Co-authored-by: myl1024 <myl92916@qq.com>

yossigo · 2021-01-03T09:32:08Z

Pinging @hwware, would really appreciate your review on this one. I had limited bandwidth to promote this and don't feel familiar enough with all potential aspects of Sentinel. Thank you!

oranagra · 2021-01-03T09:51:42Z

I haven't reviewed the code, but i noticed that sentinel.conf is unmodified (that's where the new config, and it's implications (DNS resolution delays?) should be mentioned)

hwware · 2021-01-04T23:23:07Z

Hi @yossigo i will take look the code and help for reviewing this one, thanks!

* Fix messed up formatting * Use dupSentinelAddr where possible

Also, adds tests for sentinel-user and sentinel-pass usage with Sentinel ACL configuration.

Co-authored-by: sundb <sundbcn@gmail.com>

oranagra

This is both a bugfix and an enhancement.

is there a simple bug fix here that can be separated from the big feature? or is the big feature what's fixing the bug? if there is, i think we may want to first make a PR with the bugfix so that it can be cherry picked to 6.0?

oranagra · 2021-01-18T16:22:09Z

+
+test "SENTINEL CONFIG SET handles on-the-fly credentials reconfiguration" {
+    # Make sure we're starting with a broken state...
+    after 5000


these long sleeps (the one below too) are not a wise idea, they'll slow down the whole test suite by 10 seconds.
if we keep adding more more of these we'll eventually suffer greatly.
i think we need to work hard to resist these and find other solutions.

can't that be changed to some wait_for that will usually exit a lot sooner?

I agree, I think this comment is true in general for both sentinel and cluster tests but it'll require some tinkering.

yossigo · 2021-01-18T19:37:09Z

@oranagra regarding your other comment, the bug is the fact that Redis instances themselves may use hostnames and Sentinel will do the wrong thing. There's no way to fix that without adding this capability. A potential backport could be to just produce a big error and bail out but I don't consider this a better experience.

yossigo · 2021-01-21T21:33:45Z

@redis/core-team Please approve.

New Sentinel behavior is controlled by two config keywords, which by default are backwards compatible/off.
New Sentinel commands to introspect and set configuration on the fly.

hwware

looks good to me

madolson

Interface LGTM, did not look through the code.

oranagra · 2021-01-28T14:34:47Z

@itamarhaber @yossigo who's making a doc PR?

yossigo · 2021-01-28T16:20:51Z

@oranagra I'll do that

mabrarov · 2021-01-30T06:33:03Z

Hi @yossigo,

Am I correct assuming that this PR is going to be included into Redis 6.2 release?

oranagra · 2021-01-30T07:12:21Z

@mabrarov yes.
RC3 coming out next week with all the content from unstable

bofm · 2021-02-09T15:41:41Z

Hi. I just tested redis:6.2-rc3 from dockerhub.

The setup is 3 pairs of redis + sentinel. Each pair is on a separate host.

Initial config:

...	replica-announce-ip	replicaof
redis1	host1	no one
redis2	host2	replicaof host1
redis3	host3	replicaof host1

sentinel ...	resolve-hostnames	announce-hostnames	announce-ip	monitor
sentinel1	yes	yes	host1	host1 6379 2
sentinel2	yes	yes	host2	host1 6379 2
sentinel3	yes	yes	host3	host1 6379 2

Also, on each sentinel I set:

sentinel down-after-milliseconds <mymaster> 3000
sentinel failover-timeout <mymaster> 3000

No IP addresses are mentioned in config files. Only hostnames.

After the startup redis2 and redis3 are successfully become replicas of redis1, perform a full sync.
On all of the 3 sentinel instances in the logs there are messages like:

1:X 09 Feb 2021 14:58:11.662 # +monitor master m
1:X 09 Feb 2021 14:58:11.663 * +slave slave x
1:X 09 Feb 2021 14:58:11.667 * +slave slave y
1:X 09 Feb 2021 14:58:11.753 * +sentinel sentinel a b ...
1:X 09 Feb 2021 14:58:12.715 * +sentinel sentinel c d ...
1:X 09 Feb 2021 14:59:25.100 # +new-epoch 1

So the startup seems OK.

Then I execute debug sleep 30 on redis1. After that redis2 is elected and promoted to master. Redis3 becomes a replica of redis2. So far so good.
After 30 seconds have passed I have 2 masters: redis1 and redis2. And it stays like this forever. All the sentinels keep logging every 10 seconds:

+convert-to-slave slave <redis1-ip>:6379 redis1 6379 @ redis redis2 6379

Redis1 is not switched to replica.

yossigo · 2021-02-09T18:54:36Z

@bofm Do you see anything in the logfiles of redis1?
Also, can you confirm that redis1 is able to handle DNS resolution (because when announce-hostnames is used, REPLICAOF uses the name rather than IP address).
Another thing to do is connect to redis1 and use MONITOR to see if/how Sentinel attempts to reconfigure it.

bofm · 2021-02-10T11:28:11Z

Do you see anything in the logfiles of redis1?

The last messages are these ones from the moment when redis1 woke up from that 30 seconds sleep.

# Connection with replica host3:6379 lost.
# Connection with replica host2:6379 lost.

Also, can you confirm that redis1 is able to handle DNS resolution?

Yes. Redis-cli-pinging redis2 and redis3 works on redis1: redis-cli -h host[23] ping

use MONITOR to see if/how Sentinel attempts to reconfigure it.

Every time sentinel1 is logging +convert-to-slave slave <redis1-ip>:6379 redis1 6379 the MONITOR on redis1 shows this:

1612955909.348765 [0 10.244.0.196:39303] "INFO"
1612955909.350920 [0 10.244.0.196:39303] "MULTI"
1612955909.351197 [0 10.244.0.196:39303] "EXEC"
1612955909.371758 [0 10.244.0.198:37703] "INFO"
1612955909.372426 [0 10.244.0.198:37703] "MULTI"
1612955909.372510 [0 10.244.0.198:37703] "EXEC"
1612955909.418702 [0 10.244.0.197:34307] "INFO"
1612955909.419409 [0 10.244.0.197:34307] "MULTI"
1612955909.419506 [0 10.244.0.197:34307] "EXEC"
1612955909.560671 [0 10.244.0.197:34307] "PING"
1612955909.655916 [0 10.244.0.198:37703] "PING"
1612955909.933972 [0 10.244.0.196:39303] "PING"
1612955910.634191 [0 10.244.0.197:34307] "PING"
1612955910.677667 [0 10.244.0.198:37703] "PING"

It looks like something is missing inside the MULTI-EXEC.

bofm · 2021-02-10T12:20:33Z

Another thing I noticed is that config rewrite on sentinel generates duplicated lines related to resolve-hostnames and announce-hostnames parameters in the config:

 ...
# Generated by CONFIG REWRITE
user default on nopass ~* &* +@all
sentinel myid f34f3cc00b4444e3275f202f20248e2f60fe49c1
sentinel resolve-hostnames yes
sentinel announce-hostnames yes
sentinel config-epoch my-redis 1
sentinel leader-epoch my-redis 1
sentinel current-epoch 1
sentinel resolve-hostnames yes
sentinel announce-hostnames yes
sentinel known-replica my-redis host1 6379
sentinel resolve-hostnames yes
sentinel announce-hostnames yes
sentinel known-replica my-redis host2 6379
sentinel resolve-hostnames yes
sentinel announce-hostnames yes
sentinel known-sentinel my-redis host3 26379 0c70c752383f67ed3f4430a55bf45da6448d9fa9
sentinel resolve-hostnames yes
sentinel announce-hostnames yes
sentinel known-sentinel my-redis host2 26379 815c69e70f428b5a6aea8fd8c31798285418d49a
sentinel resolve-hostnames yes
sentinel announce-hostnames yes
sentinel resolve-hostnames yes
sentinel announce-hostnames yes
sentinel resolve-hostnames yes
sentinel announce-hostnames yes

yossigo · 2021-02-10T14:55:33Z

@bofm The rewrite issue will be fixed by #8480, but I'm not able to reproduce the other problem. I would suspect it involves something with rename-command (do you use it?) but that would still not explain why the entire sequence of commands is missing.

Does the same setup work without hostnames?

bofm · 2021-02-10T16:03:59Z

The problem was that maxmemory was 100mb and every command resulted in OOM error. I'm sorry. Though it is quite surprising that 100mb is not enough for an empty Redis to work properly.

yossigo · 2021-02-10T16:39:54Z

@bofm It should be enough, perhaps you should include the output of INFO to figure this out.

This is both a bugfix and an enhancement. Internally, Sentinel relies entirely on IP addresses to identify instances. When configured with a new master, it also requires users to specify and IP and not hostname. However, replicas may use the replica-announce-ip configuration to announce a hostname. When that happens, Sentinel fails to match the announced hostname with the expected IP and considers that a different instance, triggering reconfiguration, etc. Another use case is where TLS is used and clients are expected to match the hostname to connect to with the certificate's SAN attribute. To properly implement this configuration, it is necessary for Sentinel to redirect clients to a hostname rather than an IP address. The new 'resolve-hostnames' configuration parameter determines if Sentinel is willing to accept hostnames. It is set by default to no, which maintains backwards compatibility and avoids unexpected DNS resolution delays on systems with DNS configuration issues. Internally, Sentinel continues to identify instances by their resolved IP address and will also report the IP by default. The new 'announce-hostnames' parameter determines if Sentinel should prefer to announce a hostname, when available, rather than an IP address. This applies to addresses returned to clients, as well as their representation in the configuration file, REPLICAOF configuration commands, etc. This commit also introduces SENTINEL CONFIG GET and SENTINEL CONFIG SET which can be used to introspect or configure global Sentinel configuration that was previously was only possible by directly accessing the configuration file and possibly restarting the instance. Co-authored-by: myl1024 <myl92916@qq.com> Co-authored-by: sundb <sundbcn@gmail.com>

oranagra · 2021-04-22T05:45:33Z

This test failed tonight on MacOS:
https://github.com/redis/redis/runs/2405534375?check_suite_focus=true

00:38:38> Sentinel announces hostnames: Expected [verify_hostname_announced "127.0.0.1"] || [verify_hostname_announced "::1"] (context: type eval line 7 cmd {assert {[verify_hostname_announced "127.0.0.1"] || [verify_hostname_announced "::1"]}} proc ::test)

@hwware maybe you can look into it.

hwware · 2021-04-22T15:05:14Z

@oranagra ack

oranagra added release-notes indication that this issue needs to be mentioned in the release notes state:major-decision Requires core team consensus labels Jan 3, 2021

Add missing sentinel.conf.

3e2d41b

hwware mentioned this pull request Jan 5, 2021

Sentinel: Fix Config Dependency and Rewrite Sequence issue #8271

Merged

yueki1993 mentioned this pull request Jan 5, 2021

Add support for ReadFrom.subnet and ReadFrom.regex redis/lettuce#1570

Closed

4 tasks

hwware reviewed Jan 5, 2021

View reviewed changes

Comment thread src/sentinel.c

hwware reviewed Jan 5, 2021

View reviewed changes

Comment thread src/sentinel.c Outdated

Comment thread src/sentinel.c

yossigo added 2 commits January 10, 2021 13:55

Code review changes.

530942e

* Fix messed up formatting * Use dupSentinelAddr where possible

Drop connections when auth config changes.

06a857e

Also, adds tests for sentinel-user and sentinel-pass usage with Sentinel ACL configuration.

yossigo mentioned this pull request Jan 11, 2021

in a K8s deployment, Sentinel Can't update the IP of the old master when the old master get back online again with different IP #8300

Closed

sundb reviewed Jan 12, 2021

View reviewed changes

Comment thread sentinel.conf Outdated

Merge remote-tracking branch 'upstream/unstable' into sentinel-hostnames

593d89f

sundb reviewed Jan 12, 2021

View reviewed changes

Comment thread src/sentinel.c Outdated

Apply suggestions from code review

bfcdf74

Co-authored-by: sundb <sundbcn@gmail.com>

hwware reviewed Jan 14, 2021

View reviewed changes

Comment thread src/sentinel.c

yossigo requested a review from oranagra January 18, 2021 14:18

Merge remote-tracking branch 'upstream/unstable' into sentinel-hostnames

98748a0

oranagra reviewed Jan 18, 2021

View reviewed changes

Code review cleanups.

cc38fc8

oranagra previously approved these changes Jan 21, 2021

View reviewed changes

hwware previously approved these changes Jan 21, 2021

View reviewed changes

itamarhaber previously approved these changes Jan 26, 2021

View reviewed changes

yossigo added the approval-needed Waiting for core team approval to be merged label Jan 26, 2021

Merge branch 'unstable' into sentinel-hostnames

7d3eeaa

yossigo dismissed stale reviews from hwware and oranagra via 7d3eeaa January 26, 2021 19:51

oranagra approved these changes Jan 26, 2021

View reviewed changes

madolson approved these changes Jan 27, 2021

View reviewed changes

yossigo merged commit bb7cd97 into redis:unstable Jan 28, 2021

yossigo deleted the sentinel-hostnames branch January 28, 2021 10:09

yossigo mentioned this pull request Jan 28, 2021

using dns address for the configuration of "sentinel announce-ip" and "replicaof" #7393

Closed

oranagra mentioned this pull request Jan 31, 2021

6.2 RC3 #8427

Merged

satheeshaGowda mentioned this pull request Feb 18, 2021

[QUESTION] Sentinel host name support not working as expected. #8507

Closed

hwware mentioned this pull request Jun 10, 2021

WIP: Add support to fallback to all results of getaddrinfo() (ipv6/ ipv4 dual-stack, DNS-RR) #6374

Open

aoterolorenzo mentioned this pull request Sep 27, 2022

Sentinel not picking up new master IP address after container is redeployed bitnami/containers#2967

Closed

coderabbitai Bot mentioned this pull request May 6, 2026

[FEAT] 부하테스트 및 인덱싱 캐싱 테스트 작업 완료 Hot6-NovelCraft/Hot6-NovelCraft#63

Merged

Uh oh!

Conversation

yossigo commented Jan 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yossigo commented Jan 3, 2021

Uh oh!

oranagra commented Jan 3, 2021

Uh oh!

hwware commented Jan 4, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oranagra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oranagra Jan 18, 2021

Choose a reason for hiding this comment

Uh oh!

yossigo Jan 18, 2021

Choose a reason for hiding this comment

Uh oh!

yossigo commented Jan 18, 2021

Uh oh!

yossigo commented Jan 21, 2021

Uh oh!

hwware left a comment

Choose a reason for hiding this comment

Uh oh!

madolson left a comment

Choose a reason for hiding this comment

Uh oh!

oranagra commented Jan 28, 2021

Uh oh!

yossigo commented Jan 28, 2021

Uh oh!

mabrarov commented Jan 30, 2021

Uh oh!

oranagra commented Jan 30, 2021

Uh oh!

bofm commented Feb 9, 2021

Uh oh!

yossigo commented Feb 9, 2021

Uh oh!

bofm commented Feb 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bofm commented Feb 10, 2021

Uh oh!

yossigo commented Feb 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bofm commented Feb 10, 2021

Uh oh!

yossigo commented Feb 10, 2021

Uh oh!

oranagra commented Apr 22, 2021

Uh oh!

hwware commented Apr 22, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

yossigo commented Jan 3, 2021 •

edited

Loading

bofm commented Feb 10, 2021 •

edited

Loading

yossigo commented Feb 10, 2021 •

edited

Loading