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

Alpha fixes and improvements #1319

Merged
merged 5 commits into from
Jun 19, 2023
Merged
Changes from all 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
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
``` ini

BenchmarkDotNet=v0.13.5, OS=Windows 11 (10.0.22621.1702/22H2/2022Update/SunValley2)
Intel Core i9-10885H CPU 2.40GHz, 1 CPU, 16 logical and 8 physical cores
.NET SDK=7.0.203
[Host] : .NET 7.0.5 (7.0.523.17405), X64 RyuJIT AVX2
BenchmarkDotNet=v0.13.5, OS=Windows 11 (10.0.22621.1848/22H2/2022Update/SunValley2), VM=Hyper-V
Intel Xeon Platinum 8370C CPU 2.80GHz, 1 CPU, 16 logical and 8 physical cores
.NET SDK=7.0.304
[Host] : .NET 7.0.7 (7.0.723.27404), X64 RyuJIT AVX2

Job=MediumRun Toolchain=InProcessEmitToolchain IterationCount=15
LaunchCount=2 WarmupCount=10

```
| Method | Mean | Error | StdDev | Ratio | RatioSD | Gen0 | Allocated | Alloc Ratio |
|--------------------------- |---------:|----------:|----------:|------:|--------:|-------:|----------:|------------:|
| ExecuteStrategyPipeline_V7 | 2.027 μs | 0.0282 μs | 0.0413 μs | 1.00 | 0.00 | 0.3357 | 2824 B | 1.00 |
| ExecuteStrategyPipeline_V8 | 1.783 μs | 0.0254 μs | 0.0372 μs | 0.88 | 0.03 | 0.0038 | 40 B | 0.01 |
| Method | Mean | Error | StdDev | Ratio | Gen0 | Allocated | Alloc Ratio |
|--------------------------- |---------:|----------:|----------:|------:|-------:|----------:|------------:|
| ExecuteStrategyPipeline_V7 | 2.227 μs | 0.0077 μs | 0.0116 μs | 1.00 | 0.1106 | 2824 B | 1.00 |
| ExecuteStrategyPipeline_V8 | 1.750 μs | 0.0060 μs | 0.0084 μs | 0.79 | - | 40 B | 0.01 |
2 changes: 1 addition & 1 deletion bench/Polly.Core.Benchmarks/Utils/Helper.Retry.cs
Original file line number Diff line number Diff line change
@@ -21,7 +21,7 @@ public static object CreateRetry(PollyVersion technology)
RetryCount = 3,
BackoffType = RetryBackoffType.Constant,
BaseDelay = delay,
ShouldRetry = args => args switch
ShouldHandle = args => args switch
{
{ Exception: InvalidOperationException } => PredicateResult.True,
{ Result: string result } when result == Failure => PredicateResult.True,
Original file line number Diff line number Diff line change
@@ -41,12 +41,7 @@ public abstract class CircuitBreakerStrategyOptions<TResult> : ResilienceStrateg
/// This property is required.
/// </remarks>
[Required]
public Func<OutcomeArguments<TResult, CircuitBreakerPredicateArguments>, ValueTask<bool>> ShouldHandle { get; set; } = args => args.Exception switch
{
OperationCanceledException => PredicateResult.False,
Exception => PredicateResult.True,
_ => PredicateResult.False
};
public Func<OutcomeArguments<TResult, CircuitBreakerPredicateArguments>, ValueTask<bool>> ShouldHandle { get; set; } = DefaultPredicates<CircuitBreakerPredicateArguments, TResult>.HandleOutcome;

/// <summary>
/// Gets or sets the event that is raised when the circuit resets to a <see cref="CircuitState.Closed"/> state.
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
using System.Runtime.ExceptionServices;
using Polly.Telemetry;

namespace Polly.CircuitBreaker;
@@ -151,7 +150,7 @@ public ValueTask CloseCircuitAsync(ResilienceContext context)

if (exception is not null)
{
return new Outcome<TResult>(ExceptionDispatchInfo.Capture(exception));
return new Outcome<TResult>(exception);
}

return null;
@@ -305,6 +304,8 @@ private void SetLastHandledOutcome_NeedsLock<TResult>(Outcome<TResult> outcome)
{
_breakingException = new BrokenCircuitException<TResult>(BrokenCircuitException.DefaultMessage, result!);
}

_breakingException?.TrySetStackTrace();
}

