From 9e562ef5e29d0bb04fa614983d8ea7ac43860ee7 Mon Sep 17 00:00:00 2001 From: martintmk Date: Sun, 23 Apr 2023 16:03:16 +0200 Subject: [PATCH 1/5] Implement Advanced Circuit Breaker --- .../Benchmarks/CircuitBreakerBenchmark.cs | 24 ++++ .../Internals/Helper.CircuitBreaker.cs | 38 ++++++ .../Internals/Helper.Pipeline.cs | 23 +++- src/Polly.Core.Benchmarks/Program.cs | 5 +- src/Polly.Core.Benchmarks/README.md | 17 ++- ...itBreakerResilienceStrategyBuilderTests.cs | 41 ++++++- .../AdvancedCircuitBehaviorTests.cs | 76 ++++++++++-- .../Health/HealthMetricsTests.cs | 26 ++++ .../Health/RollingHealthMetricsTests.cs | 114 ++++++++++++++++++ .../Health/SingleHealthMetricsTests.cs | 60 +++++++++ .../Helpers/FakeTimeProvider.cs | 22 +++- .../AdvancedCircuitBreakerStrategyOptions.cs | 4 +- .../BaseCircuitBreakerStrategyOptions.cs | 2 +- .../CircuitBreakerResilienceStrategy.cs | 1 + ...akerResilienceStrategyBuilderExtensions.cs | 3 +- .../Controller/AdvancedCircuitBehavior.cs | 40 +++++- .../CircuitBreaker/Health/HealthInfo.cs | 15 +++ .../CircuitBreaker/Health/HealthMetrics.cs | 26 ++++ .../Health/RollingHealthMetrics.cs | 72 +++++++++++ .../Health/SingleHealthMetrics.cs | 53 ++++++++ 20 files changed, 621 insertions(+), 41 deletions(-) create mode 100644 src/Polly.Core.Benchmarks/Benchmarks/CircuitBreakerBenchmark.cs create mode 100644 src/Polly.Core.Benchmarks/Internals/Helper.CircuitBreaker.cs create mode 100644 src/Polly.Core.Tests/CircuitBreaker/Health/HealthMetricsTests.cs create mode 100644 src/Polly.Core.Tests/CircuitBreaker/Health/RollingHealthMetricsTests.cs create mode 100644 src/Polly.Core.Tests/CircuitBreaker/Health/SingleHealthMetricsTests.cs create mode 100644 src/Polly.Core/CircuitBreaker/Health/HealthInfo.cs create mode 100644 src/Polly.Core/CircuitBreaker/Health/HealthMetrics.cs create mode 100644 src/Polly.Core/CircuitBreaker/Health/RollingHealthMetrics.cs create mode 100644 src/Polly.Core/CircuitBreaker/Health/SingleHealthMetrics.cs diff --git a/src/Polly.Core.Benchmarks/Benchmarks/CircuitBreakerBenchmark.cs b/src/Polly.Core.Benchmarks/Benchmarks/CircuitBreakerBenchmark.cs new file mode 100644 index 00000000000..34b5dd6047b --- /dev/null +++ b/src/Polly.Core.Benchmarks/Benchmarks/CircuitBreakerBenchmark.cs @@ -0,0 +1,24 @@ +using System.Threading.Tasks; +using BenchmarkDotNet.Attributes; +using Polly.Core.Benchmarks; + +namespace Polly.Benchmarks; + +public class CircuitBreakerBenchmark +{ + private object? _strategyV7; + private object? _strategyV8; + + [GlobalSetup] + public void Setup() + { + _strategyV7 = Helper.CreateCircuitBreaker(PollyVersion.V7); + _strategyV8 = Helper.CreateCircuitBreaker(PollyVersion.V8); + } + + [Benchmark(Baseline = true)] + public ValueTask ExecuteCircuitBreaker_V7() => _strategyV7!.ExecuteAsync(PollyVersion.V7); + + [Benchmark] + public ValueTask ExecuteCircuitBreaker_V8() => _strategyV8!.ExecuteAsync(PollyVersion.V8); +} diff --git a/src/Polly.Core.Benchmarks/Internals/Helper.CircuitBreaker.cs b/src/Polly.Core.Benchmarks/Internals/Helper.CircuitBreaker.cs new file mode 100644 index 00000000000..b46cf438858 --- /dev/null +++ b/src/Polly.Core.Benchmarks/Internals/Helper.CircuitBreaker.cs @@ -0,0 +1,38 @@ +#pragma warning disable S4225 // Extension methods should not extend "object" + +using System; +using Polly.CircuitBreaker; + +namespace Polly.Core.Benchmarks; + +internal static partial class Helper +{ + public static object CreateCircuitBreaker(PollyVersion technology) + { + var delay = TimeSpan.FromSeconds(10); + + return technology switch + { + PollyVersion.V7 => + Policy + .HandleResult(10) + .Or() + .AdvancedCircuitBreakerAsync(0.5, TimeSpan.FromSeconds(30), 10, TimeSpan.FromSeconds(5)), + + PollyVersion.V8 => CreateStrategy(builder => + { + var options = new AdvancedCircuitBreakerStrategyOptions + { + FailureThreshold = 0.5, + SamplingDuration = TimeSpan.FromSeconds(30), + MinimumThroughput = 10, + BreakDuration = TimeSpan.FromSeconds(5), + }; + + options.ShouldHandle.HandleOutcome((outcome, _) => outcome.Result == 10 || outcome.Exception is InvalidOperationException); + builder.AddAdvancedCircuitBreaker(options); + }), + _ => throw new NotSupportedException() + }; + } +} diff --git a/src/Polly.Core.Benchmarks/Internals/Helper.Pipeline.cs b/src/Polly.Core.Benchmarks/Internals/Helper.Pipeline.cs index e499d1aeb65..30ac5c7751c 100644 --- a/src/Polly.Core.Benchmarks/Internals/Helper.Pipeline.cs +++ b/src/Polly.Core.Benchmarks/Internals/Helper.Pipeline.cs @@ -1,5 +1,6 @@ using System.Threading.RateLimiting; using Polly; +using Polly.Strategy; namespace Polly.Core.Benchmarks; @@ -8,24 +9,34 @@ internal static partial class Helper public static object CreateStrategyPipeline(PollyVersion technology) => technology switch { PollyVersion.V7 => Policy.WrapAsync( + Policy.HandleResult(10).Or().AdvancedCircuitBreakerAsync(0.5, TimeSpan.FromSeconds(30), 10, TimeSpan.FromSeconds(5)), Policy.TimeoutAsync(TimeSpan.FromSeconds(1)), - Policy.Handle().OrResult(10).WaitAndRetryAsync(3, attempt => TimeSpan.FromSeconds(1)), + Policy.Handle().OrResult(10).WaitAndRetryAsync(3, attempt => TimeSpan.FromSeconds(1)), Policy.TimeoutAsync(TimeSpan.FromSeconds(10)), Policy.BulkheadAsync(10, 10)), PollyVersion.V8 => CreateStrategy(builder => { builder - .AddTimeout(TimeSpan.FromSeconds(1)) + .AddConcurrencyLimiter(new ConcurrencyLimiterOptions + { + QueueLimit = 10, + PermitLimit = 10 + }) + .AddTimeout(TimeSpan.FromSeconds(10)) .AddRetry( predicate => predicate.HandleException().HandleResult(10), RetryBackoffType.Constant, 3, TimeSpan.FromSeconds(1)) - .AddTimeout(TimeSpan.FromSeconds(10)) - .AddConcurrencyLimiter(new ConcurrencyLimiterOptions + .AddTimeout(TimeSpan.FromSeconds(1)) + .AddAdvancedCircuitBreaker(new AdvancedCircuitBreakerStrategyOptions { - QueueLimit = 10, - PermitLimit = 10 + FailureThreshold = 0.5, + SamplingDuration = TimeSpan.FromSeconds(30), + MinimumThroughput = 10, + BreakDuration = TimeSpan.FromSeconds(5), + ShouldHandle = new OutcomePredicate() + .HandleOutcome((outcome, _) => outcome.Result == 10 || outcome.Exception is InvalidOperationException) }); }), _ => throw new NotSupportedException() diff --git a/src/Polly.Core.Benchmarks/Program.cs b/src/Polly.Core.Benchmarks/Program.cs index 5b9850ebe8a..4f5dea3cbbe 100644 --- a/src/Polly.Core.Benchmarks/Program.cs +++ b/src/Polly.Core.Benchmarks/Program.cs @@ -8,6 +8,7 @@ var config = ManualConfig .Create(DefaultConfig.Instance) .AddJob(Job.MediumRun.WithToolchain(InProcessEmitToolchain.Instance)) - .AddDiagnoser(MemoryDiagnoser.Default); + .AddDiagnoser(MemoryDiagnoser.Default) + .WithOption(ConfigOptions.DisableOptimizationsValidator, true); -BenchmarkRunner.Run(typeof(PollyVersion).Assembly, config); +BenchmarkSwitcher.FromAssembly(typeof(PollyVersion).Assembly).Run(args, config); diff --git a/src/Polly.Core.Benchmarks/README.md b/src/Polly.Core.Benchmarks/README.md index 705a1a5c615..fc24c8c6126 100644 --- a/src/Polly.Core.Benchmarks/README.md +++ b/src/Polly.Core.Benchmarks/README.md @@ -47,9 +47,16 @@ LaunchCount=2 WarmupCount=10 | ExecuteRateLimiter_V7 | 190.8 ns | 10.01 ns | 14.98 ns | 1.00 | 0.00 | 0.0448 | 376 B | 1.00 | | ExecuteRateLimiter_V8 | 199.6 ns | 2.54 ns | 3.64 ns | 1.05 | 0.09 | 0.0048 | 40 B | 0.11 | -## STRATEGY PIPELINE (TIMEOUT + RETRY + TIMEOUT + RATE LIMITER) +## CIRCUIT BREAKER -| Method | Mean | Error | StdDev | Ratio | RatioSD | Gen0 | Allocated | Alloc Ratio | -|--------------------------- |---------:|----------:|----------:|------:|--------:|-------:|----------:|------------:| -| ExecuteStrategyPipeline_V7 | 1.265 us | 0.0372 us | 0.0558 us | 1.00 | 0.00 | 0.2861 | 2400 B | 1.00 | -| ExecuteStrategyPipeline_V8 | 1.032 us | 0.0165 us | 0.0236 us | 0.82 | 0.04 | 0.0076 | 64 B | 0.03 | +| Method | Mean | Error | StdDev | Ratio | RatioSD | Gen0 | Allocated | Alloc Ratio | +|------------------------- |---------:|--------:|--------:|------:|--------:|-------:|----------:|------------:| +| ExecuteCircuitBreaker_V7 | 198.4 ns | 2.78 ns | 3.99 ns | 1.00 | 0.00 | 0.0629 | 528 B | 1.00 | +| ExecuteCircuitBreaker_V8 | 297.9 ns | 2.63 ns | 3.77 ns | 1.50 | 0.04 | 0.0038 | 32 B | 0.06 | + +## STRATEGY PIPELINE (RATE LIMITER + TIMEOUT + RETRY + TIMEOUT + CIRCUIT BREAKER) + +| Method | Mean | Error | StdDev | Ratio | Gen0 | Allocated | Alloc Ratio | +|--------------------------- |---------:|----------:|----------:|------:|-------:|----------:|------------:| +| ExecuteStrategyPipeline_V7 | 1.523 us | 0.0092 us | 0.0137 us | 1.00 | 0.3433 | 2872 B | 1.00 | +| ExecuteStrategyPipeline_V8 | 1.276 us | 0.0128 us | 0.0191 us | 0.84 | 0.0114 | 96 B | 0.03 | diff --git a/src/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResilienceStrategyBuilderTests.cs b/src/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResilienceStrategyBuilderTests.cs index c31f8ad75ab..e20acebfe2e 100644 --- a/src/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResilienceStrategyBuilderTests.cs +++ b/src/Polly.Core.Tests/CircuitBreaker/CircuitBreakerResilienceStrategyBuilderTests.cs @@ -128,22 +128,53 @@ public void AddCircuitBreaker_IntegrationTest() [Fact] public void AddAdvancedCircuitBreaker_IntegrationTest() { + int opened = 0; + int closed = 0; + int halfOpened = 0; + var options = new AdvancedCircuitBreakerStrategyOptions { - BreakDuration = TimeSpan.FromMilliseconds(500), + FailureThreshold = 0.5, + MinimumThroughput = 10, + SamplingDuration = TimeSpan.FromSeconds(10), + BreakDuration = TimeSpan.FromSeconds(1), }; options.ShouldHandle.HandleResult(-1); - options.OnOpened.Register(() => { }); - options.OnClosed.Register(() => { }); - options.OnHalfOpened.Register(() => { }); + options.OnOpened.Register(() => opened++); + options.OnClosed.Register(() => closed++); + options.OnHalfOpened.Register(() => halfOpened++); var timeProvider = new FakeTimeProvider(); var strategy = new ResilienceStrategyBuilder { TimeProvider = timeProvider.Object }.AddAdvancedCircuitBreaker(options).Build(); var time = DateTime.UtcNow; timeProvider.Setup(v => v.UtcNow).Returns(() => time); - strategy.Should().BeOfType(); + for (int i = 0; i < 10; i++) + { + strategy.Execute(_ => -1); + } + + // Circuit opened + opened.Should().Be(1); + halfOpened.Should().Be(0); + closed.Should().Be(0); + Assert.Throws>(() => strategy.Execute(_ => 0)); + + // Circuit Half Opened + time += options.BreakDuration; + strategy.Execute(_ => -1); + Assert.Throws>(() => strategy.Execute(_ => 0)); + opened.Should().Be(2); + halfOpened.Should().Be(1); + closed.Should().Be(0); + + // Now close it + time += options.BreakDuration; + strategy.Execute(_ => 0); + opened.Should().Be(2); + halfOpened.Should().Be(2); + closed.Should().Be(1); } [Fact] diff --git a/src/Polly.Core.Tests/CircuitBreaker/Controller/AdvancedCircuitBehaviorTests.cs b/src/Polly.Core.Tests/CircuitBreaker/Controller/AdvancedCircuitBehaviorTests.cs index 21ecc5c1a48..c671acd4d7b 100644 --- a/src/Polly.Core.Tests/CircuitBreaker/Controller/AdvancedCircuitBehaviorTests.cs +++ b/src/Polly.Core.Tests/CircuitBreaker/Controller/AdvancedCircuitBehaviorTests.cs @@ -1,22 +1,72 @@ +using Moq; using Polly.CircuitBreaker; +using Polly.CircuitBreaker.Health; +using Polly.Utils; namespace Polly.Core.Tests.CircuitBreaker.Controller; + public class AdvancedCircuitBehaviorTests { + private Mock _metrics = new(MockBehavior.Strict, TimeProvider.System); + + [InlineData(10, 10, 0.0, 0.1, false)] + [InlineData(10, 10, 0.1, 0.1, true)] + [InlineData(10, 10, 0.2, 0.1, true)] + [InlineData(11, 10, 0.2, 0.1, true)] + [InlineData(9, 10, 0.1, 0.1, false)] + [Theory] + public void OnActionFailure_WhenClosed_EnsureCorrectBehavior( + int throughput, + int minimumThruput, + double failureRate, + double failureThreshold, + bool expectedShouldBreak) + { + _metrics.Setup(m => m.IncrementFailure()); + _metrics.Setup(m => m.GetHealthInfo()).Returns(new HealthInfo(throughput, failureRate)); + + var behavior = new AdvancedCircuitBehavior(new AdvancedCircuitBreakerStrategyOptions { MinimumThroughput = minimumThruput, FailureThreshold = failureThreshold }, _metrics.Object); + + behavior.OnActionFailure(CircuitState.Closed, out var shouldBreak); + + shouldBreak.Should().Be(expectedShouldBreak); + _metrics.VerifyAll(); + } + + [InlineData(CircuitState.Closed, true)] + [InlineData(CircuitState.Open, true)] + [InlineData(CircuitState.Isolated, true)] + [InlineData(CircuitState.HalfOpen, false)] + [Theory] + public void OnActionFailure_State_EnsureCorrectCalls(CircuitState state, bool shouldIncrementFailure) + { + _metrics = new(MockBehavior.Loose, TimeProvider.System); + + var sut = Create(); + + sut.OnActionFailure(state, out var shouldBreak); + + shouldBreak.Should().BeFalse(); + if (shouldIncrementFailure) + { + _metrics.Verify(v => v.IncrementFailure(), Times.Once()); + } + else + { + _metrics.Verify(v => v.IncrementFailure(), Times.Never()); + } + } + [Fact] - public void HappyPath() + public void OnCircuitClosed_Ok() { - var behavior = new AdvancedCircuitBehavior(); - - behavior - .Invoking(b => - { - behavior.OnActionFailure(CircuitState.Closed, out var shouldBreak); - shouldBreak.Should().BeFalse(); - behavior.OnCircuitClosed(); - behavior.OnActionSuccess(CircuitState.Closed); - }) - .Should() - .NotThrow(); + _metrics = new(MockBehavior.Loose, TimeProvider.System); + var sut = Create(); + + sut.OnCircuitClosed(); + + _metrics.Verify(v => v.Reset(), Times.Once()); } + + private AdvancedCircuitBehavior Create() => new(new AdvancedCircuitBreakerStrategyOptions(), _metrics.Object); } diff --git a/src/Polly.Core.Tests/CircuitBreaker/Health/HealthMetricsTests.cs b/src/Polly.Core.Tests/CircuitBreaker/Health/HealthMetricsTests.cs new file mode 100644 index 00000000000..db1d3e5d2de --- /dev/null +++ b/src/Polly.Core.Tests/CircuitBreaker/Health/HealthMetricsTests.cs @@ -0,0 +1,26 @@ +using System; +using Polly.CircuitBreaker; +using Polly.CircuitBreaker.Health; +using Polly.Utils; + +namespace Polly.Core.Tests.CircuitBreaker.Health; + +public class HealthMetricsTests +{ + [InlineData(100, typeof(SingleHealthMetrics))] + [InlineData(199, typeof(SingleHealthMetrics))] + [InlineData(200, typeof(RollingHealthMetrics))] + [InlineData(201, typeof(RollingHealthMetrics))] + [Theory] + public void Create_Ok(int samplingDurationMs, Type expectedType) + { + HealthMetrics.Create( + new AdvancedCircuitBreakerStrategyOptions + { + SamplingDuration = TimeSpan.FromMilliseconds(samplingDurationMs) + }, + TimeProvider.System) + .Should() + .BeOfType(expectedType); + } +} diff --git a/src/Polly.Core.Tests/CircuitBreaker/Health/RollingHealthMetricsTests.cs b/src/Polly.Core.Tests/CircuitBreaker/Health/RollingHealthMetricsTests.cs new file mode 100644 index 00000000000..1f2d6ae7550 --- /dev/null +++ b/src/Polly.Core.Tests/CircuitBreaker/Health/RollingHealthMetricsTests.cs @@ -0,0 +1,114 @@ +using Polly.CircuitBreaker.Health; + +namespace Polly.Core.Tests.CircuitBreaker.Health; +public class RollingHealthMetricsTests +{ + private readonly FakeTimeProvider _timeProvider; + private readonly TimeSpan _samplingDuration = TimeSpan.FromSeconds(10); + private readonly short _windows = 10; + + public RollingHealthMetricsTests() => _timeProvider = new FakeTimeProvider().SetupUtcNow(); + + [Fact] + public void Ctor_EnsureDefaults() + { + var metrics = Create(); + var health = metrics.GetHealthInfo(); + health.FailureRate.Should().Be(0); + health.Throughput.Should().Be(0); + } + + [Fact] + public void Increment_Ok() + { + var metrics = Create(); + + metrics.IncrementFailure(); + metrics.IncrementSuccess(); + metrics.IncrementSuccess(); + metrics.IncrementSuccess(); + metrics.IncrementSuccess(); + + var health = metrics.GetHealthInfo(); + + health.FailureRate.Should().Be(0.2); + health.Throughput.Should().Be(5); + } + + [Fact] + public void GetHealthInfo_EnsureWindowRespected() + { + var metrics = Create(); + var health = new List(); + + for (int i = 0; i < 5; i++) + { + if (i < 2) + { + metrics.IncrementFailure(); + } + else + { + metrics.IncrementSuccess(); + } + + metrics.IncrementSuccess(); + _timeProvider.AdvanceTime(TimeSpan.FromSeconds(2)); + health.Add(metrics.GetHealthInfo()); + } + + _timeProvider.AdvanceTime(TimeSpan.FromSeconds(2)); + health.Add(metrics.GetHealthInfo()); + + health[0].Should().Be(new HealthInfo(2, 0.5)); + health[1].Should().Be(new HealthInfo(4, 0.5)); + health[3].Should().Be(new HealthInfo(8, 0.25)); + health[4].Should().Be(new HealthInfo(8, 0.125)); + health[5].Should().Be(new HealthInfo(6, 0.0)); + } + + [Fact] + public void GetHealthInfo_EnsureWindowCapacityRespected() + { + var delay = TimeSpan.FromSeconds(1); + var metrics = Create(); + + for (int i = 0; i < _windows; i++) + { + metrics.IncrementSuccess(); + _timeProvider.AdvanceTime(delay); + } + + metrics.GetHealthInfo().Throughput.Should().Be(9); + _timeProvider.AdvanceTime(delay); + metrics.GetHealthInfo().Throughput.Should().Be(8); + } + + [Fact] + public void Reset_Ok() + { + var metrics = Create(); + + metrics.IncrementSuccess(); + metrics.Reset(); + + metrics.GetHealthInfo().Throughput.Should().Be(0); + } + + [InlineData(true)] + [InlineData(false)] + [Theory] + public void GetHealthInfo_SamplingDurationRespected(bool variance) + { + var metrics = Create(); + + metrics.IncrementSuccess(); + metrics.IncrementSuccess(); + + _timeProvider.AdvanceTime(_samplingDuration + (variance ? TimeSpan.FromMilliseconds(1) : TimeSpan.Zero)); + + metrics.GetHealthInfo().Should().Be(new HealthInfo(0, 0)); + } + + private RollingHealthMetrics Create() => new(_samplingDuration, _windows, _timeProvider.Object); +} diff --git a/src/Polly.Core.Tests/CircuitBreaker/Health/SingleHealthMetricsTests.cs b/src/Polly.Core.Tests/CircuitBreaker/Health/SingleHealthMetricsTests.cs new file mode 100644 index 00000000000..0057accf81e --- /dev/null +++ b/src/Polly.Core.Tests/CircuitBreaker/Health/SingleHealthMetricsTests.cs @@ -0,0 +1,60 @@ +using Polly.CircuitBreaker.Health; + +namespace Polly.Core.Tests.CircuitBreaker.Health; +public class SingleHealthMetricsTests +{ + private readonly FakeTimeProvider _timeProvider; + + public SingleHealthMetricsTests() => _timeProvider = new FakeTimeProvider().SetupUtcNow(); + + [Fact] + public void Ctor_EnsureDefaults() + { + var metrics = new SingleHealthMetrics(TimeSpan.FromMilliseconds(100), _timeProvider.Object); + var health = metrics.GetHealthInfo(); + + health.FailureRate.Should().Be(0); + health.Throughput.Should().Be(0); + } + + [Fact] + public void Increment_Ok() + { + var metrics = new SingleHealthMetrics(TimeSpan.FromMilliseconds(100), _timeProvider.Object); + + metrics.IncrementFailure(); + metrics.IncrementSuccess(); + metrics.IncrementSuccess(); + metrics.IncrementSuccess(); + metrics.IncrementSuccess(); + + var health = metrics.GetHealthInfo(); + + health.FailureRate.Should().Be(0.2); + health.Throughput.Should().Be(5); + } + + [Fact] + public void Reset_Ok() + { + var metrics = new SingleHealthMetrics(TimeSpan.FromMilliseconds(100), _timeProvider.Object); + + metrics.IncrementSuccess(); + metrics.Reset(); + + metrics.GetHealthInfo().Throughput.Should().Be(0); + } + + [Fact] + public void SamplingDurationRespected_Ok() + { + var metrics = new SingleHealthMetrics(TimeSpan.FromMilliseconds(100), _timeProvider.Object); + + metrics.IncrementSuccess(); + metrics.IncrementSuccess(); + + _timeProvider.AdvanceTime(TimeSpan.FromMilliseconds(100)); + + metrics.GetHealthInfo().Throughput.Should().Be(0); + } +} diff --git a/src/Polly.Core.Tests/Helpers/FakeTimeProvider.cs b/src/Polly.Core.Tests/Helpers/FakeTimeProvider.cs index f781c810e14..969d6992555 100644 --- a/src/Polly.Core.Tests/Helpers/FakeTimeProvider.cs +++ b/src/Polly.Core.Tests/Helpers/FakeTimeProvider.cs @@ -5,8 +5,10 @@ namespace Polly.Core.Tests.Helpers; internal class FakeTimeProvider : Mock { + private DateTimeOffset? _time; + public FakeTimeProvider(long frequency) - : base(MockBehavior.Strict, frequency) + : base(MockBehavior.Strict, frequency) { } @@ -15,6 +17,24 @@ public FakeTimeProvider() { } + public FakeTimeProvider SetupUtcNow(DateTimeOffset? time = null) + { + _time = time ?? DateTimeOffset.UtcNow; + Setup(x => x.UtcNow).Returns(() => _time.Value); + return this; + } + + public FakeTimeProvider AdvanceTime(TimeSpan time) + { + if (_time == null) + { + SetupUtcNow(DateTimeOffset.UtcNow); + } + + _time = _time!.Value.Add(time); + return this; + } + public FakeTimeProvider SetupAnyDelay(CancellationToken cancellationToken = default) { Setup(x => x.Delay(It.IsAny(), cancellationToken)).Returns(Task.CompletedTask); diff --git a/src/Polly.Core/CircuitBreaker/AdvancedCircuitBreakerStrategyOptions.cs b/src/Polly.Core/CircuitBreaker/AdvancedCircuitBreakerStrategyOptions.cs index 3714a7ead00..03dfaf618f4 100644 --- a/src/Polly.Core/CircuitBreaker/AdvancedCircuitBreakerStrategyOptions.cs +++ b/src/Polly.Core/CircuitBreaker/AdvancedCircuitBreakerStrategyOptions.cs @@ -45,8 +45,8 @@ public class AdvancedCircuitBreakerStrategyOptions : BaseCircuitBreakerStrategyO /// Gets or sets the duration of the sampling over which failure ratios are assessed. /// /// - /// Value must be greater than 0.5 seconds. Defaults to 30 seconds. + /// Value must be greater than 20 milliseconds. Defaults to 30 seconds. /// - [TimeSpan("00:00:00.500")] + [TimeSpan("00:00:00.020")] public TimeSpan SamplingDuration { get; set; } = CircuitBreakerConstants.DefaultSamplingDuration; } diff --git a/src/Polly.Core/CircuitBreaker/BaseCircuitBreakerStrategyOptions.cs b/src/Polly.Core/CircuitBreaker/BaseCircuitBreakerStrategyOptions.cs index 7ebabaed0f2..aceed1d7b39 100644 --- a/src/Polly.Core/CircuitBreaker/BaseCircuitBreakerStrategyOptions.cs +++ b/src/Polly.Core/CircuitBreaker/BaseCircuitBreakerStrategyOptions.cs @@ -18,7 +18,7 @@ public abstract class BaseCircuitBreakerStrategyOptions : ResilienceStrategyOpti /// /// Initializes a new instance of the class. /// - protected BaseCircuitBreakerStrategyOptions() => StrategyType = CircuitBreakerConstants.StrategyType; + private protected BaseCircuitBreakerStrategyOptions() => StrategyType = CircuitBreakerConstants.StrategyType; /// /// Gets or sets the duration of break the circuit will stay open before resetting. diff --git a/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs b/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs index 667b8a2a600..7346a0dece5 100644 --- a/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs +++ b/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs @@ -1,3 +1,4 @@ +using System.Runtime.CompilerServices; using Polly.Strategy; namespace Polly.CircuitBreaker; diff --git a/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategyBuilderExtensions.cs b/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategyBuilderExtensions.cs index 2a4cc556c5d..07a98011fd3 100644 --- a/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategyBuilderExtensions.cs +++ b/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategyBuilderExtensions.cs @@ -1,4 +1,5 @@ using Polly.CircuitBreaker; +using Polly.CircuitBreaker.Health; using Polly.Strategy; namespace Polly; @@ -104,7 +105,7 @@ internal static ResilienceStrategyBuilder AddCircuitBreakerCore(this ResilienceS { CircuitBehavior behavior = options switch { - AdvancedCircuitBreakerStrategyOptions => new AdvancedCircuitBehavior(), + AdvancedCircuitBreakerStrategyOptions o => new AdvancedCircuitBehavior(o, HealthMetrics.Create(o, context.TimeProvider)), CircuitBreakerStrategyOptions o => new ConsecutiveFailuresCircuitBehavior(o), _ => throw new NotSupportedException() }; diff --git a/src/Polly.Core/CircuitBreaker/Controller/AdvancedCircuitBehavior.cs b/src/Polly.Core/CircuitBreaker/Controller/AdvancedCircuitBehavior.cs index c5c5e92d266..3fcb8c87a20 100644 --- a/src/Polly.Core/CircuitBreaker/Controller/AdvancedCircuitBehavior.cs +++ b/src/Polly.Core/CircuitBreaker/Controller/AdvancedCircuitBehavior.cs @@ -1,18 +1,48 @@ +using Polly.CircuitBreaker.Health; + namespace Polly.CircuitBreaker; internal sealed class AdvancedCircuitBehavior : CircuitBehavior { - public override void OnActionSuccess(CircuitState currentState) + private readonly HealthMetrics _metrics; + private readonly double _failureThreshold; + private readonly int _minimumThroughput; + + public AdvancedCircuitBehavior(AdvancedCircuitBreakerStrategyOptions options, HealthMetrics metrics) { + _metrics = metrics; + _failureThreshold = options.FailureThreshold; + _minimumThroughput = options.MinimumThroughput; } + public override void OnActionSuccess(CircuitState currentState) => _metrics.IncrementSuccess(); + public override void OnActionFailure(CircuitState currentState, out bool shouldBreak) { - shouldBreak = false; - } + switch (currentState) + { + case CircuitState.Closed: + _metrics.IncrementFailure(); + var info = _metrics.GetHealthInfo(); + shouldBreak = info.Throughput >= _minimumThroughput && info.FailureRate >= _failureThreshold; + break; - public override void OnCircuitClosed() - { + case CircuitState.Open: + case CircuitState.Isolated: + // A failure call result may arrive when the circuit is open, if it was placed before the circuit broke. + // We take no action beyond tracking the metric + // We do not want to duplicate-signal onBreak + // We do not want to extend time for which the circuit is broken. + // We do not want to mask the fact that the call executed (as replacing its result with a Broken/IsolatedCircuitException would do). + _metrics.IncrementFailure(); + shouldBreak = false; + break; + default: + shouldBreak = false; + break; + } } + + public override void OnCircuitClosed() => _metrics.Reset(); } diff --git a/src/Polly.Core/CircuitBreaker/Health/HealthInfo.cs b/src/Polly.Core/CircuitBreaker/Health/HealthInfo.cs new file mode 100644 index 00000000000..0e014526b54 --- /dev/null +++ b/src/Polly.Core/CircuitBreaker/Health/HealthInfo.cs @@ -0,0 +1,15 @@ +namespace Polly.CircuitBreaker.Health; + +internal readonly record struct HealthInfo(int Throughput, double FailureRate) +{ + public static HealthInfo Create(int successes, int failures) + { + var total = successes + failures; + if (total == 0) + { + return new HealthInfo(0, 0); + } + + return new(total, failures / (double)total); + } +} diff --git a/src/Polly.Core/CircuitBreaker/Health/HealthMetrics.cs b/src/Polly.Core/CircuitBreaker/Health/HealthMetrics.cs new file mode 100644 index 00000000000..0d9ec9f573e --- /dev/null +++ b/src/Polly.Core/CircuitBreaker/Health/HealthMetrics.cs @@ -0,0 +1,26 @@ +namespace Polly.CircuitBreaker.Health; + +internal abstract class HealthMetrics +{ + private const short NumberOfWindows = 10; + private static readonly TimeSpan ResolutionOfCircuitTimer = TimeSpan.FromMilliseconds(20); + + protected HealthMetrics(TimeProvider timeProvider) => TimeProvider = timeProvider; + + public static HealthMetrics Create(AdvancedCircuitBreakerStrategyOptions options, TimeProvider timeProvider) + { + return options.SamplingDuration < TimeSpan.FromTicks(ResolutionOfCircuitTimer.Ticks * NumberOfWindows) + ? new SingleHealthMetrics(options.SamplingDuration, timeProvider) + : new RollingHealthMetrics(options.SamplingDuration, NumberOfWindows, timeProvider); + } + + protected TimeProvider TimeProvider { get; } + + public abstract void IncrementSuccess(); + + public abstract void IncrementFailure(); + + public abstract void Reset(); + + public abstract HealthInfo GetHealthInfo(); +} diff --git a/src/Polly.Core/CircuitBreaker/Health/RollingHealthMetrics.cs b/src/Polly.Core/CircuitBreaker/Health/RollingHealthMetrics.cs new file mode 100644 index 00000000000..858df04adca --- /dev/null +++ b/src/Polly.Core/CircuitBreaker/Health/RollingHealthMetrics.cs @@ -0,0 +1,72 @@ +namespace Polly.CircuitBreaker.Health; + +internal sealed class RollingHealthMetrics : HealthMetrics +{ + private readonly TimeSpan _samplingDuration; + private readonly TimeSpan _windowDuration; + private readonly Queue _windows; + + private HealthWindow? _currentWindow; + + public RollingHealthMetrics(TimeSpan samplingDuration, short numberOfWindows, TimeProvider timeProvider) + : base(timeProvider) + { + _samplingDuration = samplingDuration; + _windowDuration = TimeSpan.FromTicks(_samplingDuration.Ticks / numberOfWindows); + _windows = new(numberOfWindows + 1); + } + + public override void IncrementSuccess() => UpdateCurrentWindow().Successes++; + + public override void IncrementFailure() => UpdateCurrentWindow().Failures++; + + public override void Reset() + { + _currentWindow = null; + _windows.Clear(); + } + + public override HealthInfo GetHealthInfo() + { + UpdateCurrentWindow(); + + var successes = 0; + var failures = 0; + foreach (var window in _windows) + { + successes += window.Successes; + failures += window.Failures; + } + + return HealthInfo.Create(successes, failures); + } + + private HealthWindow UpdateCurrentWindow() + { + var now = TimeProvider.UtcNow; + if (_currentWindow == null || now - _currentWindow.StartedAt >= _windowDuration) + { + _currentWindow = new() + { + StartedAt = now + }; + _windows.Enqueue(_currentWindow); + } + + while (now - _windows.Peek().StartedAt >= _samplingDuration) + { + _windows.Dequeue(); + } + + return _currentWindow; + } + + private class HealthWindow + { + public int Successes { get; set; } + + public int Failures { get; set; } + + public DateTimeOffset StartedAt { get; set; } + } +} diff --git a/src/Polly.Core/CircuitBreaker/Health/SingleHealthMetrics.cs b/src/Polly.Core/CircuitBreaker/Health/SingleHealthMetrics.cs new file mode 100644 index 00000000000..54d6812495d --- /dev/null +++ b/src/Polly.Core/CircuitBreaker/Health/SingleHealthMetrics.cs @@ -0,0 +1,53 @@ +using Polly; + +namespace Polly.CircuitBreaker.Health; + +internal sealed class SingleHealthMetrics : HealthMetrics +{ + private readonly TimeSpan _samplingDuration; + + private int _successes; + private int _failures; + private DateTimeOffset _startedAt; + + public SingleHealthMetrics(TimeSpan samplingDuration, TimeProvider timeProvider) + : base(timeProvider) + { + _samplingDuration = samplingDuration; + _startedAt = timeProvider.UtcNow; + } + + public override void IncrementSuccess() + { + TryReset(); + _successes++; + } + + public override void IncrementFailure() + { + TryReset(); + _failures++; + } + + public override void Reset() + { + _startedAt = TimeProvider.UtcNow; + _successes = 0; + _failures = 0; + } + + public override HealthInfo GetHealthInfo() + { + TryReset(); + + return HealthInfo.Create(_successes, _failures); + } + + private void TryReset() + { + if (TimeProvider.UtcNow - _startedAt >= _samplingDuration) + { + Reset(); + } + } +} From 72030a389419a6404de6d057f2437053df8ec6c3 Mon Sep 17 00:00:00 2001 From: martintmk Date: Mon, 24 Apr 2023 10:11:45 +0200 Subject: [PATCH 2/5] Cleanup --- src/Polly.Core.Benchmarks/Program.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Polly.Core.Benchmarks/Program.cs b/src/Polly.Core.Benchmarks/Program.cs index 4f5dea3cbbe..5b9850ebe8a 100644 --- a/src/Polly.Core.Benchmarks/Program.cs +++ b/src/Polly.Core.Benchmarks/Program.cs @@ -8,7 +8,6 @@ var config = ManualConfig .Create(DefaultConfig.Instance) .AddJob(Job.MediumRun.WithToolchain(InProcessEmitToolchain.Instance)) - .AddDiagnoser(MemoryDiagnoser.Default) - .WithOption(ConfigOptions.DisableOptimizationsValidator, true); + .AddDiagnoser(MemoryDiagnoser.Default); -BenchmarkSwitcher.FromAssembly(typeof(PollyVersion).Assembly).Run(args, config); +BenchmarkRunner.Run(typeof(PollyVersion).Assembly, config); From 1f0614813094c778a0c3958a3499995fd74e8cd0 Mon Sep 17 00:00:00 2001 From: martintmk Date: Mon, 24 Apr 2023 10:20:00 +0200 Subject: [PATCH 3/5] Kill mutants --- src/Polly.Core/CircuitBreaker/Health/RollingHealthMetrics.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Polly.Core/CircuitBreaker/Health/RollingHealthMetrics.cs b/src/Polly.Core/CircuitBreaker/Health/RollingHealthMetrics.cs index 858df04adca..3f033e0e8de 100644 --- a/src/Polly.Core/CircuitBreaker/Health/RollingHealthMetrics.cs +++ b/src/Polly.Core/CircuitBreaker/Health/RollingHealthMetrics.cs @@ -13,7 +13,7 @@ public RollingHealthMetrics(TimeSpan samplingDuration, short numberOfWindows, Ti { _samplingDuration = samplingDuration; _windowDuration = TimeSpan.FromTicks(_samplingDuration.Ticks / numberOfWindows); - _windows = new(numberOfWindows + 1); + _windows = new Queue(); } public override void IncrementSuccess() => UpdateCurrentWindow().Successes++; From 4d06992c737fe43d84bd65e8e9580aa7dccc7066 Mon Sep 17 00:00:00 2001 From: martintmk Date: Mon, 24 Apr 2023 11:30:19 +0200 Subject: [PATCH 4/5] PR comments --- src/Polly.Core.Benchmarks/Internals/Helper.CircuitBreaker.cs | 2 -- src/Polly.Core.Benchmarks/Internals/Helper.Retry.cs | 2 -- .../CircuitBreaker/CircuitBreakerResilienceStrategy.cs | 1 - src/Polly.Core/CircuitBreaker/Health/HealthMetrics.cs | 4 ++++ src/Polly.Core/CircuitBreaker/Health/RollingHealthMetrics.cs | 3 ++- src/Polly.Core/CircuitBreaker/Health/SingleHealthMetrics.cs | 1 + 6 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/Polly.Core.Benchmarks/Internals/Helper.CircuitBreaker.cs b/src/Polly.Core.Benchmarks/Internals/Helper.CircuitBreaker.cs index b46cf438858..ea4bee59952 100644 --- a/src/Polly.Core.Benchmarks/Internals/Helper.CircuitBreaker.cs +++ b/src/Polly.Core.Benchmarks/Internals/Helper.CircuitBreaker.cs @@ -1,5 +1,3 @@ -#pragma warning disable S4225 // Extension methods should not extend "object" - using System; using Polly.CircuitBreaker; diff --git a/src/Polly.Core.Benchmarks/Internals/Helper.Retry.cs b/src/Polly.Core.Benchmarks/Internals/Helper.Retry.cs index d95ba0b355a..24562e40dba 100644 --- a/src/Polly.Core.Benchmarks/Internals/Helper.Retry.cs +++ b/src/Polly.Core.Benchmarks/Internals/Helper.Retry.cs @@ -1,5 +1,3 @@ -#pragma warning disable S4225 // Extension methods should not extend "object" - using System; namespace Polly.Core.Benchmarks; diff --git a/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs b/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs index 7346a0dece5..667b8a2a600 100644 --- a/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs +++ b/src/Polly.Core/CircuitBreaker/CircuitBreakerResilienceStrategy.cs @@ -1,4 +1,3 @@ -using System.Runtime.CompilerServices; using Polly.Strategy; namespace Polly.CircuitBreaker; diff --git a/src/Polly.Core/CircuitBreaker/Health/HealthMetrics.cs b/src/Polly.Core/CircuitBreaker/Health/HealthMetrics.cs index 0d9ec9f573e..08aab7850be 100644 --- a/src/Polly.Core/CircuitBreaker/Health/HealthMetrics.cs +++ b/src/Polly.Core/CircuitBreaker/Health/HealthMetrics.cs @@ -1,5 +1,9 @@ namespace Polly.CircuitBreaker.Health; +/// +/// The health metrics for advanced circuit breaker. +/// All operations here are executed from under a lock and are thread safe. +/// internal abstract class HealthMetrics { private const short NumberOfWindows = 10; diff --git a/src/Polly.Core/CircuitBreaker/Health/RollingHealthMetrics.cs b/src/Polly.Core/CircuitBreaker/Health/RollingHealthMetrics.cs index 3f033e0e8de..658aea7b7b8 100644 --- a/src/Polly.Core/CircuitBreaker/Health/RollingHealthMetrics.cs +++ b/src/Polly.Core/CircuitBreaker/Health/RollingHealthMetrics.cs @@ -1,5 +1,6 @@ namespace Polly.CircuitBreaker.Health; +/// internal sealed class RollingHealthMetrics : HealthMetrics { private readonly TimeSpan _samplingDuration; @@ -61,7 +62,7 @@ private HealthWindow UpdateCurrentWindow() return _currentWindow; } - private class HealthWindow + private sealed class HealthWindow { public int Successes { get; set; } diff --git a/src/Polly.Core/CircuitBreaker/Health/SingleHealthMetrics.cs b/src/Polly.Core/CircuitBreaker/Health/SingleHealthMetrics.cs index 54d6812495d..8afdd1470cc 100644 --- a/src/Polly.Core/CircuitBreaker/Health/SingleHealthMetrics.cs +++ b/src/Polly.Core/CircuitBreaker/Health/SingleHealthMetrics.cs @@ -2,6 +2,7 @@ namespace Polly.CircuitBreaker.Health; +/// internal sealed class SingleHealthMetrics : HealthMetrics { private readonly TimeSpan _samplingDuration; From bc0554cb84f9f16731c3f4b4d25f7c2d0fccfb6f Mon Sep 17 00:00:00 2001 From: martintmk Date: Mon, 24 Apr 2023 11:38:35 +0200 Subject: [PATCH 5/5] Turn on CA1852 --- eng/analyzers/Library.globalconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/analyzers/Library.globalconfig b/eng/analyzers/Library.globalconfig index 6b30dc09343..2a816df636d 100644 --- a/eng/analyzers/Library.globalconfig +++ b/eng/analyzers/Library.globalconfig @@ -940,7 +940,7 @@ dotnet_diagnostic.CA1851.severity = suggestion # Category : Performance # Help Link: https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1852 # Tags : Telemetry, EnabledRuleInAggressiveMode, CompilationEnd -dotnet_diagnostic.CA1852.severity = none +dotnet_diagnostic.CA1852.severity = warning # Title : Unnecessary call to 'Dictionary.ContainsKey(key)' # Category : Performance