Skip to content

Fix Sentinel connection leak and AbortOnConnectFail=false handling #3187

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

Open
HarnageaGabriel wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix Sentinel connection leak and AbortOnConnectFail=false handling #3187

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
1c2a472
Make fallback discovery and keep-alive probes cluster-slot aware
HarnageaGabriel Aug 18, 2026
19103ee
Fix Sentinel connection leak and AbortOnConnectFail=false handling
HarnageaGabriel Aug 18, 2026
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
252 changes: 162 additions & 90 deletions src/StackExchange.Redis/ConnectionMultiplexer.Sentinel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,19 @@ private static ConnectionMultiplexer SentinelPrimaryConnect(ConfigurationOptions
sentinelConfig.Password = configuration.SentinelPassword;

var sentinelConnection = SentinelConnect(sentinelConfig, log);
try
{
var muxer = sentinelConnection.GetSentinelMasterConnection(configuration, log);
// Set reference to sentinel connection so that we can dispose it
muxer.sentinelConnection = sentinelConnection;

var muxer = sentinelConnection.GetSentinelMasterConnection(configuration, log);
// Set reference to sentinel connection so that we can dispose it
muxer.sentinelConnection = sentinelConnection;

return muxer;
return muxer;
}
catch
{
try { sentinelConnection.Dispose(); } catch { }
throw;
}
}

/// <summary>
Expand All @@ -172,12 +179,19 @@ private static async Task<ConnectionMultiplexer> SentinelPrimaryConnectAsync(Con
sentinelConfig.Password = configuration.SentinelPassword;

var sentinelConnection = await SentinelConnectAsync(sentinelConfig, writer).ForAwait();
try
{
var muxer = sentinelConnection.GetSentinelMasterConnection(configuration, writer);
// Set reference to sentinel connection so that we can dispose it
muxer.sentinelConnection = sentinelConnection;

var muxer = sentinelConnection.GetSentinelMasterConnection(configuration, writer);
// Set reference to sentinel connection so that we can dispose it
muxer.sentinelConnection = sentinelConnection;

return muxer;
return muxer;
}
catch
{
try { sentinelConnection.Dispose(); } catch { }
throw;
}
}