private BrokenCircuitException GetBreakingException_NeedsLock() => _breakingException ?? new BrokenCircuitException();
5 changes: 1 addition & 4 deletions src/Polly.Core/CircuitBreaker/OnCircuitClosedArguments.cs
Original file line number Diff line number Diff line change
@@ -4,7 +4,4 @@ namespace Polly.CircuitBreaker;
/// Arguments used by <see cref="CircuitBreakerStrategyOptions{TResult}.OnClosed"/> event.
/// </summary>
/// <param name="IsManual">Indicates whether the circuit was closed manually by using <see cref="CircuitBreakerManualControl"/>.</param>
/// <remarks>
/// Always use the constructor when creating this struct, otherwise we do not guarantee binary compatibility.
/// </remarks>
public readonly record struct OnCircuitClosedArguments(bool IsManual);
public record OnCircuitClosedArguments(bool IsManual);
Original file line number Diff line number Diff line change
@@ -3,7 +3,4 @@ namespace Polly.CircuitBreaker;
/// <summary>
/// Arguments used by <see cref="CircuitBreakerStrategyOptions{TResult}.OnHalfOpened"/> event.
/// </summary>
/// <remarks>
/// Always use the constructor when creating this struct, otherwise we do not guarantee binary compatibility.
/// </remarks>
public readonly record struct OnCircuitHalfOpenedArguments();
public record OnCircuitHalfOpenedArguments();
5 changes: 1 addition & 4 deletions src/Polly.Core/CircuitBreaker/OnCircuitOpenedArguments.cs
Original file line number Diff line number Diff line change
@@ -5,7 +5,4 @@ namespace Polly.CircuitBreaker;
/// </summary>
/// <param name="BreakDuration">The duration of break.</param>
/// <param name="IsManual">Indicates whether the circuit was opened manually by using <see cref="CircuitBreakerManualControl"/>.</param>
/// <remarks>
/// Always use the constructor when creating this struct, otherwise we do not guarantee binary compatibility.
/// </remarks>
public readonly record struct OnCircuitOpenedArguments(TimeSpan BreakDuration, bool IsManual);
public record OnCircuitOpenedArguments(TimeSpan BreakDuration, bool IsManual);
8 changes: 4 additions & 4 deletions src/Polly.Core/Fallback/FallbackHandler.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
namespace Polly.Fallback;

