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

Hedging now works without routing configuration #4144

Merged
merged 5 commits into from
Jul 10, 2023

Conversation

martintmk
Copy link
Contributor

@martintmk martintmk commented Jun 30, 2023

Previously, the consumer was forced to configure the routing in order to use the hedging:

services
    .AddHttpClient("my-client")
    .AddStandardHedgingHandler()
    .RoutingStrategyBuilder.ConfigureOrderedGroups(options =>
    {
        options.Groups.Add(new EndpointGroup
        {
            Endpoints = new List<WeightedEndpoint>
            {
                new WeightedEndpoint
                {
                    Uri = new Uri("https://primary-url:1234"),
                }
            }
        });

        options.Groups.Add(new EndpointGroup
        {
            Endpoints = new List<WeightedEndpoint>
            {
                new WeightedEndpoint
                {
                    Uri = new Uri("https://secondary-url:1234"),
                }
            }
        });
    });

// ... 

var client = httpClientFactory.CreateClient("my-client");
var response = await client.GetAsync("https://primary-url:1234"); // or any URL, it will be replaced with the URLs above

Now, the consumer can just call AddStandardHedgingHandler and the hedging will work. The hedged requests will go to URL specified in the HttpRequestMessage.

services
    .AddHttpClient("my-client")
    .AddStandardHedgingHandler();

// ... 

var client = httpClientFactory.CreateClient("my-client");
var response = await client.GetAsync("https://primary-url:1234");

I have also added a new benchmark for these changes and some perf improvements:

Before:

Method Type Mean Error StdDev Gen0 Allocated
HedgingCall Weighted 7.560 us 0.1658 us 0.2430 us 0.0458 1272 B
HedgingCall Ordered 7.101 us 0.0297 us 0.0436 us 0.0458 1272 B
HedgingCall Weighted, ManyRoutes 7.649 us 0.0405 us 0.0593 us 0.0458 1272 B
HedgingCall Ordered, ManyRoutes 7.222 us 0.1072 us 0.1605 us 0.0458 1272 B
HedgingCall NoRoutes 5.881 us 0.0217 us 0.0305 us 0.0305 832 B

After:

Method Type Mean Error StdDev Gen0 Allocated
HedgingCall Weighted 6.908 us 0.0673 us 0.1007 us 0.0381 1052 B
HedgingCall Ordered 6.779 us 0.0632 us 0.0926 us 0.0381 1052 B
HedgingCall Weighted, ManyRoutes 7.322 us 0.0221 us 0.0309 us 0.0381 1053 B
HedgingCall Ordered, ManyRoutes 6.755 us 0.0604 us 0.0885 us 0.0381 1052 B
HedgingCall NoRoutes 5.866 us 0.0194 us 0.0272 us 0.0305 832 B

I have also updated the HttpResilience benchmarks:

Method Mean Error StdDev Ratio RatioSD Gen0 Allocated Alloc Ratio
DefaultClient 329.3 ns 3.99 ns 5.85 ns 1.00 0.00 0.0234 592 B 1.00
SingleHandler 338.3 ns 4.89 ns 6.85 ns 1.03 0.03 0.0262 664 B 1.12
StandardResilienceHandler 2,972.9 ns 21.21 ns 31.10 ns 9.03 0.16 0.0267 752 B 1.27
StandardHedgingHandler_RoutesFromRequest 6,032.5 ns 36.62 ns 52.52 ns 18.31 0.30 0.0305 832 B 1.41
StandardHedgingHandler_RoutesFromConfig 6,925.1 ns 36.73 ns 54.97 ns 21.04 0.39 0.0381 1063 B 1.80
Microsoft Reviewers: Open in CodeFlow

@ghost ghost assigned martintmk Jun 30, 2023
Copy link
Member

@RussKie RussKie left a 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)
Copy link
Member

Choose a reason for hiding this comment

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

[nit]

Suggested change
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);
Copy link
Contributor

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?

Copy link
Contributor Author

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.

@RussKie RussKie added the waiting-author-feedback 📭 The author of this issue needs to respond in order for us to continue investigating this issue. label Jul 7, 2023
@ghost ghost removed the waiting-author-feedback 📭 The author of this issue needs to respond in order for us to continue investigating this issue. label Jul 10, 2023
@martintmk martintmk enabled auto-merge (squash) July 10, 2023 05:58
@martintmk martintmk merged commit 2ac803e into main Jul 10, 2023
6 checks passed
@martintmk martintmk deleted the mtomka/default-hedging-routing branch July 10, 2023 06:35
@ghost ghost added this to the 8.0 Preview7 milestone Jul 10, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Aug 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants