diff --git a/docs/Failover.md b/docs/Failover.md index 58a4c43eb..4275a1bd4 100644 --- a/docs/Failover.md +++ b/docs/Failover.md @@ -380,7 +380,7 @@ ConnectionGroupMember[] members = [ ]; ``` -To disable periodic checking for the whole group, set `MultiGroupOptions.HealthCheckInterval` to `TimeSpan.MaxValue`; the group is then only re-evaluated in response to connection events (such as a tripped circuit-breaker). +To disable periodic checking for the whole group, set `MultiGroupOptions.HealthCheckInterval` to `TimeSpan.MaxValue`; the group is then only re-evaluated in response to connection events (a tripped circuit-breaker, or a member answering `LOADING`). ### Built-in Probes @@ -495,6 +495,8 @@ Where health checks *actively* probe each member on a timer, a **circuit breaker When a circuit breaker trips, it shuts the underlying physical connection down with `ConnectionFailureType.CircuitBreaker`. The connection then reconnects as usual, and - in a connection group - the health check and member-selection logic route traffic to other members until the affected member recovers. A circuit breaker is evaluated **per connection**, so its state is scoped to exactly the connection whose health it is measuring; a reconnect starts from a clean slate - but breaking the connection is sufficient for the group to notice non-availability. +A member that answers `-LOADING` (a server still loading its dataset after a restart) is treated the same way by a connection group: the connection is dropped with `ConnectionFailureType.Loading`, the group re-probes and re-selects immediately, and traffic moves to another member rather than queuing against one whose every reconnect will fail with the same error until the load completes. The health check brings the member back once it passes. + ### Configuring a Circuit Breaker for a Group Circuit breakers are configured for all members via `MultiGroupOptions`, alongside the health check. The setting flows into every member connection: diff --git a/src/StackExchange.Redis/Availability/MultiGroupMultiplexer.cs b/src/StackExchange.Redis/Availability/MultiGroupMultiplexer.cs index a426fdb78..82b8ebc4d 100644 --- a/src/StackExchange.Redis/Availability/MultiGroupMultiplexer.cs +++ b/src/StackExchange.Redis/Availability/MultiGroupMultiplexer.cs @@ -993,13 +993,17 @@ private static bool RemoveHandler(ref T? field, T? value) where T : Delegate } } - private int _circuitBreakerDebounce = 0; + private int _rerouteDebounce = 0; private void OnMemberConnectionFailed(object? sender, ConnectionFailedEventArgs e) { - // deliberately scoped to this one failure type; re-probe and re-select promptly rather - // than waiting for the next poll tick, so traffic routes away from the dropped member - if (e.FailureType is ConnectionFailureType.CircuitBreaker - && Interlocked.CompareExchange(ref _circuitBreakerDebounce, 1, 0) is 0) + // deliberately scoped to these failure types; re-probe and re-select promptly rather than + // waiting for the next poll tick, so traffic routes away from the dropped member. A breaker + // trip is the member's own traffic telling us it is unstable; LOADING is the server telling + // us outright that it cannot serve yet (and its handshake will keep failing the same way + // until it can, so nothing routed there in the meantime gets a reply - the breaker only ever + // sees replies, so it cannot be what moves us). Ordinary socket drops are left to reconnect + if (e.FailureType is ConnectionFailureType.CircuitBreaker or ConnectionFailureType.Loading + && Interlocked.CompareExchange(ref _rerouteDebounce, 1, 0) is 0) { GetMember(sender as ConnectionMultiplexer)?.SetUnhealthy(); _ = Task.Run(async () => @@ -1014,7 +1018,7 @@ private void OnMemberConnectionFailed(object? sender, ConnectionFailedEventArgs } finally { - Volatile.Write(ref _circuitBreakerDebounce, 0); + Volatile.Write(ref _rerouteDebounce, 0); } }); } diff --git a/tests/StackExchange.Redis.Tests/MultiGroupTests/LoadingRerouteTests.cs b/tests/StackExchange.Redis.Tests/MultiGroupTests/LoadingRerouteTests.cs new file mode 100644 index 000000000..e03a26d03 --- /dev/null +++ b/tests/StackExchange.Redis.Tests/MultiGroupTests/LoadingRerouteTests.cs @@ -0,0 +1,78 @@ +using System; +using System.Net; +using System.Threading.Tasks; +using StackExchange.Redis.Availability; +using Xunit; + +namespace StackExchange.Redis.Tests.MultiGroupTests; + +[RunPerProtocol] +public class LoadingRerouteTests(ITestOutputHelper log) : TestBase(log) +{ + // A member answering LOADING must steer the group away promptly, via the same ConnectionFailed fast-path + // as a circuit-breaker trip. The breaker cannot be what moves us here: once the server is loading, every + // reconnect fails during the handshake, so later commands die in the backlog without a reply, and the + // breaker only ever observes replies. As with the breaker test, the poll interval is enormous so that the + // fast-path is the only thing that can reroute within the test window, and the probe holds A down so the + // reroute is deterministic. + [Fact] + public async Task LoadingMember_ReroutesAwayPromptly() + { + EndPoint alpha = new DnsEndPoint("alpha", 6379); + EndPoint bravo = new DnsEndPoint("bravo", 6379); + + using var serverA = new InProcessTestServer(Output, endpoint: alpha); + using var serverB = new InProcessTestServer(Output, endpoint: bravo); + + var probe = new ControllableProbe(); + + ConnectionGroupMember[] members = + [ + new(serverA.GetClientConfig(), "A") { Weight = 9 }, // highest weight -> initially active + new(serverB.GetClientConfig(), "B") { Weight = 1 }, // failover target + ]; + + MultiGroupOptions options = new MultiGroupOptions.Builder + { + HealthCheckInterval = TimeSpan.FromMinutes(30), // enormous: the fast-path is what reroutes us + HealthCheck = new HealthCheck.Builder + { + Probe = probe, + ProbeCount = 1, + ProbeTimeout = TimeSpan.FromSeconds(5), + }, + }; + + await using var conn = await ConnectionMultiplexer.ConnectGroupAsync(members, options); + var typed = Assert.IsType(conn); + + var loadingEvents = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + typed.ConnectionFailed += (_, e) => + { + Log($"ConnectionFailed: {e.FailureType} @ {e.EndPoint}"); + if (e.FailureType == ConnectionFailureType.Loading) + { + loadingEvents.TrySetResult(true); + } + }; + + await GroupWait.AssertConnectedAsync(conn); + Assert.Same(members[0], conn.ActiveMember); + + // knock A into LOADING and hold it down; the next command to A is what surfaces the failure + probe.MarkDown(alpha); + serverA.IsLoading = true; + var fault = await Assert.ThrowsAnyAsync(() => conn.GetDatabase().PingAsync()); + Log($"observed fault: {fault.GetType().Name}: {fault.Message}"); + + var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(5); + while (DateTime.UtcNow < deadline && ReferenceEquals(conn.ActiveMember, members[0])) + { + await Task.Delay(50, TestContext.Current.CancellationToken); + } + Assert.Same(members[1], conn.ActiveMember); + + var completed = await Task.WhenAny(loadingEvents.Task, Task.Delay(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken)); + Assert.True(ReferenceEquals(completed, loadingEvents.Task), "expected a ConnectionFailed event with FailureType == Loading"); + } +} diff --git a/tests/StackExchange.Redis.Tests/RetryTests/RetryEndToEndTests.cs b/tests/StackExchange.Redis.Tests/RetryTests/RetryEndToEndTests.cs index aa9614027..0d49de0c8 100644 --- a/tests/StackExchange.Redis.Tests/RetryTests/RetryEndToEndTests.cs +++ b/tests/StackExchange.Redis.Tests/RetryTests/RetryEndToEndTests.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Net; using System.Threading; using System.Threading.Tasks; @@ -225,8 +225,9 @@ public async Task WithRetry_FailsOverBetweenGroupsOnLoading() } // The unhappy path of the failover threshold: the attempt count says "wait for a failover now", but no - // failover ever comes (the member stays nominally healthy - a couple of LOADING replies are not enough - // to trip the default breaker). We wait out FailoverDelay and then carry on retrying the original + // failover ever comes (the member stays nominally healthy: a couple of LOADING replies are not enough + // to trip the default breaker, and the re-probe they trigger finds the member healthy, so the group + // keeps it). We wait out FailoverDelay and then carry on retrying the original // member rather than giving up, and the group never moves. [Fact] public async Task WithRetry_WhenFailoverNeverArrives_KeepsRetryingSameMember()