Tagging subscribers to this area: @dotnet/ncl
Notify danmosemsft if you want to be subscribed.

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@alnikola, I've been staring at the PR and I think I'm missing something fundamental. Where are connections throttled to the max limit set?

@alnikola, I've been staring at the PR and I think I'm missing something fundamental. Where are connections throttled to the max limit set?

It's in the Http2Connection. If the connection count == max, it will default to awaiting for a free stream on the connection instead of creating a new one.

Though I do wonder if in some cases, it might end up with creating more connections. Eventually throttling, but still creating some more than the setting says.

it will default to awaiting for a free stream on the connection instead of creating a new one

On which connection?

On which connection?

You're right, it seems on a new one. I should've known better and assume you already saw through this 😊

@stephentoub Throttling itself is triggered in Http2Connection.SendHeadersAsync by throwing an exception when the stream limit is reached, but the total number of connections is still below MaxHttp2ConnectionsPerServer as evaluated in HttpConnectionPool.ThrowOnStreamLimitReached. Further, the exception gets handled by catch block in HttpConnectionPool.SendWithRetryAsync that retries the request starting with a search for a new connection having stream count below the limit in GetHttp2ConnectionAsync -> GetHttp2ConnectionAcceptingNewStreams.
Later on, when the maximum of connections set by MaxHttp2ConnectionsPerServer is reached, the ThrowOnStreamLimitReached becomes false which restores the common code flow with streams queueing.

Later on, when the maximum of connections set by MaxHttp2ConnectionsPerServer is reached, the ThrowOnStreamLimitReached becomes false which restores the common code flow with streams queueing.

Where? GetHttp2ConnectionAsync calls GetHttp2ConnectionAcceptingNewStreams, which will return null if it couldn't find a connection with availability, at which point it will proceed to create a new connection. What am I missing, and where is the test that proves it?

If GetHttp2ConnectionAcceptingNewStreams returns null, GetHttp2ConnectionAsync proceeds further down to new HTTP/2 connection construction lock. After the lock is passed, it again checks for existing HTTP/2 connection with available streams (e.g. somebody has created a new connection while we were waiting on the lock). Finally, if no connection is found, it calls Http2Connection ctor either here or here, and then adds the new instance into _http2Connections collection in AddHttp2Connection.

@JamesNK It works almost exactly as you described with only one difference that the connection search goes from the newest to the oldest because I assumed it would be better to use the "hottest" connection. But, it can be easily changed if you need to reverse the order.

Ok. I don't have a strong preference between the two approaches.

By the way, what happens when a bunch of requests hit the limit at once and you need new connections, do you make one at a time and make the requests wait, or could you end up creating many at once, each with only one request?

Closing this PR for now in favor of an alternative simpler implementation #39439
The other version is about 8% to 11% slower in some scenarios, but a way simpler and lower risky.

Temporary re-open it to merge in the correct PrepareConnection implementation and make sure tests now pass.

Closing again for now.