Issue at most one connection attempt per request#67114
Issue at most one connection attempt per request#67114MihaZupan merged 5 commits intodotnet:mainfrom
Conversation
|
Tagging subscribers to this area: @dotnet/ncl Issue DetailsThis PR changes how we associate connection attempts to requests: Instead of always associating with the first request in the queue, each request in the queue will be associated with at most one connection attempt. If the connection ends up establishing, it will always process the associated request first, before looking at the rest of the queue. This means that we may now have completed requests amongst the canceled and pending ones in the queue. To track which requests already have an associated connection attempt, the
|
|
/azp run runtime-libraries-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| Task.Run(() => AddHttp11ConnectionAsync(request)); | ||
| RequestQueueItem<HttpConnection> queueItem = _http11RequestQueue.PeekNextRequestForConnectionAttempt(); | ||
|
|
||
| Task.Run(() => AddHttp11ConnectionAsync(queueItem)); |
There was a problem hiding this comment.
This change is also avoiding a closure allocation on the fast path that seems to have snuck into CheckForHttp11ConnectionInjection.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs
Show resolved
Hide resolved
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs
Show resolved
Hide resolved
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs
Show resolved
Hide resolved
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs
Show resolved
Hide resolved
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs
Outdated
Show resolved
Hide resolved
* Issue at most one connection attempt per request * Drop the Try- from PeekNextRequestForConnectionAttempt * Add test * PR feedback * Entry => QueueItem
ghost
locked as resolved and limited conversation to collaborators
2 participants
This PR changes how we associate connection attempts to requests:
Instead of always associating with the first request in the queue, each request in the queue will be associated with at most one connection attempt.
If the connection ends up establishing, it will always process the associated request first, before looking at the rest of the queue.
If it fails, it will fail only the associated request, as long as it wasn't already processed by a different connection.
This means that we may now have completed requests amongst the canceled and pending ones in the queue.
We are relying on other connections to prune the head of the queue, similar to #66992.
This is something we could improve in the future (immediately remove such entries to reduce memory usage).
To track which requests already have an associated connection attempt, the
RequestQueuekeeps an extra head pointer (_attemptedConnectionsOffset). This pointer tracks the first request without an associated connection attempt.