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
Show file tree
Hide file tree
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
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
using System.Runtime.ExceptionServices;
using Polly.Telemetry;

namespace Polly.CircuitBreaker;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
5 changes: 1 addition & 4 deletions src/Polly.Core/CircuitBreaker/OnCircuitClosedArguments.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Up @@ -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
Expand Up @@ -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
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Up @@ -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);
Expand All @@ -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);
}
Expand Down
7 changes: 4 additions & 3 deletions src/Polly.Core/Fallback/FallbackStrategyOptions.TResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
5 changes: 1 addition & 4 deletions src/Polly.Core/Fallback/OnFallbackArguments.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Up @@ -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);

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

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

Expand All @@ -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)
Expand Down Expand Up @@ -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();
Expand Down
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)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Polly.Core/Hedging/Controller/TaskExecution.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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