make AA configuration API consistent

src/StackExchange.Redis/Availability/CircuitBreaker.cs

@@ -15,10 +15,24 @@ namespace StackExchange.Redis.Availability;
 [Experimental(Experiments.ActiveActive, UrlFormat = Experiments.UrlFormat)]
 public abstract class CircuitBreaker
 {
+    internal const double DefaultFailureRateThreshold = 10;
+    internal const int DefaultMinimumNumberOfFailures = 1000;
+    internal static readonly TimeSpan DefaultMetricsWindowSize = TimeSpan.FromSeconds(2);
+
+    private static readonly CircuitBreaker DefaultInstance = new DefaultCircuitBreaker(
+#pragma warning disable SA1114 // Parameter list should follow declaration - false positive: the #if directive splits the argument list
+#if NET8_0_OR_GREATER
+        null,
+#endif
+#pragma warning restore SA1114
+        DefaultFailureRateThreshold,
+        DefaultMinimumNumberOfFailures,
+        DefaultMetricsWindowSize);
+
     /// <summary>
-    /// Default circuit-breaker logic.
+    /// Default circuit-breaker logic: trips when the failure rate over a short rolling window crosses a threshold.
     /// </summary>
-    public static CircuitBreaker Default => Builder.DefaultInstance;
+    public static CircuitBreaker Default => DefaultInstance;
 
     /// <summary>
     /// No circuit-breaker logic is applied.
@@ -28,22 +42,9 @@ public abstract class CircuitBreaker
     /// <summary>
     /// Allows configuration of the default <see cref="CircuitBreaker"/> implementation.
     /// </summary>
-    public class Builder
+    [Experimental(Experiments.ActiveActive, UrlFormat = Experiments.UrlFormat)]
+    public sealed class Builder
     {
-        private const double DefaultFailureRateThreshold = 10;
-        private const int DefaultMinimumNumberOfFailures = 1000;
-        private static readonly TimeSpan DefaultMetricsWindowSize = TimeSpan.FromSeconds(2);
-
-        internal static CircuitBreaker DefaultInstance = new DefaultCircuitBreaker(
-#pragma warning disable SA1114 // Parameter list should follow declaration - false positive: the #if directive splits the argument list
-#if NET8_0_OR_GREATER
-            null,
-#endif
-#pragma warning restore SA1114
-            DefaultFailureRateThreshold,
-            DefaultMinimumNumberOfFailures,
-            DefaultMetricsWindowSize);
-
         /// <summary>
         /// Percentage of failures to trigger circuit breaker.
         /// </summary>
@@ -73,6 +74,11 @@ public class Builder
         /// </summary>
         public CircuitBreaker Create()
         {
+            if (FailureRateThreshold is < 0 or > 100) throw new ArgumentOutOfRangeException(nameof(FailureRateThreshold), FailureRateThreshold, "A percentage between 0 and 100 is required.");
+            if (MinimumNumberOfFailures < 1) throw new ArgumentOutOfRangeException(nameof(MinimumNumberOfFailures), MinimumNumberOfFailures, "At least one failure is required; use CircuitBreaker.None to disable circuit-breaking.");
+            if (MetricsWindowSize <= TimeSpan.Zero) throw new ArgumentOutOfRangeException(nameof(MetricsWindowSize), MetricsWindowSize, "A positive window is required.");
+            if (MetricsWindowSize.TotalSeconds > int.MaxValue) throw new ArgumentOutOfRangeException(nameof(MetricsWindowSize), MetricsWindowSize, "The window is too large.");
+
             if ((FailureRateThreshold is DefaultFailureRateThreshold
 #if NET8_0_OR_GREATER
                  & TimeProvider is null

src/StackExchange.Redis/Availability/DatabaseExtensions.cs

@@ -1,20 +1,36 @@
 using System.Diagnostics.CodeAnalysis;
 using RESPite;
 
-namespace StackExchange.Redis.Availability
+namespace StackExchange.Redis.Availability;
+
+/// <summary>
+///     Provides availability-related extension methods (such as <see cref="WithRetry"/>) to database instances.
+/// </summary>
+public static class DatabaseExtensions
 {
     /// <summary>
-    ///     Provides availability-related extension methods (such as <see cref="WithRetry"/>) to database instances.
+    /// Automatically retry operations when connection failure occurs. This has deep integration with
+    /// SE.Redis concepts, so can respond to server failover events, apply circuit-breaker rules, and
+    /// respect command effect categorization.
     /// </summary>
-    public static class DatabaseExtensions
+    /// <param name="database">The database to wrap.</param>
+    /// <param name="retryPolicy">
+    /// The policy to apply; when <see langword="null"/> (the default), the policy configured for the
+    /// underlying connection is used - <see cref="MultiGroupOptions.RetryPolicy"/> for a connection group,
+    /// <see cref="ConfigurationOptions.RetryPolicy"/> for a single connection, else
+    /// <see cref="RetryPolicy.Default"/>.
+    /// </param>
+    [Experimental(Experiments.ActiveActive, UrlFormat = Experiments.UrlFormat)]
+    public static IDatabaseAsync WithRetry(this IDatabaseAsync database, RetryPolicy? retryPolicy = null)
+        => new RetryDatabase(database, retryPolicy ?? ResolveRetryPolicy(database));
+
+    // IDatabaseAsync always exposes its multiplexer (via IRedisAsync), so the configured policy is reachable
+    // without the caller having to thread it through; note IConnectionMultiplexer is a public interface that
+    // callers may implement or mock, so every step here degrades to the default rather than assuming a type
+    private static RetryPolicy ResolveRetryPolicy(IDatabaseAsync database) => database.Multiplexer switch
     {
-        /// <summary>
-        /// Automatically retry operations when connection failure occurs. This has deep integration with
-        /// SE.Redis concepts, so can respond to server failover events, apply circuit-breaker rules, and
-        /// respect command effect categorization.
-        /// </summary>
-        [Experimental(Experiments.ActiveActive, UrlFormat = Experiments.UrlFormat)]
-        public static IDatabaseAsync WithRetry(this IDatabaseAsync database, RetryPolicy retryPolicy)
-            => new RetryDatabase(database, retryPolicy);
-    }
+        IConnectionGroup group => group.Options.RetryPolicy,
+        IInternalConnectionMultiplexer muxer => muxer.RawConfig.RetryPolicy ?? RetryPolicy.Default,
+        _ => RetryPolicy.Default,
+    };
 }

src/StackExchange.Redis/Availability/HealthCheck.Execute.cs

@@ -11,7 +11,9 @@ public sealed partial class HealthCheck
     /// Evaluate the health of the specified multiplexer, by evaluating all endpoints.
     /// </summary>
     public Task<HealthCheckResult> CheckHealthAsync(IConnectionMultiplexer multiplexer)
-        => multiplexer.IsConnected ? CheckHealthCoreAsync(multiplexer) : HealthCheckProbe.UnhealthyTask;
+        => !IsEnabled ? HealthCheckProbe.InconclusiveTask
+        : multiplexer.IsConnected ? CheckHealthCoreAsync(multiplexer)
+        : HealthCheckProbe.UnhealthyTask;
 
     private async Task<HealthCheckResult> CheckHealthCoreAsync(IConnectionMultiplexer multiplexer)
     {
@@ -51,21 +53,32 @@ private async Task<HealthCheckResult> CheckHealthCoreAsync(IConnectionMultiplexe
 
     internal int TotalTimeoutMillis()
     {
-        int count = ProbeCount;
-        if (count <= 0)
-        {
-            Debug.Fail("We shouldn't get as far as calculating timeouts with a non-positive probe count.");
-            return 0;
-        }
+        bool valid = TryComputeTotalTimeoutMillis(ProbeCount, ProbeTimeout, ProbeInterval, out int millis);
+        Debug.Assert(valid, "The probe budget is validated by HealthCheck.Builder.Create, so should always be usable here.");
+        return millis;
+    }
 
-        TimeSpan probeTimeout = ProbeTimeout, probeInterval = ProbeInterval;
+    // the total time budget for a full health check, in milliseconds; shared with Builder.Create, which uses
+    // it to reject a configuration whose budget cannot be expressed (rather than overflowing at check time)
+    private static bool TryComputeTotalTimeoutMillis(int probeCount, TimeSpan probeTimeout, TimeSpan probeInterval, out int millis)
+    {
+        millis = 0;
+        if (probeCount < 1 || probeTimeout <= TimeSpan.Zero || probeInterval < TimeSpan.Zero) return false;
 
-        // the first probe doesn't have an interval before it, the rest do
-        var totalTicks = probeTimeout.Ticks
-            + ((probeTimeout.Ticks + probeInterval.Ticks) * (count - 1));
-        var millis = (int)TimeSpan.FromTicks(totalTicks).TotalMilliseconds;
-        Debug.Assert(millis > 0, "Total timeout should be positive");
-        return millis;
+        try
+        {
+            // the first probe doesn't have an interval before it, the rest do
+            long totalTicks = checked(probeTimeout.Ticks + ((probeTimeout.Ticks + probeInterval.Ticks) * (probeCount - 1)));
+            long totalMillis = totalTicks / TimeSpan.TicksPerMillisecond;
+            if (totalMillis is <= 0 or > int.MaxValue) return false;
+
+            millis = (int)totalMillis;
+            return true;
+        }
+        catch (OverflowException)
+        {
+            return false;
+        }
     }
 
     // apply timeout and collation logic to a group of probes
@@ -130,19 +143,22 @@ internal static void PutReusablePending(ref Task<HealthCheckResult>[]? field, re
     /// Evaluate the health of an endpoint.
     /// </summary>
     public Task<HealthCheckResult> CheckHealthAsync(IServer server)
-        => server.IsConnected ? CheckHealthCoreAsync(server) : HealthCheckProbe.UnhealthyTask;
+        => !IsEnabled ? HealthCheckProbe.InconclusiveTask
+        : server.IsConnected ? CheckHealthCoreAsync(server)
+        : HealthCheckProbe.UnhealthyTask;
 
     private async Task<HealthCheckResult> CheckHealthCoreAsync(IServer server)
     {
         try
         {
             int timeout = (int)ProbeTimeout.TotalMilliseconds, success = 0, failure = 0, remaining = ProbeCount;
+            HealthCheckContext context = new(server, ProbeTimeout);
             while (remaining > 0)
             {
                 HealthCheckResult probeResult;
                 try
                 {
-                    var pendingProbe = Probe.CheckHealthAsync(this, server);
+                    var pendingProbe = Probe.CheckHealthAsync(context);
                     probeResult = await pendingProbe.TimeoutAfter(timeout).ForAwait()
                         ? await pendingProbe.ForAwait() // completed
                         : HealthCheckResult.Unhealthy; // timeout