/// <summary>
Expand All @@ -187,7 +201,8 @@ private static async Task<ConnectionMultiplexer> SentinelPrimaryConnectAsync(Con
/// <param name="log">The writer to log to, if any.</param>
public ConnectionMultiplexer GetSentinelMasterConnection(ConfigurationOptions config, TextWriter? log = null)
{
if (ServerSelectionStrategy.ServerType != ServerType.Sentinel)
// A soft-failed SentinelConnect cannot detect a server type, but _isSentinel still records its intent.
if (ServerSelectionStrategy.ServerType != ServerType.Sentinel && (config.AbortOnConnectFail || !_isSentinel))
{
throw new RedisConnectionException(
ConnectionFailureType.UnableToConnect,
Expand All @@ -210,110 +225,162 @@ public ConnectionMultiplexer GetSentinelMasterConnection(ConfigurationOptions co
bool success = false;
ConnectionMultiplexer? connection = null;
EndPointCollection? endpoints = null;
RedisConnectionException? failure = null;

var sw = ValueStopwatch.StartNew();
do
try
{
// Sentinel has some fun race behavior internally - give things a few shots for a quicker overall connect.
const int queryAttempts = 2;

EndPoint? newPrimaryEndPoint = null;
for (int i = 0; i < queryAttempts && newPrimaryEndPoint is null; i++)
var sw = ValueStopwatch.StartNew();
do
{
newPrimaryEndPoint = GetConfiguredPrimaryForService(serviceName);
}
// Sentinel has some fun race behavior internally - give things a few shots for a quicker overall connect.
const int queryAttempts = 2;

if (newPrimaryEndPoint is null)
{
throw new RedisConnectionException(
ConnectionFailureType.UnableToConnect,
CommandFlags.None,
$"Sentinel: Failed connecting to configured primary for service: {config.ServiceName}");
}
EndPoint? newPrimaryEndPoint = null;
for (int i = 0; i < queryAttempts && newPrimaryEndPoint is null; i++)
{
newPrimaryEndPoint = GetConfiguredPrimaryForService(serviceName);
}

EndPoint[]? replicaEndPoints = null;
for (int i = 0; i < queryAttempts && replicaEndPoints is null; i++)
{
replicaEndPoints = GetReplicasForService(serviceName);
}
if (newPrimaryEndPoint is null)
{
failure = new RedisConnectionException(
ConnectionFailureType.UnableToConnect,
CommandFlags.None,
$"Sentinel: Failed connecting to configured primary for service: {config.ServiceName}");
if (config.AbortOnConnectFail)
{
throw failure;
}

endpoints = config.EndPoints.Clone();
if (connection is not null) try { connection.Dispose(); } catch { }
endpoints = config.EndPoints.Clone();
connection = ConnectImpl(config, log, endpoints: endpoints);
break;
}

// Replace the primary endpoint, if we found another one
// If not, assume the last state is the best we have and minimize the race
if (endpoints.Count == 1)
{
endpoints[0] = newPrimaryEndPoint;
}
else
{
endpoints.Clear();
endpoints.TryAdd(newPrimaryEndPoint);
}
EndPoint[]? replicaEndPoints = null;
for (int i = 0; i < queryAttempts && replicaEndPoints is null; i++)
{
replicaEndPoints = GetReplicasForService(serviceName);
}

if (replicaEndPoints is not null)
{
foreach (var replicaEndPoint in replicaEndPoints)
endpoints = config.EndPoints.Clone();

// Replace the primary endpoint, if we found another one
// If not, assume the last state is the best we have and minimize the race
if (endpoints.Count == 1)
{
endpoints.TryAdd(replicaEndPoint);
endpoints[0] = newPrimaryEndPoint;
}
else
{
endpoints.Clear();
endpoints.TryAdd(newPrimaryEndPoint);
}
}

connection = ConnectImpl(config, log, endpoints: endpoints);
if (replicaEndPoints is not null)
{
foreach (var replicaEndPoint in replicaEndPoints)
{
endpoints.TryAdd(replicaEndPoint);
}
}

// verify role is primary according to:
// https://redis.io/topics/sentinel-clients
bool isPrimary;
var server = connection.GetServer(newPrimaryEndPoint);
// ReSharper disable once ConditionIsAlwaysTrueOrFalseAccordingToNullableAPIContract
if (server is { })
{
try
if (connection is not null) try { connection.Dispose(); } catch { }
connection = ConnectImpl(config, log, endpoints: endpoints);

// verify role is primary according to:
// https://redis.io/topics/sentinel-clients
bool isPrimary;
var server = connection.GetServer(newPrimaryEndPoint);
// ReSharper disable once ConditionIsAlwaysTrueOrFalseAccordingToNullableAPIContract
if (server is { })
{
isPrimary = connection.CommandMap.IsAvailable(RedisCommand.ROLE)
? server.Role()?.Value == Role.LabelForMaster
: !server.IsReplica;
try
{
isPrimary = connection.CommandMap.IsAvailable(RedisCommand.ROLE)
? server.Role()?.Value == Role.LabelForMaster
: !server.IsReplica;
}
catch
{
// fallback if ROLE unavailable but not declared; see #3064
isPrimary = !server.IsReplica;
}

if (isPrimary)
{
success = true;
break;
}
}
catch

Thread.Sleep(100);
}
while (sw.ElapsedMilliseconds < config.ConnectTimeout);

if (!success)
{
failure ??= new RedisConnectionException(
ConnectionFailureType.UnableToConnect,
CommandFlags.None,
$"Sentinel: Failed connecting to configured primary for service: {config.ServiceName}");
if (config.AbortOnConnectFail)
{
// fallback if ROLE unavailable but not declared; see #3064
isPrimary = !server.IsReplica;
throw failure;
}

if (isPrimary)
if (connection is null)
{
success = true;
break;
endpoints = config.EndPoints.Clone();
connection = ConnectImpl(config, log, endpoints: endpoints);
}

connection.LastException = failure;
connection.Logger?.LogErrorSyncConnectTimeout(failure, failure.Message);
}

Thread.Sleep(100);
}
while (sw.ElapsedMilliseconds < config.ConnectTimeout);
// Attach to reconnect event to ensure proper connection to the new primary
connection.ConnectionRestored += OnManagedConnectionRestored;

if (!success)
{
throw new RedisConnectionException(
ConnectionFailureType.UnableToConnect,
CommandFlags.None,
$"Sentinel: Failed connecting to configured primary for service: {config.ServiceName}");
}
// If we lost the connection, run a switch to a least try and get updated info about the primary
connection.ConnectionFailed += OnManagedConnectionFailed;

// Attach to reconnect event to ensure proper connection to the new primary
connection.ConnectionRestored += OnManagedConnectionRestored;
lock (sentinelConnectionChildren)
{
sentinelConnectionChildren[serviceName] = connection;
}

// If we lost the connection, run a switch to a least try and get updated info about the primary
connection.ConnectionFailed += OnManagedConnectionFailed;
// Perform the initial switchover
var switchBlame = endpoints is { Count: > 0 } ? endpoints[0] : null;
try
{
SwitchPrimary(switchBlame, connection, log);
}
catch when (!success && !config.AbortOnConnectFail)
{
ScheduleSentinelPrimaryReconnect(connection, switchBlame);
}

lock (sentinelConnectionChildren)
return connection;
}
catch
{
sentinelConnectionChildren[serviceName] = connection;
if (connection is not null)
{
connection.ConnectionRestored -= OnManagedConnectionRestored;
connection.ConnectionFailed -= OnManagedConnectionFailed;
lock (sentinelConnectionChildren)
{
if (sentinelConnectionChildren.TryGetValue(serviceName, out var child) && ReferenceEquals(child, connection))
{
sentinelConnectionChildren.Remove(serviceName);
}
}
try { connection.Dispose(); } catch { }
}
throw;
}

// Perform the initial switchover
SwitchPrimary(endpoints[0], connection, log);

return connection;
}

[System.Diagnostics.CodeAnalysis.SuppressMessage("Roslynator", "RCS1075:Avoid empty catch clause that catches System.Exception.", Justification = "We don't care.")]
Expand Down Expand Up @@ -360,14 +427,19 @@ internal void OnManagedConnectionRestored(object? sender, ConnectionFailedEventA
}
}

[System.Diagnostics.CodeAnalysis.SuppressMessage("Roslynator", "RCS1075:Avoid empty catch clause that catches System.Exception.", Justification = "We don't care.")]
internal void OnManagedConnectionFailed(object? sender, ConnectionFailedEventArgs e)
{
if (sender is not ConnectionMultiplexer connection)
{
return; // This should never happen - called from non-nullable ConnectionFailedEventArgs
}

ScheduleSentinelPrimaryReconnect(connection, e.EndPoint);
}

[System.Diagnostics.CodeAnalysis.SuppressMessage("Roslynator", "RCS1075:Avoid empty catch clause that catches System.Exception.", Justification = "We don't care.")]
private void ScheduleSentinelPrimaryReconnect(ConnectionMultiplexer connection, EndPoint? endPoint)
{
// Periodically check to see if we can reconnect to the proper primary.
// This is here in case we lost our subscription to a good sentinel instance
// or if we miss the published primary change.
Expand All @@ -379,7 +451,7 @@ internal void OnManagedConnectionFailed(object? sender, ConnectionFailedEventArg
try
{
// Attempt, but do not fail here
SwitchPrimary(e.EndPoint, connection);
SwitchPrimary(endPoint, connection);
}
catch (Exception)
{
Expand Down
27 changes: 23 additions & 4 deletions src/StackExchange.Redis/ServerEndPoint.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ public void SetClusterConfiguration(ClusterConfiguration configuration)

public void UpdateNodeRelations(ClusterConfiguration configuration)
{
var thisNode = configuration.Nodes.FirstOrDefault(x => x.EndPoint?.Equals(EndPoint) == true);
var thisNode = GetClusterNode(configuration);
if (thisNode != null)
{
Multiplexer.Trace($"Updating node relations for {Format.ToString(thisNode.EndPoint)}...");
Expand All @@ -379,6 +379,9 @@ public void UpdateNodeRelations(ClusterConfiguration configuration)
}
}

private ClusterNode? GetClusterNode(ClusterConfiguration? configuration) =>
configuration?.Nodes.FirstOrDefault(x => x.EndPoint?.Equals(EndPoint) == true);

public void SetUnselectable(UnselectableFlags flags)
{
if (flags != 0)
Expand Down Expand Up @@ -502,7 +505,9 @@ internal async Task AutoConfigureAsync(PhysicalConnection? connection, ILogger?
await WriteDirectOrQueueFireAndForgetAsync(connection, msg, autoConfigProcessor).ForAwait();
}
}
else if (commandMap.IsAvailable(RedisCommand.SET) && !(helloPending || RoleKnownFromHello))
else if (commandMap.IsAvailable(RedisCommand.SET)
&& !(helloPending || RoleKnownFromHello)
&& !(ServerType == ServerType.Cluster && GetClusterNode(ClusterConfiguration) is not null))
{
// This is a nasty way to find if we are a replica, and it will only work on up-level servers, but...
// (note we only get here when HELLO isn't going to tell us: the HELLO reply carries "role", and
Expand Down Expand Up @@ -535,7 +540,9 @@ internal async Task AutoConfigureAsync(PhysicalConnection? connection, ILogger?
}
// If we are going to fetch a tie breaker, do so last and we'll get it in before the tracer fires completing the connection
// But if GETs are disabled on this, do not fail the connection - we just don't get tiebreaker benefits
if (Multiplexer.RawConfig.TryGetTieBreaker(out var tieBreakerKey) && Multiplexer.CommandMap.IsAvailable(RedisCommand.GET))
if (ServerType != ServerType.Cluster
&& Multiplexer.RawConfig.TryGetTieBreaker(out var tieBreakerKey)
&& Multiplexer.CommandMap.IsAvailable(RedisCommand.GET))
{
log?.LogInformationRequestingTieBreak(new(EndPoint), tieBreakerKey);
msg = Message.Create(0, flags, RedisCommand.GET, tieBreakerKey);
Expand Down Expand Up @@ -680,12 +687,24 @@ internal Message GetTracerMessage(bool checkResponse)
else
{
map.AssertAvailable(RedisCommand.EXISTS);
msg = Message.Create(0, flags, RedisCommand.EXISTS, (RedisValue)Multiplexer.UniqueId);
msg = Message.Create(0, flags, RedisCommand.EXISTS, GetTracerKey());
}
msg.SetInternalCall();
return msg;
}

internal RedisKey GetTracerKey()
{
RedisKey key = Multiplexer.UniqueId;
if (ServerType == ServerType.Cluster
&& GetClusterNode(ClusterConfiguration) is { } node
&& node.Slots.Count > 0)
{
key = key.Prepend(ServerSelectionStrategy.GetHashTagPrefix(node.Slots[0].From));
}
return key;
}

internal UnselectableFlags GetUnselectableFlags() => unselectableReasons;

internal bool IsSelectable(RedisCommand command, bool allowDisconnected = false)
Expand Down
Loading
Loading