Skip to content

Sentinel: remove Thread.Sleep and throws #1403

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 25, 2020
Merged

Sentinel: remove Thread.Sleep and throws #1403

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
10a7996
Sentinel: remove Thread.Sleep and throws
Mar 24, 2020
a241223
Moving 2 master failover races to local DEBUG builds
Mar 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
156 changes: 74 additions & 82 deletions src/StackExchange.Redis/ConnectionMultiplexer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2194,9 +2194,6 @@ internal void InitializeSentinel(LogProxy logProxy)
}
}

private const string FAILED_CONFIGURE_MASTER_FOR_SERVICE_MSG =
"Failed connecting to configured master for service: {0}, after {1} retries within interval {2}";

/// <summary>
/// Returns a managed connection to the master server indicated by
/// the ServiceName in the config.
Expand All @@ -2217,19 +2214,27 @@ public ConnectionMultiplexer GetSentinelMasterConnection(ConfigurationOptions co
return sentinelConnectionChildren[config.ServiceName];
}

// Clear out the endpoints
config.EndPoints.Clear();

// Get an initial endpoint
var msg = string.Format(FAILED_CONFIGURE_MASTER_FOR_SERVICE_MSG, config.ServiceName, 5, 10);
EndPoint initialMasterEndPoint = Retry<EndPoint>(5, 10, () => GetConfiguredMasterForService(config.ServiceName), msg);
// Get an initial endpoint - try twice
EndPoint newMasterEndPoint = GetConfiguredMasterForService(config.ServiceName)
?? GetConfiguredMasterForService(config.ServiceName);

//SHADI: do
//{
// initialMasterEndPoint = GetConfiguredMasterForService(config.ServiceName);
//} while(initialMasterEndPoint == null);
if (newMasterEndPoint == null)
{
throw new RedisConnectionException(ConnectionFailureType.UnableToConnect,
$"Sentinel: Failed connecting to configured master for service: {config.ServiceName}");
}

config.EndPoints.Add(initialMasterEndPoint);
// Replace the master endpoint, if we found another one
// If not, assume the last state is the best we have and minimize the race
if (config.EndPoints.Count == 1)
{
config.EndPoints[0] = newMasterEndPoint;
}
else
{
config.EndPoints.Clear();
config.EndPoints.Add(newMasterEndPoint);
}

ConnectionMultiplexer connection = Connect(config, log);

Expand Down Expand Up @@ -2259,35 +2264,37 @@ internal void OnManagedConnectionRestored(object sender, ConnectionFailedEventAr
connection.sentinelMasterReconnectTimer.Dispose();
connection.sentinelMasterReconnectTimer = null;
}

// Run a switch to make sure we have update-to-date
// information about which master we should connect to
SwitchMaster(e.EndPoint, connection);

try
{
// Verify that the reconnected endpoint is a master,
// and the correct one otherwise we should reconnect
if (connection.GetServer(e.EndPoint).IsSlave || e.EndPoint != connection.currentSentinelMasterEndPoint)

// Run a switch to make sure we have update-to-date
// information about which master we should connect to
SwitchMaster(e.EndPoint, connection);

try
{
// Verify that the reconnected endpoint is a master,
// and the correct one otherwise we should reconnect
if (connection.GetServer(e.EndPoint).IsSlave || e.EndPoint != connection.currentSentinelMasterEndPoint)
{
// This isn't a master, so try connecting again
SwitchMaster(e.EndPoint, connection);
}
}
catch (Exception)
{
// Wait for things to smooth out
Thread.Sleep(200);
// If we get here it means that we tried to reconnect to a server that is no longer
// considered a master by Sentinel and was removed from the list of endpoints.

// This isn't a master, so try connecting again
// If we caught an exception, we may have gotten a stale endpoint
// we are not aware of, so retry
SwitchMaster(e.EndPoint, connection);
}
}
catch (Exception)
{
// If we get here it means that we tried to reconnect to a server that is no longer
// considered a master by Sentinel and was removed from the list of endpoints.

// Wait for things to smooth out
Thread.Sleep(200);

// If we caught an exception, we may have gotten a stale endpoint
// we are not aware of, so retry
SwitchMaster(e.EndPoint, connection);
// Log, but don't throw in an event handler
// TODO: Log via new event handler? a la ConnectionFailed?
}
}

Expand All @@ -2301,7 +2308,15 @@ internal void OnManagedConnectionFailed(object sender, ConnectionFailedEventArgs
{
connection.sentinelMasterReconnectTimer = new Timer((_) =>
{
SwitchMaster(e.EndPoint, connection);
try
{
// Attempt, but do not fail here
SwitchMaster(e.EndPoint, connection);
}
catch (Exception)
{

}
}, null, TimeSpan.FromSeconds(0), TimeSpan.FromSeconds(1));
}
}
Expand All @@ -2321,11 +2336,11 @@ internal EndPoint GetConfiguredMasterForService(string serviceName) =>
internal EndPoint currentSentinelMasterEndPoint;

