search

FZambia / sentinel

Redis Sentinel support for Redigo library
Apache License 2.0
69 stars 24 forks source link

Fix role check in configuration example #7

Closed jacobvosmaer closed 2 years ago

jacobvosmaer commented 2 years ago

The documentation suggested that when using Redis Sentinel, we should call sentinel.TestRole() every time we retrieve a Redis connection from the connection pool. This runs against the Redis documentation, which says that Sentinel clients should check the server role just once after they established a connection. https://redis.io/docs/reference/sentinel-clients/

If users of sentinel-go follow the old configuration example they will make many unnecessary ROLE calls to Redis.

This commit fixes the documentation to suggest that users call sentinel.TestRole() from within the Dial callback. This matches what the Redis documentation suggests we do.

FZambia commented 2 years ago

Thanks, so according to this:

Starting with Redis 2.8.12, when Redis Sentinel changes the configuration of an instance, for example promoting a replica to a master, demoting a master to replicate to the new master after a failover, or simply changing the master address of a stale replica instance, it sends a CLIENT KILL type normal command to the instance in order to make sure all the clients are disconnected from the reconfigured instance. This will force clients to resolve the master address again.

– connections in the pool should be closed automatically and this triggers a new Dial. Do I understand it right?

jacobvosmaer commented 2 years ago

Yes, that is correct.

As far as I understand the purpose of the ROLE check is to make sure there was no failover between the moment we looked up the master and the moment we connected to the master.