Isolate long-polls and file transfers on separate HTTP/2 pools - #825
Conversation
…s on HTTP/1.1 httpcore blocks on a per-connection stream semaphore once slots fill instead of opening another connection. Patch is_available + non-blocking acquire so the pool spreads load. Route upload_file/download_file through a dedicated HTTP/1.1 pool so bulk bodies cannot starve latency-sensitive RPCs on the shared H2 session. Co-authored-by: Cursor <cursoragent@cursor.com>
Drop the httpcore monkeypatch. Route long-polls (/wait_for_status) and upload/download onto separate shared H2 bulkhead pools with configurable shards (default 2), hashed by resource id, so control-plane RPCs keep their own connection without HTTP/1.1 explosion. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Add TLS+ALPN integration tests that saturate waits / stall upload bodies and assert create lands on a different connection and stays fast. Guard sync bulkhead client creation with a lock and enumerate long-lived resource paths so new endpoints cannot silently join the API pool. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Replace CRC32 resource affinity with per-client counters so concurrent waits/uploads spread across shards instead of pinning a busy resource to one connection. Co-authored-by: Cursor <cursoragent@cursor.com>
Avoid every new SDK client landing first wait/upload on global shard 0 when transports are process-shared. Co-authored-by: Cursor <cursoragent@cursor.com>
| :param background_pool_shards: H2 shards for long-polls (round-robin), defaults to 2 | ||
| :type background_pool_shards: int, optional | ||
| :param transfer_pool_shards: H2 shards for upload/download (round-robin), defaults to 2 | ||
| :type transfer_pool_shards: int, optional |
There was a problem hiding this comment.
Hmm... It sounds like the python http stack doesn't auto-increase the number of connections? I'm surprised there isn't some way to enable that. (I think we should still have the separate pools -- it would just be nice if the shards-per-pool could scale with the request load...)
I'm also curious about the underlying problem here. HTTP/2 is supposed to share connections among many streams and break large messages on one stream into data frames to interleave them with smaller messages on other streams. I wonder if we could achieve the same thing by setting the stream config when we know we have large messages in either direction. (eg, lower the priority, set the interleave bit, potentially reduce the window size)
I believe the max streams-per-connection is set by the server, so that will influence things here as well. Do we know what Jetty & the NLB are doing here? (I imagine there might be some interplay b/w these as well?)
Regarding the pool sizes, 2 seems pretty small for the hanging-poll type messages. Clients might reasonably want to have 1000s of those at once. For the file upload/download pool, 2 also seems small, since it is often easier to saturate the link with a few more connections? (Then again, we could keep it small to start as a protection mechanism against accidentally spammy workloads...)
There was a problem hiding this comment.
Long poll doesn't wait forever, it's capped to 25-30s. It's not ideal but this prevents the connection from getting starved out
The other option is to just scrap http2 but that's also expensive for these types of long poll endpoints
There was a problem hiding this comment.
Ugh yeah - I just read up on python httpx - it really is just a single connect per server! That's a shockingly broken design...
Given that, I think it would be good to add a pool for the short-lived connections as well. It looks like the Jetty default is 128 streams/connection, so what about these pool sizes?
fast-pool: 5-10
long-polls: 10-20
data-xfer: 2
There was a problem hiding this comment.
Updated defaults to API 8 / background 16 / transfer 2, and the short-RPC (API) pool is sharded as well now.
| class _SharedTransport(httpx.BaseTransport): | ||
| """Refcounted wrapper: delegates to a real transport, closes it when refcount hits 0.""" | ||
|
|
||
| def __init__(self, transport: httpx.BaseTransport) -> None: |
There was a problem hiding this comment.
It might be cleaner to add the HTTP/2 connection pooling logic to the _SharedTransport class, so then we can just re-use that directly for the different request categories
There was a problem hiding this comment.
Good call — pooling now lives on _SharedTransport.Registry / _SharedAsyncTransport.Registry, with per-client _SyncClientPool / _AsyncClientPool wrappers. Routing is a single _get_client_for_path(path).
Also took your size guidance: defaults are now API 8, background 16, transfer 2, and the API pool is sharded too (not a single control-plane connection anymore).
| def _next_background_client(self) -> httpx.Client: | ||
| # Select under the lock; ensure afterward so _ensure_* can take the same lock. | ||
| with self._bulkhead_lock: | ||
| shard = self._background_next % self._background_pool_shards | ||
| self._background_next += 1 | ||
| return self._ensure_background_client(shard) |
There was a problem hiding this comment.
we've some duplication here that would be cleaner if we pushed it into the _SharedTransport class. I think that would simplify the locking and acquire logic as well, and reduce the chance of errors later on.
There was a problem hiding this comment.
Done — acquire/refcount/shard map are in the Registry; client wrappers no longer duplicate that locking.
| if _is_background_path(path): | ||
| return self._next_background_client() | ||
| if _is_transfer_path(path): | ||
| return self._next_transfer_client() |
There was a problem hiding this comment.
how about just:
client = get_client_for_path(path)
and then we push all of the pool management down in to the wrapper library?
There was a problem hiding this comment.
Switched to _get_client_for_path(path) as the single entry point; pool selection lives under that.
jrvb-rl
left a comment
There was a problem hiding this comment.
I like this direction - it definitely seems necessary given httpx's single-connection-per-server behavior for HTTP/2.
I think we can simplify this by encapsulating the pooling logic in the _SharedTransport class that we already have. See embedded comments below..
Move shard registries onto SharedTransport, route via get_client_for_path, and shard the API pool too (defaults: API 8 / background 16 / transfer 2). Co-authored-by: Cursor <cursoragent@cursor.com>
| self._shards.clear() | ||
| return values | ||
|
|
||
| def shard_ids(self) -> set[int]: |
There was a problem hiding this comment.
Let's note that this is used only for testing
There was a problem hiding this comment.
Done — take_all / shard_ids (and async clear) are documented as test-only.
jrvb-rl
left a comment
There was a problem hiding this comment.
Looks good - thanks for the tweaks!
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Problem
Under concurrent load, the Python SDK can stall short API calls (for example
devboxes.create) while many long-lived requests share one HTTP/2 connection.httpx/httpcore multiplexes streams on a single H2 connection and can block when that connection’s stream capacity is exhausted instead of opening another connection. Long-polls such as
wait_for_statusand largeupload_file/download_filebodies are the usual way to fill that capacity.Change
Route traffic onto three separate sharded HTTP/2 pools:
create,execute, list/get, …)/wait_for_status/upload_fileand/download_fileEach shard is roughly one H2 connection. Requests are assigned with per-client round-robin from a random start offset, so busy clients spread across shards and many short-lived clients do not all land on shard 0.
Pooling is encapsulated in
_SharedTransport.Registry/_SharedAsyncTransport.Registry, with per-client wrappers and a single_get_client_for_path(path)entry point.Passing a custom
http_clientdisables this isolation; that client owns the full transport stack.Shared transports remain process-wide so multiple SDK instances can reuse connections. Round-robin counters stay local to each SDK client.
Notes
*_pool_shardsincreases capacity.Test plan
createon the API pool