Skip to content

Commit

Permalink
API Review Feedback (#1521)
Browse files Browse the repository at this point in the history
  • Loading branch information
martintmk authored Aug 29, 2023
1 parent daf1529 commit 94db492
Show file tree
Hide file tree
Showing 29 changed files with 203 additions and 210 deletions.
2 changes: 1 addition & 1 deletion bench/Polly.Core.Benchmarks/Utils/Helper.Hedging.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public static ResiliencePipeline<string> CreateHedging()
builder.AddHedging(new HedgingStrategyOptions<string>
{
ShouldHandle = args => new ValueTask<bool>(args.Outcome.Result == Failure),
HedgingActionGenerator = args => () => Outcome.FromResultAsTask("hedged response"),
ActionGenerator = args => () => Outcome.FromResultAsTask("hedged response"),
});
});
}
Expand Down
12 changes: 6 additions & 6 deletions bench/Polly.Core.Benchmarks/Utils/Helper.MultipleStrategies.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ internal static partial class Helper
.AddTimeout(TimeSpan.FromSeconds(10))
.AddRetry(new()
{
BackoffType = RetryBackoffType.Constant,
RetryCount = 3,
BaseDelay = TimeSpan.FromSeconds(1),
BackoffType = DelayBackoffType.Constant,
MaxRetryAttempts = 3,
Delay = TimeSpan.FromSeconds(1),
ShouldHandle = args => args.Outcome switch
{
{ Exception: InvalidOperationException } => PredicateResult.True,
Expand Down Expand Up @@ -68,9 +68,9 @@ public static ResiliencePipeline CreateNonGenericStrategyPipeline(bool telemetry
.AddTimeout(TimeSpan.FromSeconds(10))
.AddRetry(new()
{
BackoffType = RetryBackoffType.Constant,
RetryCount = 3,
BaseDelay = TimeSpan.FromSeconds(1),
BackoffType = DelayBackoffType.Constant,
MaxRetryAttempts = 3,
Delay = TimeSpan.FromSeconds(1),
ShouldHandle = args => args.Outcome switch
{
{ Exception: InvalidOperationException } => PredicateResult.True,
Expand Down
6 changes: 3 additions & 3 deletions bench/Polly.Core.Benchmarks/Utils/Helper.Retry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ public static object CreateRetry(PollyVersion technology)
{
builder.AddRetry(new RetryStrategyOptions<string>
{
RetryCount = 3,
BackoffType = RetryBackoffType.Constant,
BaseDelay = delay,
MaxRetryAttempts = 3,
BackoffType = DelayBackoffType.Constant,
Delay = delay,
ShouldHandle = args => args.Outcome switch
{
{ Exception: InvalidOperationException } => PredicateResult.True,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,38 +1,36 @@
namespace Polly.Retry;

/// <summary>
/// The backoff type used by the retry strategy.
/// The backoff type used by the strategies.
/// </summary>
public enum RetryBackoffType
public enum DelayBackoffType
{
/// <summary>
/// The constant retry type.
/// The constant backoff type.
/// </summary>
/// <example>
/// 200ms, 200ms, 200ms, etc.
/// </example>
/// <remarks>
/// Ensures a constant wait duration before each retry attempt.
/// For concurrent database access with a possibility of conflicting updates,
/// retrying the failures in a constant manner allows for consistent transient failure mitigation.
/// Ensures a constant backoff for each attempt.
/// </remarks>
Constant,

/// <summary>
/// The linear retry type.
/// The linear backoff type.
/// </summary>
/// <example>
/// 100ms, 200ms, 300ms, 400ms, etc.
/// </example>
/// <remarks>
/// Generates sleep durations in an linear manner.
/// Generates backoffs in an linear manner.
/// In the case randomization introduced by the jitter and exponential growth are not appropriate,
/// the linear growth allows for more precise control over the delay intervals.
/// the linear growth allows for more precise control over the backoff intervals.
/// </remarks>
Linear,

/// <summary>
/// The exponential delay type with the power of 2.
/// The exponential backoff type with the power of 2.
/// </summary>
/// <example>
/// 200ms, 400ms, 800ms.
Expand Down
4 changes: 2 additions & 2 deletions src/Polly.Core/Hedging/HedgingActionGeneratorArguments.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public readonly struct HedgingActionGeneratorArguments<TResult>
/// </summary>
/// <param name="primaryContext">The primary resilience context.</param>
/// <param name="actionContext">
/// The context that will be passed to action generated by <see cref="HedgingStrategyOptions{TResult}.HedgingActionGenerator"/>.
/// The context that will be passed to action generated by <see cref="HedgingStrategyOptions{TResult}.ActionGenerator"/>.
/// .</param>
/// <param name="attemptNumber">The zero-based hedging attempt number.</param>
/// <param name="callback">The callback passed to hedging strategy.</param>
Expand All @@ -38,7 +38,7 @@ public HedgingActionGeneratorArguments(
public ResilienceContext PrimaryContext { get; }

/// <summary>
/// Gets the context that will be passed to action generated by <see cref="HedgingStrategyOptions{TResult}.HedgingActionGenerator"/>.
/// Gets the context that will be passed to action generated by <see cref="HedgingStrategyOptions{TResult}.ActionGenerator"/>.
/// </summary>
/// <remarks>
/// This context is cloned from <see cref="PrimaryContext"/>.
Expand Down
4 changes: 2 additions & 2 deletions src/Polly.Core/Hedging/HedgingConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ internal static class HedgingConstants

public const string OnHedgingEventName = "OnHedging";

public const int DefaultMaxHedgedAttempts = 2;
public const int DefaultMaxHedgedAttempts = 1;

public const int MinimumHedgedAttempts = 2;
public const int MinimumHedgedAttempts = 1;

public const int MaximumHedgedAttempts = 10;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,15 @@ private static HedgingResilienceStrategy<TResult> CreateHedgingStrategy<TResult>
{
var handler = new HedgingHandler<TResult>(
options.ShouldHandle!,
options.HedgingActionGenerator,
options.ActionGenerator,
IsGeneric: isGeneric);

return new HedgingResilienceStrategy<TResult>(
options.HedgingDelay,
options.Delay,
options.MaxHedgedAttempts,
handler,
options.OnHedging,
options.HedgingDelayGenerator,
options.DelayGenerator,
context.TimeProvider,
context.Telemetry);
}
Expand Down
14 changes: 7 additions & 7 deletions src/Polly.Core/Hedging/HedgingResilienceStrategy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,21 @@ public HedgingResilienceStrategy(
ResilienceStrategyTelemetry telemetry)
{
HedgingDelay = hedgingDelay;
MaxHedgedAttempts = maxHedgedAttempts;
HedgingDelayGenerator = hedgingDelayGenerator;
TotalAttempts = maxHedgedAttempts + 1; // include the initial attempt
DelayGenerator = hedgingDelayGenerator;
_timeProvider = timeProvider;
HedgingHandler = hedgingHandler;
OnHedging = onHedging;

_telemetry = telemetry;
_controller = new HedgingController<T>(telemetry, timeProvider, HedgingHandler, maxHedgedAttempts);
_controller = new HedgingController<T>(telemetry, timeProvider, HedgingHandler, TotalAttempts);
}

public TimeSpan HedgingDelay { get; }

public int MaxHedgedAttempts { get; }
public int TotalAttempts { get; }

public Func<HedgingDelayGeneratorArguments, ValueTask<TimeSpan>>? HedgingDelayGenerator { get; }
public Func<HedgingDelayGeneratorArguments, ValueTask<TimeSpan>>? DelayGenerator { get; }

public HedgingHandler<T> HedgingHandler { get; }

Expand Down Expand Up @@ -128,11 +128,11 @@ private async ValueTask HandleOnHedgingAsync(OnHedgingArguments<T> args)

internal ValueTask<TimeSpan> GetHedgingDelayAsync(ResilienceContext context, int attempt)
{
if (HedgingDelayGenerator == null)
if (DelayGenerator == null)
{
return new ValueTask<TimeSpan>(HedgingDelay);
}

return HedgingDelayGenerator(new HedgingDelayGeneratorArguments(context, attempt));
return DelayGenerator(new HedgingDelayGeneratorArguments(context, attempt));
}
}
29 changes: 12 additions & 17 deletions src/Polly.Core/Hedging/HedgingStrategyOptions.TResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,28 +14,23 @@ public class HedgingStrategyOptions<TResult> : ResilienceStrategyOptions
public HedgingStrategyOptions() => Name = HedgingConstants.DefaultName;

/// <summary>
/// Gets or sets the minimal time of waiting before spawning a new hedged call.
/// Gets or sets the maximum waiting time before spawning a new hedged action.
/// </summary>
/// <remarks>
/// You can also use <see cref="TimeSpan.Zero"/> to create all hedged tasks (value of <see cref="MaxHedgedAttempts"/>) at once
/// or <see cref="System.Threading.Timeout.InfiniteTimeSpan"/> to force the hedging strategy to never create new task before the old one is finished.
/// <para> If you want a greater control over hedging delay customization use <see cref="HedgingDelayGenerator"/>.</para>
/// You can use <see cref="TimeSpan.Zero"/> to create all hedged actions (value of <see cref="MaxHedgedAttempts"/>) at once
/// or <see cref="System.Threading.Timeout.InfiniteTimeSpan"/> to force the hedging strategy to never create new action before the old one is finished.
/// <para> If you want a greater control over hedging delay customization use <see cref="DelayGenerator"/>.</para>
/// </remarks>
/// <value>
/// The default value is 2 seconds.
/// </value>
public TimeSpan HedgingDelay { get; set; } = HedgingConstants.DefaultHedgingDelay;
public TimeSpan Delay { get; set; } = HedgingConstants.DefaultHedgingDelay;

/// <summary>
/// Gets or sets the maximum hedged attempts to perform the desired task.
/// Gets or sets the maximum number of hedged actions to use, in addition to the original action.
/// </summary>
/// <remarks>
/// The value defines how many concurrent hedged tasks will be triggered by the strategy.
/// This includes the primary hedged task that is initially performed, and the further tasks that will
/// be fetched from the provider and spawned in parallel.
/// </remarks>
/// <value>
/// The default value is 2. The value must be bigger or equal to 2, and lower or equal to 10.
/// The default value is 1. The value must be bigger or equal to 1, and lower or equal to 10.
/// </value>
[Range(HedgingConstants.MinimumHedgedAttempts, HedgingConstants.MaximumHedgedAttempts)]
public int MaxHedgedAttempts { get; set; } = HedgingConstants.DefaultMaxHedgedAttempts;
Expand All @@ -57,7 +52,7 @@ public class HedgingStrategyOptions<TResult> : ResilienceStrategyOptions
/// The default generator executes the original callback that was passed to the hedging resilience strategy. This property is required.
/// </value>
[Required]
public Func<HedgingActionGeneratorArguments<TResult>, Func<ValueTask<Outcome<TResult>>>?> HedgingActionGenerator { get; set; } = args =>
public Func<HedgingActionGeneratorArguments<TResult>, Func<ValueTask<Outcome<TResult>>>?> ActionGenerator { get; set; } = args =>
{
return async () =>
{
Expand All @@ -71,22 +66,22 @@ public class HedgingStrategyOptions<TResult> : ResilienceStrategyOptions
};

/// <summary>
/// Gets or sets a generator that generates hedging delays for each hedging attempt.
/// Gets or sets a generator that generates hedging delays for each hedging action.
/// </summary>
/// <remarks>
/// The <see cref="HedgingDelayGenerator"/> takes precedence over <see cref="HedgingDelay"/>. If specified, the <see cref="HedgingDelay"/> is ignored.
/// The <see cref="DelayGenerator"/> takes precedence over <see cref="Delay"/>. If specified, the <see cref="Delay"/> is ignored.
/// </remarks>
/// <value>
/// The default value is <see langword="null"/>.
/// </value>
public Func<HedgingDelayGeneratorArguments, ValueTask<TimeSpan>>? HedgingDelayGenerator { get; set; }
public Func<HedgingDelayGeneratorArguments, ValueTask<TimeSpan>>? DelayGenerator { get; set; }

/// <summary>
/// Gets or sets the event that is raised when a hedging is performed.
/// </summary>
/// <remarks>
/// The hedging is executed when the current attempt outcome is not successful and the <see cref="ShouldHandle"/> predicate returns <see langword="true"/> or when
/// the current attempt did not finish within the <see cref="HedgingDelay"/>.
/// the current attempt did not finish within the <see cref="Delay"/>.
/// </remarks>
/// <value>
/// The default value is <see langword="null"/>.
Expand Down
34 changes: 17 additions & 17 deletions src/Polly.Core/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,12 @@ Polly.Hedging.HedgingPredicateArguments<TResult>.HedgingPredicateArguments() ->
Polly.Hedging.HedgingPredicateArguments<TResult>.HedgingPredicateArguments(Polly.ResilienceContext! context, Polly.Outcome<TResult> outcome) -> void
Polly.Hedging.HedgingPredicateArguments<TResult>.Outcome.get -> Polly.Outcome<TResult>
Polly.Hedging.HedgingStrategyOptions<TResult>
Polly.Hedging.HedgingStrategyOptions<TResult>.HedgingActionGenerator.get -> System.Func<Polly.Hedging.HedgingActionGeneratorArguments<TResult>, System.Func<System.Threading.Tasks.ValueTask<Polly.Outcome<TResult>>>?>!
Polly.Hedging.HedgingStrategyOptions<TResult>.HedgingActionGenerator.set -> void
Polly.Hedging.HedgingStrategyOptions<TResult>.HedgingDelay.get -> System.TimeSpan
Polly.Hedging.HedgingStrategyOptions<TResult>.HedgingDelay.set -> void
Polly.Hedging.HedgingStrategyOptions<TResult>.HedgingDelayGenerator.get -> System.Func<Polly.Hedging.HedgingDelayGeneratorArguments, System.Threading.Tasks.ValueTask<System.TimeSpan>>?
Polly.Hedging.HedgingStrategyOptions<TResult>.HedgingDelayGenerator.set -> void
Polly.Hedging.HedgingStrategyOptions<TResult>.ActionGenerator.get -> System.Func<Polly.Hedging.HedgingActionGeneratorArguments<TResult>, System.Func<System.Threading.Tasks.ValueTask<Polly.Outcome<TResult>>>?>!
Polly.Hedging.HedgingStrategyOptions<TResult>.ActionGenerator.set -> void
Polly.Hedging.HedgingStrategyOptions<TResult>.Delay.get -> System.TimeSpan
Polly.Hedging.HedgingStrategyOptions<TResult>.Delay.set -> void
Polly.Hedging.HedgingStrategyOptions<TResult>.DelayGenerator.get -> System.Func<Polly.Hedging.HedgingDelayGeneratorArguments, System.Threading.Tasks.ValueTask<System.TimeSpan>>?
Polly.Hedging.HedgingStrategyOptions<TResult>.DelayGenerator.set -> void
Polly.Hedging.HedgingStrategyOptions<TResult>.HedgingStrategyOptions() -> void
Polly.Hedging.HedgingStrategyOptions<TResult>.MaxHedgedAttempts.get -> int
Polly.Hedging.HedgingStrategyOptions<TResult>.MaxHedgedAttempts.set -> void
Expand Down Expand Up @@ -281,6 +281,10 @@ Polly.ResilienceStrategyOptions
Polly.ResilienceStrategyOptions.Name.get -> string?
Polly.ResilienceStrategyOptions.Name.set -> void
Polly.ResilienceStrategyOptions.ResilienceStrategyOptions() -> void
Polly.Retry.DelayBackoffType
Polly.Retry.DelayBackoffType.Constant = 0 -> Polly.Retry.DelayBackoffType
Polly.Retry.DelayBackoffType.Exponential = 2 -> Polly.Retry.DelayBackoffType
Polly.Retry.DelayBackoffType.Linear = 1 -> Polly.Retry.DelayBackoffType
Polly.Retry.OnRetryArguments<TResult>
Polly.Retry.OnRetryArguments<TResult>.AttemptNumber.get -> int
Polly.Retry.OnRetryArguments<TResult>.Context.get -> Polly.ResilienceContext!
Expand All @@ -289,10 +293,6 @@ Polly.Retry.OnRetryArguments<TResult>.OnRetryArguments() -> void
Polly.Retry.OnRetryArguments<TResult>.OnRetryArguments(Polly.ResilienceContext! context, Polly.Outcome<TResult> outcome, int attemptNumber, System.TimeSpan retryDelay, System.TimeSpan duration) -> void
Polly.Retry.OnRetryArguments<TResult>.Outcome.get -> Polly.Outcome<TResult>
Polly.Retry.OnRetryArguments<TResult>.RetryDelay.get -> System.TimeSpan
Polly.Retry.RetryBackoffType
Polly.Retry.RetryBackoffType.Constant = 0 -> Polly.Retry.RetryBackoffType
Polly.Retry.RetryBackoffType.Exponential = 2 -> Polly.Retry.RetryBackoffType
Polly.Retry.RetryBackoffType.Linear = 1 -> Polly.Retry.RetryBackoffType
Polly.Retry.RetryDelayGeneratorArguments<TResult>
Polly.Retry.RetryDelayGeneratorArguments<TResult>.AttemptNumber.get -> int
Polly.Retry.RetryDelayGeneratorArguments<TResult>.Context.get -> Polly.ResilienceContext!
Expand All @@ -309,18 +309,18 @@ Polly.Retry.RetryPredicateArguments<TResult>.RetryPredicateArguments(Polly.Resil
Polly.Retry.RetryStrategyOptions
Polly.Retry.RetryStrategyOptions.RetryStrategyOptions() -> void
Polly.Retry.RetryStrategyOptions<TResult>
Polly.Retry.RetryStrategyOptions<TResult>.BackoffType.get -> Polly.Retry.RetryBackoffType
Polly.Retry.RetryStrategyOptions<TResult>.BackoffType.get -> Polly.Retry.DelayBackoffType
Polly.Retry.RetryStrategyOptions<TResult>.BackoffType.set -> void
Polly.Retry.RetryStrategyOptions<TResult>.BaseDelay.get -> System.TimeSpan
Polly.Retry.RetryStrategyOptions<TResult>.BaseDelay.set -> void
Polly.Retry.RetryStrategyOptions<TResult>.Delay.get -> System.TimeSpan
Polly.Retry.RetryStrategyOptions<TResult>.Delay.set -> void
Polly.Retry.RetryStrategyOptions<TResult>.DelayGenerator.get -> System.Func<Polly.Retry.RetryDelayGeneratorArguments<TResult>, System.Threading.Tasks.ValueTask<System.TimeSpan>>?
Polly.Retry.RetryStrategyOptions<TResult>.DelayGenerator.set -> void
Polly.Retry.RetryStrategyOptions<TResult>.MaxRetryAttempts.get -> int
Polly.Retry.RetryStrategyOptions<TResult>.MaxRetryAttempts.set -> void
Polly.Retry.RetryStrategyOptions<TResult>.OnRetry.get -> System.Func<Polly.Retry.OnRetryArguments<TResult>, System.Threading.Tasks.ValueTask>?
Polly.Retry.RetryStrategyOptions<TResult>.OnRetry.set -> void
Polly.Retry.RetryStrategyOptions<TResult>.Randomizer.get -> System.Func<double>!
Polly.Retry.RetryStrategyOptions<TResult>.Randomizer.set -> void
Polly.Retry.RetryStrategyOptions<TResult>.RetryCount.get -> int
Polly.Retry.RetryStrategyOptions<TResult>.RetryCount.set -> void
Polly.Retry.RetryStrategyOptions<TResult>.RetryDelayGenerator.get -> System.Func<Polly.Retry.RetryDelayGeneratorArguments<TResult>, System.Threading.Tasks.ValueTask<System.TimeSpan>>?
Polly.Retry.RetryStrategyOptions<TResult>.RetryDelayGenerator.set -> void
Polly.Retry.RetryStrategyOptions<TResult>.RetryStrategyOptions() -> void
Polly.Retry.RetryStrategyOptions<TResult>.ShouldHandle.get -> System.Func<Polly.Retry.RetryPredicateArguments<TResult>, System.Threading.Tasks.ValueTask<bool>>!
Polly.Retry.RetryStrategyOptions<TResult>.ShouldHandle.set -> void
Expand Down
2 changes: 1 addition & 1 deletion src/Polly.Core/Retry/RetryConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ internal static class RetryConstants

public const string OnRetryEvent = "OnRetry";

public const RetryBackoffType DefaultBackoffType = RetryBackoffType.Constant;
public const DelayBackoffType DefaultBackoffType = DelayBackoffType.Constant;

public const int DefaultRetryCount = 3;

Expand Down
2 changes: 1 addition & 1 deletion src/Polly.Core/Retry/RetryDelayGeneratorArguments.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ namespace Polly.Retry;
#pragma warning disable CA1815 // Override equals and operator equals on value types

/// <summary>
/// Represents the arguments used by <see cref="RetryStrategyOptions{TResult}.RetryDelayGenerator"/> for generating the next retry delay.
/// Represents the arguments used by <see cref="RetryStrategyOptions{TResult}.DelayGenerator"/> for generating the next retry delay.
/// </summary>
/// <typeparam name="TResult">The type of result.</typeparam>
/// <remarks>
Expand Down
Loading

0 comments on commit 94db492

Please sign in to comment.