/// <summary>
/// Switches the SentinelMasterConnection over to a new master.
/// Switches the SentinelMasterConnection over to a new master.
/// </summary>
/// <param name="switchBlame">the endpoing responsible for the switch</param>
/// <param name="connection">the connection that should be switched over to a new master endpoint</param>
/// <param name="log">log output</param>
/// <param name="switchBlame">The endpoing responsible for the switch</param>
/// <param name="connection">The connection that should be switched over to a new master endpoint</param>
/// <param name="log">Log to write to, if any</param>
internal void SwitchMaster(EndPoint switchBlame, ConnectionMultiplexer connection, TextWriter log = null)
{
if (log == null) log = TextWriter.Null;
Expand All @@ -2334,56 +2349,33 @@ internal void SwitchMaster(EndPoint switchBlame, ConnectionMultiplexer connectio
{
string serviceName = connection.RawConfig.ServiceName;

// Get new master
var msg = string.Format(FAILED_CONFIGURE_MASTER_FOR_SERVICE_MSG, serviceName, 5, 10);
EndPoint masterEndPoint = Retry<EndPoint>(5, 10, () => GetConfiguredMasterForService(serviceName), msg);

//Shadi: do
//{
// masterEndPoint = GetConfiguredMasterForService(serviceName);
//} while(masterEndPoint == null);

connection.currentSentinelMasterEndPoint = masterEndPoint;
// Get new master - try twice
EndPoint newMasterEndPoint = GetConfiguredMasterForService(serviceName)
?? GetConfiguredMasterForService(serviceName);

if (!connection.servers.Contains(masterEndPoint))
if (newMasterEndPoint == null)
{
connection.RawConfig.EndPoints.Clear();
connection.servers.Clear();
//connection._serverSnapshot = new ServerEndPoint[0];
connection.RawConfig.EndPoints.Add(masterEndPoint);
Trace(string.Format("Switching master to {0}", masterEndPoint));
// Trigger a reconfigure
connection.ReconfigureAsync(false, false, logProxy, switchBlame, string.Format("master switch {0}", serviceName), false, CommandFlags.PreferMaster).Wait();
throw new RedisConnectionException(ConnectionFailureType.UnableToConnect,
$"Sentinel: Failed connecting to switch master for service: {serviceName}");
}

UpdateSentinelAddressList(serviceName);
}
}

/// <summary>
/// retry mechanism that executing func t times to get a non-null result within a constant interval between retries.
/// if t exceeds the times parameter without success, it will throw an exception with a descriptive message
/// </summary>
/// <typeparam name="T">A generic result type expected to return</typeparam>
/// <param name="times">retries times trying to get a result before throw exception</param>
/// <param name="interval">interval between retries in millisconds</param>
/// <param name="func">delegate repeatedly runs until getting the desired result</param>
/// <param name="message">message to be used within NullReferenceException that should be thrown in case
/// there is no result after n retries</param>
/// <returns>object of type T</returns>
private T Retry<T>(int times, int interval, Func<T> func, string message)
{
for (var t = 0; t < times; t++)
{
var result = func();
if (result != null)
if (newMasterEndPoint != null)
{
return result;
connection.currentSentinelMasterEndPoint = newMasterEndPoint;

if (!connection.servers.Contains(newMasterEndPoint))
{
connection.RawConfig.EndPoints.Clear();
connection.servers.Clear();
connection.RawConfig.EndPoints.Add(newMasterEndPoint);
Trace(string.Format("Switching master to {0}", newMasterEndPoint));
// Trigger a reconfigure
connection.ReconfigureAsync(false, false, logProxy, switchBlame, string.Format("master switch {0}", serviceName), false, CommandFlags.PreferMaster).Wait();
}

UpdateSentinelAddressList(serviceName);
}
Thread.Sleep(interval);
}

throw new NullReferenceException(message);
}

internal void UpdateSentinelAddressList(string serviceName)
Expand Down
2 changes: 2 additions & 0 deletions tests/StackExchange.Redis.Tests/Sentinel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,7 @@ public async Task SentinelFailoverAsyncTest()
}
}

#if DEBUG
[Fact]
public async Task GetSentinelMasterConnectionFailoverTest()
{
Expand Down Expand Up @@ -467,6 +468,7 @@ await UntilCondition(TimeSpan.FromSeconds(10), () => {
var conn1 = Conn.GetSentinelMasterConnection(ServiceOptions);
Assert.NotEqual(endpoint, conn1.currentSentinelMasterEndPoint.ToString());
}
#endif

[Fact]
public async Task GetSentinelMasterConnectionWriteReadFailover()
Expand Down