Skip to content

Configure pooled connection lifetime in client factory #1862

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Aug 29, 2022
Merged

Configure pooled connection lifetime in client factory #1862

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6299195
Configure pooled connection lifetime in client factory
JamesNK Aug 26, 2022
8595793
Update
JamesNK Aug 26, 2022
e994f23
Update
JamesNK Aug 26, 2022
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
12 changes: 11 additions & 1 deletion src/Grpc.Net.ClientFactory/GrpcClientServiceExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,13 +334,23 @@ private static IHttpClientBuilder AddGrpcHttpClient<

services.PostConfigure<HttpClientFactoryOptions>(name, options =>
{
options.HttpMessageHandlerBuilderActions.Add(static builder =>
options.HttpMessageHandlerBuilderActions.Add(builder =>
{
if (builder.PrimaryHandler == null)
{
// This will throw in .NET Standard 2.0 with a prompt that a user must set a handler.
// Because it throws it should only be called in PostConfigure if no handler has been set.
var handler = HttpHandlerFactory.CreatePrimaryHandler();
#if NET5_0_OR_GREATER
if (handler is SocketsHttpHandler socketsHttpHandler)
{
// A channel is created once per client, lives forever, and the primary handler never changes.
// It's possible that long lived connections cause the client to miss out on DNS changes.
// Replicate the core benefit of a handler lifetime (periodic connection recreation)
// by setting PooledConnectionLifetime to handler lifetime.
Copy link

@SamGuoMsft SamGuoMsft Aug 26, 2022

Choose a reason for hiding this comment

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

Thanks @JamesNK to address this issue based on our offline discussion. Leave few comments here:

  1. Since we cache channel per typed/named client (no matter the client is transient or singleton), and use PooledConnectionLifetime to control handler lifetime, we are only using httpclientfactory as configuration API, the httpclientfactory handler pool is redundant in this scenario. I think we should work with runtime team to make httpclientfactory just as configuration API.
  2. With caching channel and use PooledConnectionLifetime, would you mind to share more details on why we DI GrpcClientFactory as transient rather than singleton (same as DefatultHttpClientFactory)?
    services.TryAddTransient<GrpcClientFactory, DefaultGrpcClientFactory>();

Copy link
Member Author

@JamesNK JamesNK Aug 27, 2022

Choose a reason for hiding this comment

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

With caching channel and use PooledConnectionLifetime, would you mind to share more details on why we DI GrpcClientFactory as transient rather than singleton (same as DefatultHttpClientFactory)?
services.TryAddTransient<GrpcClientFactory, DefaultGrpcClientFactory>();

So the current IServiceProvider scope is injected into the factory.

socketsHttpHandler.PooledConnectionLifetime = options.HandlerLifetime;
}
#endif
#if NET5_0
handler = HttpHandlerFactory.EnsureTelemetryHandler(handler);
#endif
Expand Down
13 changes: 8 additions & 5 deletions test/Grpc.Net.ClientFactory.Tests/DefaultGrpcClientFactoryTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,8 @@ public void CreateClient_NoPrimaryHandlerNet5OrLater_SocketsHttpHandlerConfigure
// Arrange
var services = new ServiceCollection();
services
.AddGrpcClient<TestGreeterClient>(o => o.Address = new Uri("https://localhost"));
.AddGrpcClient<TestGreeterClient>(o => o.Address = new Uri("https://localhost"))
.SetHandlerLifetime(TimeSpan.FromSeconds(10));

var serviceProvider = services.BuildServiceProvider(validateScopes: true);

Expand All @@ -206,20 +207,22 @@ public void CreateClient_NoPrimaryHandlerNet5OrLater_SocketsHttpHandlerConfigure
var handler = handlerFactory.CreateHandler(nameof(TestGreeterClient));

// Assert
var hasSocketsHttpHandler = false;
SocketsHttpHandler? socketsHttpHandler = null;
HttpMessageHandler? currentHandler = handler;
while (currentHandler is DelegatingHandler delegatingHandler)
{
currentHandler = delegatingHandler.InnerHandler;

if (currentHandler?.GetType() == typeof(SocketsHttpHandler))
if (currentHandler is SocketsHttpHandler s)
{
hasSocketsHttpHandler = true;
socketsHttpHandler = s;
break;
}
}

Assert.IsTrue(hasSocketsHttpHandler);
Assert.IsNotNull(socketsHttpHandler);
Assert.AreEqual(true, socketsHttpHandler!.EnableMultipleHttp2Connections);
Assert.AreEqual(TimeSpan.FromSeconds(10), socketsHttpHandler!.PooledConnectionLifetime);
}
#endif

Expand Down