internal sealed record class FallbackHandler<T>(
PredicateInvoker<HandleFallbackArguments> ShouldHandle,
Func<OutcomeArguments<T, HandleFallbackArguments>, ValueTask<Outcome<T>>> ActionGenerator,
PredicateInvoker<FallbackPredicateArguments> ShouldHandle,
Func<OutcomeArguments<T, FallbackPredicateArguments>, ValueTask<Outcome<T>>> ActionGenerator,
bool IsGeneric)
{
public bool HandlesFallback<TResult>() => IsGeneric switch
@@ -11,9 +11,9 @@ internal sealed record class FallbackHandler<T>(
false => true
};

public async ValueTask<Outcome<TResult>> GetFallbackOutcomeAsync<TResult>(OutcomeArguments<TResult, HandleFallbackArguments> args)
public async ValueTask<Outcome<TResult>> GetFallbackOutcomeAsync<TResult>(OutcomeArguments<TResult, FallbackPredicateArguments> args)
{
var copiedArgs = new OutcomeArguments<T, HandleFallbackArguments>(
var copiedArgs = new OutcomeArguments<T, FallbackPredicateArguments>(
args.Context,
args.Outcome.AsOutcome<T>(),
args.Arguments);
Original file line number Diff line number Diff line change
@@ -6,4 +6,4 @@ namespace Polly.Fallback;
/// <remarks>
/// Always use the constructor when creating this struct, otherwise we do not guarantee binary compatibility.
/// </remarks>
public readonly record struct HandleFallbackArguments();
public readonly record struct FallbackPredicateArguments();
5 changes: 2 additions & 3 deletions src/Polly.Core/Fallback/FallbackResilienceStrategy.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
using System.Runtime.ExceptionServices;
using Polly.Telemetry;

namespace Polly.Fallback;
@@ -29,7 +28,7 @@ protected internal override async ValueTask<Outcome<TResult>> ExecuteCoreAsync<T
}

var outcome = await ExecuteCallbackSafeAsync(callback, context, state).ConfigureAwait(context.ContinueOnCapturedContext);
var handleFallbackArgs = new OutcomeArguments<TResult, HandleFallbackArguments>(context, outcome, new HandleFallbackArguments());
var handleFallbackArgs = new OutcomeArguments<TResult, FallbackPredicateArguments>(context, outcome, new FallbackPredicateArguments());
if (!await _handler.ShouldHandle.HandleAsync(handleFallbackArgs).ConfigureAwait(context.ContinueOnCapturedContext))
{
return outcome;
@@ -50,7 +49,7 @@ protected internal override async ValueTask<Outcome<TResult>> ExecuteCoreAsync<T
}
catch (Exception e)
{
return new Outcome<TResult>(ExceptionDispatchInfo.Capture(e));
return new Outcome<TResult>(e);
}
}
}
Original file line number Diff line number Diff line change
@@ -20,7 +20,7 @@ public static class FallbackResilienceStrategyBuilderExtensions
public static ResilienceStrategyBuilder<TResult> AddFallback<TResult>(
this ResilienceStrategyBuilder<TResult> builder,
Action<PredicateBuilder<TResult>> shouldHandle,
Func<OutcomeArguments<TResult, HandleFallbackArguments>, ValueTask<Outcome<TResult>>> fallbackAction)
Func<OutcomeArguments<TResult, FallbackPredicateArguments>, ValueTask<Outcome<TResult>>> fallbackAction)
{
Guard.NotNull(builder);
Guard.NotNull(shouldHandle);
@@ -34,7 +34,7 @@ public static ResilienceStrategyBuilder<TResult> AddFallback<TResult>(
var predicateBuilder = new PredicateBuilder<TResult>();
shouldHandle(predicateBuilder);

options.ShouldHandle = predicateBuilder.CreatePredicate<HandleFallbackArguments>();
options.ShouldHandle = predicateBuilder.CreatePredicate<FallbackPredicateArguments>();

return builder.AddFallback(options);
}
7 changes: 4 additions & 3 deletions src/Polly.Core/Fallback/FallbackStrategyOptions.TResult.cs
Original file line number Diff line number Diff line change
@@ -18,10 +18,11 @@ public class FallbackStrategyOptions<TResult> : ResilienceStrategyOptions
/// Gets or sets the outcome predicate for determining whether a fallback should be executed.
/// </summary>
/// <remarks>
/// This property is required. Defaults to <see langword="null"/>.
/// Defaults to a delegate that hedges on any exception except <see cref="OperationCanceledException"/>.
/// This property is required.
/// </remarks>
[Required]
public Func<OutcomeArguments<TResult, HandleFallbackArguments>, ValueTask<bool>>? ShouldHandle { get; set; }
public Func<OutcomeArguments<TResult, FallbackPredicateArguments>, ValueTask<bool>> ShouldHandle { get; set; } = DefaultPredicates<FallbackPredicateArguments, TResult>.HandleOutcome;

/// <summary>
/// Gets or sets the fallback action to be executed when the <see cref="ShouldHandle"/> predicate evaluates as true.
@@ -30,7 +31,7 @@ public class FallbackStrategyOptions<TResult> : ResilienceStrategyOptions
/// This property is required. Defaults to <see langword="null"/>.
/// </remarks>
[Required]
public Func<OutcomeArguments<TResult, HandleFallbackArguments>, ValueTask<Outcome<TResult>>>? FallbackAction { get; set; }
public Func<OutcomeArguments<TResult, FallbackPredicateArguments>, ValueTask<Outcome<TResult>>>? FallbackAction { get; set; }

/// <summary>
/// Gets or sets the outcome event instance responsible for triggering fallback events.
5 changes: 1 addition & 4 deletions src/Polly.Core/Fallback/OnFallbackArguments.cs
Original file line number Diff line number Diff line change
@@ -3,7 +3,4 @@ namespace Polly.Fallback;
/// <summary>
/// Represents arguments used in fallback handling scenarios.
/// </summary>
/// <remarks>
/// Always use the constructor when creating this struct, otherwise we do not guarantee binary compatibility.
/// </remarks>
public readonly record struct OnFallbackArguments();
public record OnFallbackArguments();
28 changes: 10 additions & 18 deletions src/Polly.Core/Hedging/Controller/HedgingExecutionContext.cs
Original file line number Diff line number Diff line change
@@ -8,7 +8,7 @@ namespace Polly.Hedging.Utils;
/// The context associated with an execution of hedging resilience strategy.
/// It holds the resources for all executed hedged tasks (primary + secondary) and is responsible for resource disposal.
/// </summary>
internal sealed class HedgingExecutionContext<T>
internal sealed class HedgingExecutionContext<T> : IAsyncDisposable
{
public readonly record struct ExecutionInfo<TResult>(TaskExecution<T>? Execution, bool Loaded, Outcome<TResult>? Outcome);

@@ -81,7 +81,7 @@ public async ValueTask<ExecutionInfo<TResult>> LoadExecutionAsync<TResult, TStat
}
}

