feat: adds customization options for SocketsHttpHandler #539
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In `Grpc.Net.Client`, the default HttpHandler on .NET runtimes .NET 5.0 or greater is `SocketsHttpHandler`. In this PR we create a new options class to encapsulate customizations to the handler, most importantly the pooled connection timeout and whether to enable multiple http connections. The default value for the idle connection timeout is 1 minute. We find this too strict in lambda environments, where the container may freeze and thaw in greater than 1 minute spans but less than the server timeout (5 minutes as of writing). Therefore we set the lambda config pooled connection idle timeout higher, to 6 minutes. That way a lambda function will not needlessly reconnect when the connection is still open.
Previously we relied on .NET Framework 4.61 as the minimum version. The next minor version introduced async iterator features necessary for the topic client. Because of this, at the time, we added preprocessor directives to exclude the topic client if the runtime wasn't .NET Standard 2.0 or greater. Now that we add another target framework, .NET 6.0, maintaining the different platform directives is difficult. Instead we bump the target framework from net461 to net462 to gain the async iterators and remove the platform conditional compilation directives.
malandis
force-pushed
the
feat/sockets-http-handler-customizations
branch
from
7bdc8e3
to
90ccf64
Compare
malandis
commented
Feb 28, 2024
Comment on lines
+103
to
+108
| #if NET5_0_OR_GREATER | ||
| HttpHandler = new System.Net.Http.SocketsHttpHandler | ||
| { | ||
| EnableMultipleHttp2Connections = config.TransportStrategy.GrpcConfig.SocketsHttpHandlerOptions.EnableMultipleHttp2Connections, | ||
| PooledConnectionIdleTimeout = config.TransportStrategy.GrpcConfig.SocketsHttpHandlerOptions.PooledConnectionIdleTimeout | ||
| } |
There was a problem hiding this comment.
Here is where we set the SocketsHttpHandler settings for the control client
Comment on lines
+287
to
+292
| #if NET5_0_OR_GREATER | ||
| channelOptions.HttpHandler = new SocketsHttpHandler | ||
| { | ||
| EnableMultipleHttp2Connections = config.TransportStrategy.GrpcConfig.SocketsHttpHandlerOptions.EnableMultipleHttp2Connections, | ||
| PooledConnectionIdleTimeout = config.TransportStrategy.GrpcConfig.SocketsHttpHandlerOptions.PooledConnectionIdleTimeout | ||
| }; |
There was a problem hiding this comment.
Here is where we set the data client SocketsHttpHandler settings
| @@ -379,8 +379,6 @@ public static TSuccess LogTraceCollectionRequestSuccess<TSuccess>(this ILogger _ | |||
| } | |||
| return success; | |||
| } | |||
|
|
|||
| #if NETSTANDARD2_0_OR_GREATER | |||
There was a problem hiding this comment.
None of these NETSTANDARD2.0 directives are necessary anymore since I bumped the target framework from .NET 4.61 to .NET 4.62.
| @@ -2,11 +2,9 @@ | |||
| <Project Sdk="Microsoft.NET.Sdk"> | |||
| <PropertyGroup> | |||
| <!-- Build Configuration --> | |||
| <TargetFrameworks>netstandard2.0;net461</TargetFrameworks> | |||
| <TargetFrameworks>netstandard2.0;net6.0;net462</TargetFrameworks> | |||
There was a problem hiding this comment.
Adding net6.0 and bumping net 4.61 to 4.62.
Comment on lines
-8
to
-9
| <!-- Microsoft.Bcl.AsyncInterfaces on net461 isn't supported; suppress warning --> | ||
| <SuppressTfmSupportBuildWarnings>true</SuppressTfmSupportBuildWarnings> |
There was a problem hiding this comment.
No longer necessary since we are on net462
| { | ||
| public static TimeSpan DefaultPooledConnectionIdleTimeout { get; } = TimeSpan.FromMinutes(1); | ||
| public TimeSpan PooledConnectionIdleTimeout { get; } = DefaultPooledConnectionIdleTimeout; | ||
| public bool EnableMultipleHttp2Connections { get; } = true; |
There was a problem hiding this comment.
Curious if we needed in this PR and if true is default (should be but confirming). I am having a hard time connect this with gRPC as ideally the http2 control should be in the gRPC libraries.
There was a problem hiding this comment.
This is to align with the gRPC .NET client defaults. We can revisit later: https://github.com/grpc/grpc-dotnet/blob/098dca892a3949ade411c3f2f66003f7b330dfd2/src/Shared/HttpHandlerFactory.cs#L27-L51
| EnableMultipleHttp2Connections = config.TransportStrategy.GrpcConfig.SocketsHttpHandlerOptions.EnableMultipleHttp2Connections, | ||
| PooledConnectionIdleTimeout = config.TransportStrategy.GrpcConfig.SocketsHttpHandlerOptions.PooledConnectionIdleTimeout | ||
| }; | ||
| #elif USE_GRPC_WEB | ||
| channelOptions.HttpHandler = new GrpcWebHandler(new HttpClientHandler()); |
There was a problem hiding this comment.
Okay nice since the grpc handler takes this it shouldn't affect global state !
pratik151192
approved these changes
Feb 28, 2024
nand4011
approved these changes
Feb 28, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
SocketHttpHandler.PooledConnectionIdleTimeoutof 6 minutes.
to net462 for async iterator support.
SocketHttpHandler config
In
Grpc.Net.Client, the default HttpHandler on .NET runtimes .NET5.0 or greater is
SocketsHttpHandler. In this PR we create a newoptions class to encapsulate customizations to the handler, most
importantly the pooled connection timeout and whether to enable
multiple http connections.
Lambda config
The default value for the idle connection timeout is 1 minute. We find
this too strict in lambda environments, where the container may freeze
and thaw in greater than 1 minute spans but less than the server
timeout (5 minutes as of writing). Therefore we set the lambda config
pooled connection idle timeout higher, to 6 minutes. That way a lambda
function will not needlessly reconnect when the connection is still
open.
Target framework
Previously we relied on .NET Framework 4.61 as the .NET Framework
target. The next minor version introduced async iterator features
necessary for the topic client. Because of this, at the time, we added
preprocessor directives to exclude the topic client if the build target
wasn't .NET Standard 2.0 or greater.
Now that we add another target framework, .NET 6.0, maintaining the
different platform directives is difficult. Instead we bump the Framework
target from net461 to net462 to gain the async iterators and remove
the platform conditional compilation directives. There is no customer
impact by removing exactly .NET 4.61 as a target framework.
.NET 4.62 also includes
WinHttpHandler, so we can revisit the use ofgRPC web in that Framework.