Skip to content
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.

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

Implement Advanced Circuit Breaker #1153

Merged
merged 5 commits into from
Apr 24, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
24 changes: 24 additions & 0 deletions src/Polly.Core.Benchmarks/Benchmarks/CircuitBreakerBenchmark.cs
Original file line number Diff line number Diff line change
@@ -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);
}
38 changes: 38 additions & 0 deletions src/Polly.Core.Benchmarks/Internals/Helper.CircuitBreaker.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
#pragma warning disable S4225 // Extension methods should not extend "object"
martincostello marked this conversation as resolved.
Show resolved Hide resolved

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<InvalidOperationException>()
.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<int>((outcome, _) => outcome.Result == 10 || outcome.Exception is InvalidOperationException);
builder.AddAdvancedCircuitBreaker(options);
}),
_ => throw new NotSupportedException()
};
}
}
23 changes: 17 additions & 6 deletions src/Polly.Core.Benchmarks/Internals/Helper.Pipeline.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Threading.RateLimiting;
using Polly;
using Polly.Strategy;

namespace Polly.Core.Benchmarks;

Expand All @@ -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<InvalidOperationException>().AdvancedCircuitBreakerAsync(0.5, TimeSpan.FromSeconds(30), 10, TimeSpan.FromSeconds(5)),
Policy.TimeoutAsync<int>(TimeSpan.FromSeconds(1)),
Policy.Handle<InvalidOperationException>().OrResult<int>(10).WaitAndRetryAsync(3, attempt => TimeSpan.FromSeconds(1)),
Policy.Handle<InvalidOperationException>().OrResult(10).WaitAndRetryAsync(3, attempt => TimeSpan.FromSeconds(1)),
Policy.TimeoutAsync<int>(TimeSpan.FromSeconds(10)),
Policy.BulkheadAsync<int>(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<InvalidOperationException>().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<CircuitBreakerPredicateArguments>()
.HandleOutcome<int>((outcome, _) => outcome.Result == 10 || outcome.Exception is InvalidOperationException)
});
}),
_ => throw new NotSupportedException()
Expand Down
17 changes: 12 additions & 5 deletions src/Polly.Core.Benchmarks/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Comment on lines +54 to +55
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting that it's slower.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will give this another shot later to see if we can bring it down. Allocations are greatly reduced though, that for us makes bigger impact.


## 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 |
Original file line number Diff line number Diff line change
Expand Up @@ -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<int>(() => { });
options.OnClosed.Register<int>(() => { });
options.OnHalfOpened.Register(() => { });
options.OnOpened.Register<int>(() => opened++);
options.OnClosed.Register<int>(() => 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<CircuitBreakerResilienceStrategy>();
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<BrokenCircuitException<int>>(() => strategy.Execute(_ => 0));

// Circuit Half Opened
time += options.BreakDuration;
strategy.Execute(_ => -1);
Assert.Throws<BrokenCircuitException<int>>(() => 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]
Expand Down
Original file line number Diff line number Diff line change
@@ -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<HealthMetrics> _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);
}
26 changes: 26 additions & 0 deletions src/Polly.Core.Tests/CircuitBreaker/Health/HealthMetricsTests.cs
Original file line number Diff line number Diff line change
@@ -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);
}
}
Loading