Added Sentinel Support to ConnectionMultiplexer #406
Conversation
Initial pass at adding Sentinel support. Adds ConnectionMultiplexer that
is managed by a set of Sentinel servers. Usage:
```C#
ConfigurationOptions sentinelConfig = new ConfigurationOptions();
sentinelConfig.ServiceName = "ServiceNameValue";
sentinelConfig.EndPoints.Add("127.0.0.1", 26379);
sentinelConfig.EndPoints.Add("127.0.0.2", 26379);
sentinelConfig.EndPoints.Add("127.0.0.4", 26379);
sentinelConfig.TieBreaker = "";
sentinelConfig.DefaultVersion = new Version(3, 0);
// Add the configuration for the masters that we will connect to
ConfigurationOptions masterConfig = new ConfigurationOptions();
sentinelConfig.SentinelMasterConfignfigurationOptions = mo;
// Connect!
ConnectionMultiplexer sentinelConnection =
ConnectionMultiplexer.Connect(sentinelConfig);
// When we need to perform a command against the masters, use the master
connection
sentinelConnection.SentinelMasterConnection.GetDatabase().MyCommand();
```
This reverts commit abe6af2.
Modified approach to allow for a sentinel-only setup with the option of requesting a connection to a master managed by the sentinel connection. Can also support multiple services on the same sentinel.
… feature/sentinel
Add some basic handling for disposed connections that are managed by the Sentinel connection.
|
Adjusted the usage based on discussions in #22 to separate the connection to a Sentinel and getting a new connection to a master that is managed by the Sentinel. The sentinel connection can now create multiple connections to masters based on the ServiceName associated with the master's ConnectionOptions. ConfigurationOptions sentinelConfig = new ConfigurationOptions();
sentinelConfig.EndPoints.Add("127.0.0.1", 26379);
sentinelConfig.EndPoints.Add("127.0.0.2", 26379);
sentinelConfig.EndPoints.Add("127.0.0.4", 26379);
sentinelConfig.TieBreaker = "";
// Connect!
ConnectionMultiplexer sentinelConnection = ConnectionMultiplexer.Connect(sentinelConfig);
// Get a connection to the master
ConfigurationOptions masterConfig = new ConfigurationOptions();
masterConfig.ServiceName = "MyMasterService";
ConnectionMultiplexer masterConnection = sentinelConnection.GetSentinelMasterConnection(masterConfig); |
|
Any idea when this is going into a release? |
|
Unfortunately no. I have yet to hear back from any of the maintainers on this pull request. |
|
Really looking forward to this, our team is thinking about going with ServiceStack.Redis if Sentinel support is not going to be part of StackExchange.Redis. |
|
Unfortunately this project does not seem very active at the moment. We are looking at moving to redis cluster. |
|
Hopefully the extra attention will get the maintainers to respond back on this. |
|
Hi guys, what with this pull request? Cluster also not supported fully correctly, so we need some solution for failover situations. |
|
@Areson Is this Merged in to master?I am waiting for using it... |
|
Probably not. I haven't heared anything from The maintainers. |
|
@mgravell @NickCraver what are your thoughts on this? |
|
Any updates on this? |
|
Still no feedback from the maintainers. On Fri, Nov 11, 2016 at 2:14 PM, Shakil Thakur notifications@github.com
|
|
Hey guys and gals, sorry for the radio silence here - we are extremely busy on many things and the OSS is hard to keep up with anywhere nearly as often as we'd like. Here's the main issue here: we don't use Sentinel. That doesn't mean we don't want PRs with it. It's very much the opposite. However, it means it's exceedingly hard to test it. The same is true for clustering and a litany of issues we can't reproduce. So what are we doing about it? I'm working with some Micosoft folks to make spinning up test clusters in Azure a thing. With docker and such finally going mainstream, this is finally becoming a reality. It'll let us test Sentinel, Cluster, Master/Slave ("replica" soon?), etc. behavior - including broken behavior and seeing how things behave and adapt. I know it's a lot to ask, but please bear with us. I don't have time to update (quite literally) a thousand issues I need to add feedback or status updates on, but we are working on a dependency chain involved here to test all of these things much more quickly going forward. I am headed back from Seattle on a flight now, and was discussing this with the guys working on these bits at Microsoft this week. |
|
No worries! You definitely have a lot on your plate. I appreciate you On Fri, Nov 11, 2016 at 3:51 PM, Nick Craver notifications@github.com
|
|
@Areson I've tried this code, but it did not work for me. Specifically on Do you have any tests for this? Configs for Sentinel to make this work? I've used a pretty basic configuration for the Redis/Sentinel instances. I've tested that my configuration works from the standpoint that killing an instance of Redis triggers a vote by the running Sentinel instances to elect a new master and this master does, in fact work. Bringing Redis instaces up/down multiple time works flawlessly. But using this code with the same instances does not work. |
|
@iangriffin I'm afraid that this code was written while I was at a different employer than I am now, so the config that I did have is no longer accessible to me. However, I might be able to shed some light on things. You get the error that a connection is not of type "Sentinel" when the server selection strategy isn't sentinal. This gets determined in the ConnectionMultiplexer.cs file on lines 1415 - 1427. If you have any connections that return back that they are standalone (and I don't remember what that means), then it won't be considered a sentinal connection. Lines 1345 - 1357 also contribute to this. I think that all of the endpoint you add must be started as Sentinel instances and nothing else, and if you include a non-Sentinel endpoint then things will not work properly. I'm a bit hazy has it has been a while. |
|
@iangriffin , I had the same issue with you. where the type was recognized as Standalone. In my case, I had to add the following command in the sentinel configuration and the issue was resolved:
Hope this helps. |
|
@kgalanop Thank you, that's what I needed. Also, I had to set |
|
This seems to work well. I've got a test that will just insert X times in a loop. The insert itself is wrapped in Polly to handle retries on Exception (expected exception of Timeout or ConnectionException), which will ensure the write is completed when a new master is online. Outside the test, I can sleep/kill the master Redis instance which causes Sentinel to change the master and communicate it back to the running test. It seems this is working flawlessly in my (very) limited test. Previously, I entered all three servers into the connection string - when I killed one, SE.Redis would eventually connect to the new master, but I would always lose data in my loop (some writes don't happen). With Sentinel via the new code by @Areson, it's 100%. I'll work on a script to get a simple Redis/Sentinel configuration running. But I'm not sure I'll make time to remove Polly (which is critical because it handles the retries). Regardless, I'll push a PR for the TEST back in so others can at least view it. Ultimately, unit tests to simulate the Sentinel events would be ideal to get this accepted into the main repo by the maintainers. Maybe we can all pitch in. |
|
Any idea on the timeline on this or if you're still planning on merging it eventually? We're currently investigating automated failover with sentinel and are trying to determine if we need to write our own code to support it. I need to do more testing as this PR doesn't seem to change anything for me with respect to detecting new masters and preventing lost data, so I'm not entirely sure this is actually needed either. |
| if(connection.GetServer(e.EndPoint).IsSlave || e.EndPoint != currentSentinelMasterEndPoint) | ||
| { | ||
| // Wait for things to smooth out | ||
| Thread.Sleep(200); |
There was a problem hiding this comment.
make 200 a constant so that it can be tuned if required?
|
I tested the PR on my config: 3 Redis, 3 Sentinel. So far it works great. If I reboot the Redis master, everything continue to work in my application.
I hope that PR will be merged soon because it saved me a lot of time ! public static async Task<ConnectionMultiplexer> ConnectAsync(string configuration, TextWriter log = null)
{
IDisposable killMe = null;
try
{
var muxer = CreateMultiplexer(configuration);
killMe = muxer;
bool configured = await muxer.ReconfigureAsync(true, false, log, null, "connect").ObserveErrors().ForAwait();
if (!configured)
{
throw ExceptionFactory.UnableToConnect(muxer.failureMessage);
}
killMe = null;
if (muxer.ServerSelectionStrategy.ServerType == ServerType.Sentinel)
{
// Initialize the Sentinel handlers
muxer.InitializeSentinel(log);
}
return muxer;
}
finally
{
if (killMe != null) try { killMe.Dispose(); } catch { }
}
}
/// <summary>
/// Create a new ConnectionMultiplexer instance
/// </summary>
public static async Task<ConnectionMultiplexer> ConnectAsync(ConfigurationOptions configuration, TextWriter log = null)
{
IDisposable killMe = null;
try
{
var muxer = CreateMultiplexer(configuration);
killMe = muxer;
bool configured = await muxer.ReconfigureAsync(true, false, log, null, "connect").ObserveErrors().ForAwait();
if (!configured)
{
throw ExceptionFactory.UnableToConnect(muxer.failureMessage);
}
killMe = null;
if (muxer.ServerSelectionStrategy.ServerType == ServerType.Sentinel)
{
// Initialize the Sentinel handlers
muxer.InitializeSentinel(log);
}
return muxer;
}
finally
{
if (killMe != null) try { killMe.Dispose(); } catch { }
}
} |
|
I've done something similar to @lecaillon , branching off of this branch for a work project involving Sentinel. In addition to the .NET Core fixes mentioned above, I ran into a bug in if (firstCompleteRequest.Result?.Result == null)
return;Our configuration is similar to that above, and we've tested hammering the servers while pulling the virtual network interfaces. We see the harness issue exceptions for about five seconds (our configured down time before fail over) and then pick back up as if nothing ever happened. |
|
Hey guys. Do you know when this will be merged into master and pushed to NuGet? We need Sentinel in our very importatant element in web of micro services. |
|
Hello Nick. How can we proceed/help in order to add Sentinel support to your project? From my side I have found, thanks to this thread, a workaround that works until now, but something more robust is necessary. Thank you in advance. Philippe |
|
Are you guys ever going to merge this? HA support is not exactly some niche edge case requirement. |
|
Hey guys and gals, I spent a lot of time getting unit tests workable on current tooling and CI (see #700). I think we have testing up to the place we can test sentinel as well (testing which current PRs lack). I'll work on getting Sentinel configured and scripted locally for testing spin up, is anyone willing to help on the testing front once that's done? I think active progress has moved these commits to #692 (so we should work there), but I would like to get this in the library. We need a lot of help maintaining these things, and on the testing front it's especially helpful because in areas we don't use in production we often don't know how something should behave or what the corner cases are. I've added docs here on testing: https://stackexchange.github.io/StackExchange.Redis/Testing I'm sorry this is very slow, many libraries, a lot of work, and it's hard to keep up. PRs with tests (which wasn't an easy ask before, and this isn't a trivial ask as it involves new server types) will be very important moving forward, it's just too much to keep up with in the amount of time an effort needed to research and validate changes otherwise. Does anyone have time to help here? |
|
I'm closing this out not to end discussion, let's just focus on the active PR please - if you're willing to help or have thoughts, please use #692 |
|
When i connect to sentinel i am getting "faulted: ProtocolFailure on PING" No masters detected. If i connect development redis sentinel is working fine. But when i connect QA redis sentinel, I am facing this issue. Could any one please help me to resolve this issue |
Initial pass at adding Sentinel support to the ConnectionMultiplexer. Usage:
Pulled the initial code from PR #121 and updated to fit into the latest version. Added support for handling various reconnect scenarios, as outlined in the client documentation at http://redis.io/topics/sentinel-clients, even if the client misses the "change master" event from the Sentinel servers. Also added support updating the connection's list of Sentinel nodes as they are added after the connection has been started.