public void Complete()
public async ValueTask DisposeAsync()
{
UpdateOriginalContext();

@@ -91,10 +91,14 @@ public void Complete()
pair.Cancel();
}

// We are intentionally doing the cleanup in the background as we do not want to
// delay the hedging.
// The background cleanup is safe. All exceptions are handled.
_ = CleanupInBackgroundAsync();
foreach (var task in _tasks)
{
await task.ExecutionTaskSafe!.ConfigureAwait(false);
await task.ResetAsync().ConfigureAwait(false);
_executionPool.Return(task);
}

Reset();
}

public async ValueTask<TaskExecution<T>?> TryWaitForCompletedExecutionAsync(TimeSpan hedgingDelay)
@@ -226,18 +230,6 @@ private void UpdateOriginalContext()
}
}

private async Task CleanupInBackgroundAsync()
{
foreach (var task in _tasks)
{
await task.ExecutionTaskSafe!.ConfigureAwait(false);
await task.ResetAsync().ConfigureAwait(false);
_executionPool.Return(task);
}

Reset();
}

private void Reset()
{
_replacedProperties.Clear();
2 changes: 1 addition & 1 deletion src/Polly.Core/Hedging/Controller/HedgingHandler.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
namespace Polly.Hedging.Utils;

internal sealed record class HedgingHandler<T>(
PredicateInvoker<HandleHedgingArguments> ShouldHandle,
PredicateInvoker<HedgingPredicateArguments> ShouldHandle,
Func<HedgingActionGeneratorArguments<T>, Func<ValueTask<Outcome<T>>>?> ActionGenerator,
bool IsGeneric)
{
2 changes: 1 addition & 1 deletion src/Polly.Core/Hedging/Controller/TaskExecution.cs
Original file line number Diff line number Diff line change
@@ -208,7 +208,7 @@ private async Task ExecutePrimaryActionAsync<TResult, TState>(Func<ResilienceCon

private async Task UpdateOutcomeAsync<TResult>(Outcome<TResult> outcome)
{
var args = new OutcomeArguments<TResult, HandleHedgingArguments>(Context, outcome, new HandleHedgingArguments());
var args = new OutcomeArguments<TResult, HedgingPredicateArguments>(Context, outcome, new HedgingPredicateArguments());
Outcome = outcome.AsOutcome();
IsHandled = await _handler.ShouldHandle.HandleAsync(args).ConfigureAwait(Context.ContinueOnCapturedContext);
}
Original file line number Diff line number Diff line change
@@ -6,4 +6,4 @@ namespace Polly.Hedging;
/// <remarks>
/// Always use the constructor when creating this struct, otherwise we do not guarantee binary compatibility.
/// </remarks>
public readonly record struct HandleHedgingArguments();
public readonly record struct HedgingPredicateArguments();
Loading