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

[Docs] Add clarification about property precedence #1918

Merged
merged 18 commits into from
Jan 24, 2024
Merged
Show file tree
Hide file tree
Changes from 8 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
4 changes: 3 additions & 1 deletion docs/chaos/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ Chaos strategies (formerly known as Monkey strategies) are in essence a [Resilie
| [Fault](fault.md) | No | Injects exceptions in your system. |
| [Outcome](outcome.md) | Yes | Injects fake outcomes (results or exceptions) in your system. |
| [Latency](latency.md) | No | Injects latency into executions before the calls are made. |
| [Behavior](behavior.md) | No | Allows you to inject *any* extra behaviour, before a call is placed. |
| [Behavior](behavior.md) | No | Allows you to inject *any* extra behavior, before a call is placed. |

## Common options across strategies

Expand All @@ -85,6 +85,8 @@ All the strategies' options implement the [`ChaosStrategyOptions`](xref:Polly.Si
| `EnabledGenerator` | `null` | The generator that indicates whether the chaos strategy is enabled for a given execution. |

> [!NOTE]
> If both `InjectionRate` and `InjectionRateGenerator` are specified then `InjectionRate` will be ignored.
>
> If both `Enabled` and `EnabledGenerator` are specified then `Enabled` will be ignored.

[simmy]: https://github.com/Polly-Contrib/Simmy
3 changes: 3 additions & 0 deletions docs/strategies/circuit-breaker.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ new ResiliencePipelineBuilder<HttpResponseMessage>().AddCircuitBreaker(optionsSt
| `ManualControl` | `null` | Allows for manual control to isolate or close the circuit. |
| `StateProvider` | `null` | Enables the retrieval of the current state of the circuit. |

> [!NOTE]
> If both `BreakDuration` and `BreakDurationGenerator` are specified then `BreakDuration` will be ignored.

## Diagrams

### State diagram
Expand Down
7 changes: 5 additions & 2 deletions docs/strategies/hedging.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,19 +60,22 @@ new ResiliencePipelineBuilder<HttpResponseMessage>().AddHedging(optionsDefaults)
## Defaults

| Property | Default Value | Description |
| ------------------- | -------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------- |
|---------------------|----------------------------------------------------------------------------|------------------------------------------------------------------------------------------|
| `ShouldHandle` | Predicate that handles all exceptions except `OperationCanceledException`. | Predicate that determines what results and exceptions are handled by the retry strategy. |
| `MaxHedgedAttempts` | 1 | The maximum number of hedged actions to use, in addition to the original action. |
| `Delay` | 2 seconds | The maximum waiting time before spawning a new hedged action. |
| `ActionGenerator` | Returns the original callback that was passed to the hedging strategy. | Generator that creates hedged actions. |
| `DelayGenerator` | `null` | Used for generating custom delays for hedging. If `null` then `Delay` is used. |
| `DelayGenerator` | `null` | Used for generating custom delays for hedging. |
| `OnHedging` | `null` | Event that is raised when a hedging is performed. |

You can use the following special values for `Delay` or in `DelayGenerator`:

- `0 seconds` - the hedging strategy immediately creates a total of `MaxHedgedAttempts` and completes when the fastest acceptable result is available.
- `-1 millisecond` - this value indicates that the strategy does not create a new hedged task before the previous one completes. This enables scenarios where having multiple concurrent hedged tasks can cause side effects.

> [!NOTE]
> If both `Delay` and `DelayGenerator` are specified then `Delay` will be ignored.

## Concurrency modes

In the sections below, explore the different concurrency modes available in the hedging strategy. The behavior is primarily controlled by the `Delay` property value.
Expand Down
48 changes: 48 additions & 0 deletions docs/strategies/retry.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,54 @@ new ResiliencePipelineBuilder<HttpResponseMessage>().AddRetry(optionsExtractDela
| `OnRetry` | `null` | Action executed when retry occurs. |
| `MaxDelay` | `null` | Caps the calculated retry delay to a specified maximum duration. |

## Calculation of the next delay

If the `ShouldHandle` predicate returns `true` and the next attempt number is not greater than `MaxRetryAttempts` then the retry strategy calculates the next delay.

There are many properties that may contribute to this calculation:

- `BackoffType`: It specifies which calculation algorithm should run.
- `Delay`: If only this one is specified then it will be used as is. If others are defined as well then this will be used as a *base delay*.
- `DelayGenerator`: If specified then it will overwrite other properties based calculation. **Except** if it returns a negative `TimeSpan` where the other properties based calculation is used.
peter-csala marked this conversation as resolved.
Show resolved Hide resolved
- `MaxDelay`: If specified then it caps the delay if the calculated one is greater than this value. **Except** if `DelayGenerator` is used where no capping is applied.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- `MaxDelay`: If specified then it caps the delay if the calculated one is greater than this value. **Except** if `DelayGenerator` is used where no capping is applied.
- `MaxDelay`: If specified then it caps the delay if the calculated one is greater than this value. **Except** if `DelayGenerator` is used where no capping is applied.

Just wondering, shall we apply the capping to values produced by generator too?

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 would say yes. That was my intuition before I checked the implementation. But let's deal with that in a separate PR if you don't mind.

Copy link
Contributor

@martintmk martintmk Jan 24, 2024

Choose a reason for hiding this comment

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

My intuition says yes too, the less special cases, the better. The only downside is that sometimes the Retry-After won't be fully respected if the value is greater than MaxDelay.

https://github.com/dotnet/extensions/blob/9f25d9d74674483298cd167cf8ff6e9d9026bf20/src/Libraries/Microsoft.Extensions.Http.Resilience/Polly/HttpRetryStrategyOptions.cs#L57

Thoughts @martincostello

Copy link
Member

Choose a reason for hiding this comment

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

I would agree we should apply it - Retry-After isn't necessarily authoritative and the user may wish to try earlier by overriding. If we do that we should make it clear in the documentation.

Copy link
Contributor Author

@peter-csala peter-csala Jan 24, 2024

Choose a reason for hiding this comment

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

It is a concision decision by the strategy configurator that (s)he sets the MaxDelay. If it is set then the strategy should cap the value regardless how it is calculated/retrieved IMHO.

- `UseJitter`: If enabled then it adds a random value between between -25% of `Delay` and +25% of `Delay`. **Except** if `BackoffType` is set to `Exponential` where jitter is always used.
peter-csala marked this conversation as resolved.
Show resolved Hide resolved

> [!IMPORTANT]
> The summarized description below is an implementation detail. It may change in the future without notice.

The `BackoffType` property's data type is the [`DelayBackOffType`](xref:Polly.DelayBackOffType) enumeration. This primarily controls how the calculation is done.
peter-csala marked this conversation as resolved.
Show resolved Hide resolved

### Constant
Copy link
Contributor

@martintmk martintmk Jan 24, 2024

Choose a reason for hiding this comment

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

Suggested change
### Constant
### Example calculation
Below is the example calculation for `Constant` delay type. It showcases how the delay is calculated, the calculation is almost identical for the other (`Linear`, `Exponential`) backoff types.

Or something in that sense.

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've added examples sections for each.

Screenshot 2024-01-24 at 14 49 23
Screenshot 2024-01-24 at 14 49 34
Screenshot 2024-01-24 at 14 49 48


Even though the `Constant` name could imply that only the `Delay` property is used, in reality all the above listed properties are used.

Step 1: Calculating the base delay:

- If `UseJitter` is set to `false` and `Delay` is specified then `Delay` will be used.
- If `UseJitter` is set to `true` and `Delay` is specified then a random value is added to the `Delay`.
- The random value is between -25% of `Delay` and +25% of `Delay`.

Step 2: Capping the delay if needed:

- If `MaxDelay` is not set then the previously calculated delay will be used.
- If `MaxDelay` is set and the previously calculated delay is greater than `MaxDelay` then `MaxDelay` will be used.

Step 3: Using the generator if supplied

- If the returned `TimeSpan` of the `DelayGenerator` method call is positive then it will be used.
- If the returned `TimeSpan` of the `DelayGenerator` method call is negative then it will use the step 2's result.

> [!NOTE]
> The `DelayGenerator`'s returned value is not capped with the `MaxDelay`.

### Linear

### Exponential

> [!TIP]
> For more details please check out the [`RetryHelper`](https://github.com/App-vNext/Polly/blob/main/src/Polly.Core/Retry/RetryHelper.cs)
> and the [`RetryResilienceStrategy`](https://github.com/App-vNext/Polly/blob/main/src/Polly.Core/Retry/RetryResilienceStrategy.cs) classes.

## Diagrams

Let's suppose we have a retry strategy with `MaxRetryAttempts`: `2`.
Expand Down
3 changes: 3 additions & 0 deletions docs/strategies/timeout.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ The `OnTimeout` delegate can be useful when you define a resilience pipeline whi
| `TimeoutGenerator` | `null` | Generates the timeout for a given execution. |
| `OnTimeout` | `null` | Event that is raised when timeout occurs. |

> [!NOTE]
> If both `Timeout` and `TimeoutGenerator` are specified then `Timeout` will be ignored.

## Diagrams

### Happy path sequence diagram
Expand Down