Maintain connection reservations incrementally in the pool#1076
Open
Kludex wants to merge 4 commits into
Open
Conversation
Assignment does not change an HTTP/1.1 connection's state, so a newly idle connection was handed to every queued request in one pass and re-picked by later passes until the winner sent on it. Every loser woke up, failed with ConnectionNotAvailable, re-entered the queue, and triggered another full O(n) assignment pass - quadratic churn at high queue depth. Drop a connection from the availability snapshot once assigned, and exclude idle connections already reserved by an in-flight request when building the snapshot. 1000 concurrent requests against a local server drop from 5.1s to 0.9s with the default pool, and from 100s to 1.1s with max_connections=1. HTTP/2 multiplexing is unaffected: an active h2 connection is not idle, so it stays available to additional streams.
An idle HTTP/2 connection can serve further requests while reserved, so treating it like HTTP/1.1 could leave a queued burst waiting for the next pool event instead of multiplexing. Add can_multiplex() to the connection interface (False by default, True for established HTTP/2) and only apply the reserved-idle exclusion to connections that cannot multiplex.
Each assignment pass walked every in-flight request even when the pool was saturated, and re-probed reserved idle connections for expiry with an is_readable socket check on every interleaved pass. Break out of the assignment loop once no connection is available and no new one may be created, and skip expiry checks and surplus-keepalive eviction for connections reserved by an assigned request - they were health-checked at assignment time, and evicting them would hand the winning request a closed connection. 1000 unbounded concurrent requests against a local server now complete in 0.41s versus 0.51s sequential, compared to 0.93s before this change and 5.1s before #1075.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Stacked on #1075. Implements the follow-up suggested by @11kkw there: the last O(in-flight) cost per assignment pass was rebuilding
request_connectionsfrom the full request list, which is quadratic in aggregate across a large burst.Changes
{r.connection for r in self._requests}rebuild with a refcounteddict[connection, int]on the pool.ConnectionNotAvailablerequeue (now under the thread lock), cancellation, and response close.Benchmarks
Unbounded concurrent requests against a local uvicorn server, best of 3:
Throughput on #1075 decays as burst size grows (the residual quadratic term); with this change it stays flat, making total pool work effectively linear.
Verification
http2=Trueunchanged; streaming, cancellation storms, keepalive expiry,PoolTimeout, and threaded syncClientall behave as before.AI Disclaimer
This PR was developed with the assistance of either Claude or Codex. I've reviewed and verified the changes.