Redis sentinel failover

[ederuiter]

I was looking at implementing this: redis-rs/redis-rs#1438 but it looks like implementing this in redis-rs is not feasible atm (atleast for me) .

So I was thinking that maybe I can easily add it here .. there is already logic for recycling connections; it would be easy enough to add a check if the redis server that the connection points to, is still a healthy master/replica according to the sentinels.

Would you be open to accept a PR adding this functionality?

[bikeshedder]

PRs are always welcome. tbh. I know very little about redis cluster/sentinel so I'm entirely dependent on contributions by the community.

[ederuiter]

I was thinking this should be fairly easy .. but I am already running into an issue, currently the sentinel manager holds MultiplexedConnection's, however in order to check if the connection can be re-used according to the sentinel we need a way to retrieve the server ip/port of the multiplexedConnection.

And that is where I got stuck; as far as I can see there is no method to retrieve the server ip/port from a multiplexedConnection.

Another option I was thinking of is storing metadata for the connection in the Manager; eg: instead of Type being MultiplexedConnection let it be a struct that holds a MultiplexedConnection and some other metadata.
But this changes the return type of the Manager and breaks backward compatibility.

I might be able to use something like CLIENT LIST command to retrieve the listen address (laddr) from the redis connection; but that feels like a big hack to me.

Do you know any better way to retrieve the ip/port of the connection (basically the connection_info that is used to create the connection)?

[bikeshedder]

Don't worry about backwards incompatibility. Every time the redis crate bumps the 0.x version number it's an incompatible change anyways and I have to make an incompatible release of deadpool-redis, too.

Changing the return type of the Manager is fine as it's already our own custom type with a Wrapper that implements the ConnectionLike trait. The impact for the end user should be basically like any other new deadpool-redis release.

[ederuiter]

Ok I think I have a first draft of how to achieve this: master...ederuiter:deadpool:feature/sentinel-failover

^ I have introduced a SentinelConnection that wraps the MultiplexedConnection and changed the Connection / ConnectionLike implementation to work with that (and no I don't like the conn.conn; but was too tired to come up with better names).
Furthermore I switched from the SentinelClient to the Sentinel as this allows more visibility into the underlying redis server.

At least this seems to work as it used to 🎉. Now I "only" need to implement the logic for the detecting whenever a failover occurs and tying that into the recycle logic. I will have a go at that over the weekend.

[ederuiter]

I did some additional testing; but it seems that in order for this to work I still need some changes in redis-rs.
Switching to the Sentinel client was not enough; it does not expose the connection to the sentinel so I cannot subscribe to failover events :-(
The idea is to add an on_failover callback that we can use in deadpool-redis. My idea is to have a simple mechanic that increments a sentinel_revision integer on every failover event (on the manager). If we store the value of this in each SentinelConnection when creating a new connection; we can later check if the revision has changed in recycle. If it has changed we simply discard it.

[bikeshedder]

This problem sounds an awful lot like...

• Change pool configuration at runtime #166

I also toyed with the idea of adding a kind of "revision" or "version" information to all objects so it is easy to discard them if the configuration and/or managed is changed.