-
Notifications
You must be signed in to change notification settings - Fork 764
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
Hedging now works without routing configuration #4144
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 though I'm not an SME in this.
@@ -31,6 +31,11 @@ public RoutingResilienceStrategy(Func<RequestRoutingStrategy> provider) | |||
Throw.InvalidOperationException("The HTTP request message was not found in the resilience context."); | |||
} | |||
|
|||
if (_provider == null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nit]
if (_provider == null) | |
if (_provider is null) |
@@ -31,6 +31,11 @@ public RoutingResilienceStrategy(Func<RequestRoutingStrategy> provider) | |||
Throw.InvalidOperationException("The HTTP request message was not found in the resilience context."); | |||
} | |||
|
|||
if (_provider == null) | |||
{ | |||
return await callback(context, state).ConfigureAwait(context.ContinueOnCapturedContext); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to await?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, as we need to dispose the RequestRoutingStrategy
after callback is executed.
Previously, the consumer was forced to configure the routing in order to use the hedging:
Now, the consumer can just call
AddStandardHedgingHandler
and the hedging will work. The hedged requests will go to URL specified in theHttpRequestMessage
.I have also added a new benchmark for these changes and some perf improvements:
Before:
After:
I have also updated the HttpResilience benchmarks:
Microsoft Reviewers: Open in CodeFlow