SocketsHttpHandler should use first available connection (don't wait on new connection creation)

[geoffkizer]

AB#1254012
Currently, when we have no idle HTTP/1.1 connections (and are not at the connection limit) and a new request comes in, we will create a new connection and then use it for the new request.

The connect itself may take a while, especially for HTTPS connections. During this time, an existing connection may complete its current request and become available. If so, we should just use this idle connection instead of waiting for the new connection to finish its connect.

This should help request latency, and probably reduce the number of connections created during burst usage as well (see #43764).

More details:

When this happens, the new connection can still be put in the pool as idle, even if we don't have a request for it at the moment. If another new request comes in, we will use it like any other idle connection.

Note also that we should avoid creating new connections when we already have enough pending connections to (eventually) handle all the pending requests. Consider this scenario, starting with no connections:

• Request A arrives, we initiate a connect for Connection 1, wait for it, and start processing A on it
• Request B arrives, we initiate a connect for Connection 2. Before the connect completes, A finishes and Connection 1 becomes available. We start processing B on Connection 1.
• Request C arrives. There's no established connection for it to use, but there is a pending connection (Connection 2). So there's no need for us to initiate another connection right now.
• Request D arrives. Now, we have 0 available connections, 1 pending connection (Connection 2), and 2 pending requests (C and D). So we should go ahead and initiate a new connect for Connection 3.

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Description:	Currently, when we have no idle HTTP/1.1 connections (and are not at the connection limit) and a new request comes in, we will create a new connection and then use it for the new request. The connect itself may take a while, especially for HTTPS connections. During this time, an existing connection may complete its current request and become available. If so, we should just use this idle connection instead of waiting for the new connection to finish its connect. This should help request latency, and probably reduce the number of connections created during burst usage as well (see #43764). More details: When this happens, the new connection can still be put in the pool as idle, even if we don't have a request for it at the moment. If another new request comes in, we will use it like any other idle connection. Note also that we should avoid creating new connections when we already have enough pending connections to (eventually) handle all the pending requests. Consider this scenario, starting with no connections: Request A arrives, we initiate a connect for Connection 1, wait for it, and start processing A on it Request B arrives, we initiate a connect for Connection 2. Before the connect completes, A finishes and Connection 1 becomes available. We start processing B on Connection 1. Request C arrives. There's no established connection for it to use, but there is a pending connection (Connection 2). So there's no need for us to initiate another connection right now. Request D arrives. Now, we have 0 available connections, 1 pending connection (Connection 2), and 2 pending requests (C and D). So we should go ahead and initiate a new connect for Connection 3.
Author:	geoffkizer
Assignees:	-
Labels:	area-System.Net.Http, tenet-performance
Milestone:	[object Object]

[Tratcher]

Could the same approach be used for HTTP/2 when EnableMultipleHttp2Connections is enabled?

[geoffkizer]

Could the same approach be used for HTTP/2 when EnableMultipleHttp2Connections is enabled?

Potentially, yeah. It's a different set of logic though. The way EnableMultipleHttp2Connections works is a bit wonky today, so we should probably consider this as part of a general cleanup here.

[stephentoub]

Seems like a reasonable thing to do, albeit likely adding some implementation trickiness (in particular when it intersects with a set MaxConnectionsPerServer) to what is largely a straightforward process today.