Add sentinel support.

[jasongoodwin]

'-slave-restart-as-master' scenario is handled as well as the failover end which is an undocumented master state change that can otherwise take out a production environment.

That particular case is logged in a ticket here: https://github.com/antirez/redis/issues/1276#issuecomment-25942348

Failover-end scenario is also handled. I think that the restart of the actor is a fairly smart solution, but as per Gaetan's comment, it may be better to just send the ShardManager an updated shard instead of restarting the whole actor. Unsure which I prefer - the restart makes the actor immutable. Sending in the shard leaves supervision for true exception cases - as Gaetan mentioned in conversation. I think failover could arguably be considered an exception state.

This should be a simple and complete enough implementation to begin integration testing. It has been peer reviewed now, and while there are some opportunities, I believe this is a significantly better solution than using the reconfig hooks due to some of the peculiarities around sentinel behaviour such as the slave restart as master case.

[jasongoodwin]

Added a bit more - it'll failover on slave restart as master condition now (undocumented change to master state).

I think it needs a heartbeat on the active sentinel as it's possible for sentinel to fail without any noisy notification. I'll finish that up on my free time and submit it into the pull request.

The other thing is the feature for getting master info/sentinel isn't quite correct: Step 2: ask for master address Once a connection with a Sentinel is established, the client should retry to execute the following command on the Sentinel: SENTINEL get-master-addr-by-name master-name Where master-name should be replaced with the actual service name specified by the user. The result from this call can be one of the following three replies: An ip:port pair. A null reply. An -IDONTKNOW error. If an ip:port pair is received, this address should be used to connect to the Redis master. Otherwise if a null reply or -IDONTKNOW reply is received, the client should try the next Sentinel in the list.

I'll try to fix that up too.

It needs better testing around the lifecycle as well

[ghouet]

This looks good. I would just avoid throwing an exception that restarts the actor as it is actually not an exception for SentinelClient, that's everything it is suppose to do : failover. I would just replace the broken shard by sending a new Shard() to the shard manager.

Also, it might be good to rename the three classes that can be instantiated to talk to Redis : Brando->ShardManager->SentinelClient. The name of the class should reflect the functionality. SentinelClient is actually the actor you send request to, it's not only a SentinelClient. Same with ShardManager.

The following ones are bad suggestions but you get the ideas : Brando / ShardingBrando / FailoveringBrando

:+1: for the rest !

[jasongoodwin]

Thanks Gaetan - I think that the failover or slave-restart-as-master states could possibly be considered 'exception' states too. By using a restart the code is immutable but the other sentinel ha solution I implemented at mdialog does update the shards so I can see both as logical choices. Justification for the restart - as mentioned - immutability and the actor is actually in an exception state once a failover has occurred.

I would be open to making this change if the pull request is accepted - it seems clearer to update the shard as you sugguest.

And I'm starting to think that this may be better implemented as a trait rather than another supervising actor as there are three or four tiers of actors now - maybe an extension of the shard manager for example. And ya the name could be improved.

Thanks for reviewing this - glad you're as happy with this as I am.

[chrisdinn]

Looking forward to getting this in!

[jasongoodwin]

I didn't get to this over the weekend but I'll target this week.

[jasongoodwin]

Awww I never did this. :) Closing this ticket.