Skip to content

Commit b6451ee

Browse files
Reflexcursoragent
andcommitted
fix: round-robin H2 bulkhead shards per SDK client
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>
1 parent 8bdcd53 commit b6451ee

5 files changed

Lines changed: 69 additions & 67 deletions

File tree

src/runloop_api_client/_base_client.py

Lines changed: 38 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
import json
55
import time
66
import uuid
7-
import zlib
87
import email
98
import asyncio
109
import inspect
@@ -179,7 +178,8 @@ async def aclose(self) -> None:
179178

180179
# Sharded H2 bulkheads: long-polls and file transfers stay off the API control-plane
181180
# connection. Each shard index maps to its own shared transport (≈ one H2 connection).
182-
# Removable once httpcore respects stream capacity when opening connections.
181+
# Per-client round-robin spreads concurrent requests across shards. Removable once
182+
# httpcore respects stream capacity when opening connections.
183183
_shared_sync_background_transports: dict[int, _SharedTransport] = {}
184184
_shared_sync_transfer_transports: dict[int, _SharedTransport] = {}
185185
_shared_async_background_transports: weakref.WeakKeyDictionary[
@@ -201,30 +201,6 @@ def _is_transfer_path(path: str) -> bool:
201201
return path.endswith(_TRANSFER_PATH_SUFFIXES)
202202

203203

204-
def _pool_affinity_key(path: str) -> str:
205-
"""Pick a stable resource id from the URL for shard routing."""
206-
parts = [p for p in path.split("/") if p]
207-
try:
208-
if "executions" in parts:
209-
idx = parts.index("executions")
210-
if idx + 1 < len(parts):
211-
return parts[idx + 1]
212-
if "devboxes" in parts:
213-
idx = parts.index("devboxes")
214-
if idx + 1 < len(parts):
215-
return parts[idx + 1]
216-
except ValueError:
217-
pass
218-
return path
219-
220-
221-
def _shard_index(key: str, shards: int) -> int:
222-
if shards <= 1:
223-
return 0
224-
# crc32 is stable across processes (unlike PYTHONHASHSEED-randomized hash()).
225-
return zlib.crc32(key.encode("utf-8")) % shards
226-
227-
228204
def _acquire_shared_sync_transport(bucket: dict[int, _SharedTransport], shard: int) -> _SharedTransport:
229205
with _pool_lock:
230206
existing = bucket.get(shard)
@@ -1019,6 +995,8 @@ class SyncAPIClient(BaseClient[httpx.Client, Stream[Any]]):
1019995
_isolate_workload_pools: bool
1020996
_background_pool_shards: int
1021997
_transfer_pool_shards: int
998+
_background_next: int
999+
_transfer_next: int
10221000
_closed: bool
10231001

10241002
def __init__(
@@ -1076,6 +1054,8 @@ def __init__(
10761054
self._bulkhead_lock = threading.Lock()
10771055
self._background_pool_shards = background_pool_shards
10781056
self._transfer_pool_shards = transfer_pool_shards
1057+
self._background_next = 0
1058+
self._transfer_next = 0
10791059
# Custom http_client owns the full transport stack; don't invent sibling pools.
10801060
self._isolate_workload_pools = http_client is None
10811061

@@ -1151,15 +1131,27 @@ def _ensure_transfer_client(self, shard: int) -> httpx.Client:
11511131
self._transfer_clients[shard] = client
11521132
return client
11531133

1134+
def _next_background_client(self) -> httpx.Client:
1135+
# Select under the lock; ensure afterward so _ensure_* can take the same lock.
1136+
with self._bulkhead_lock:
1137+
shard = self._background_next % self._background_pool_shards
1138+
self._background_next += 1
1139+
return self._ensure_background_client(shard)
1140+
1141+
def _next_transfer_client(self) -> httpx.Client:
1142+
with self._bulkhead_lock:
1143+
shard = self._transfer_next % self._transfer_pool_shards
1144+
self._transfer_next += 1
1145+
return self._ensure_transfer_client(shard)
1146+
11541147
def _send_client_for_request(self, request: httpx.Request) -> httpx.Client:
11551148
if not self._isolate_workload_pools:
11561149
return self._client
11571150
path = request.url.path
1158-
key = _pool_affinity_key(path)
11591151
if _is_background_path(path):
1160-
return self._ensure_background_client(_shard_index(key, self._background_pool_shards))
1152+
return self._next_background_client()
11611153
if _is_transfer_path(path):
1162-
return self._ensure_transfer_client(_shard_index(key, self._transfer_pool_shards))
1154+
return self._next_transfer_client()
11631155
return self._client
11641156

11651157
def is_closed(self) -> bool:
@@ -1736,6 +1728,8 @@ class AsyncAPIClient(BaseClient[httpx.AsyncClient, AsyncStream[Any]]):
17361728
_isolate_workload_pools: bool
17371729
_background_pool_shards: int
17381730
_transfer_pool_shards: int
1731+
_background_next: int
1732+
_transfer_next: int
17391733
_closed: bool
17401734

17411735
def __init__(
@@ -1792,6 +1786,8 @@ def __init__(
17921786
self._transfer_clients = {}
17931787
self._background_pool_shards = background_pool_shards
17941788
self._transfer_pool_shards = transfer_pool_shards
1789+
self._background_next = 0
1790+
self._transfer_next = 0
17951791
# Custom http_client owns the full transport stack; don't invent sibling pools.
17961792
self._isolate_workload_pools = http_client is None
17971793

@@ -1877,15 +1873,25 @@ def _ensure_transfer_client(self, shard: int) -> httpx.AsyncClient:
18771873
self._transfer_clients[shard] = client
18781874
return client
18791875

1876+
def _next_background_client(self) -> httpx.AsyncClient:
1877+
# Single-threaded event loop: counter bump needs no lock when there is no await.
1878+
shard = self._background_next % self._background_pool_shards
1879+
self._background_next += 1
1880+
return self._ensure_background_client(shard)
1881+
1882+
def _next_transfer_client(self) -> httpx.AsyncClient:
1883+
shard = self._transfer_next % self._transfer_pool_shards
1884+
self._transfer_next += 1
1885+
return self._ensure_transfer_client(shard)
1886+
18801887
def _send_client_for_request(self, request: httpx.Request) -> httpx.AsyncClient:
18811888
if not self._isolate_workload_pools:
18821889
return self._client
18831890
path = request.url.path
1884-
key = _pool_affinity_key(path)
18851891
if _is_background_path(path):
1886-
return self._ensure_background_client(_shard_index(key, self._background_pool_shards))
1892+
return self._next_background_client()
18871893
if _is_transfer_path(path):
1888-
return self._ensure_transfer_client(_shard_index(key, self._transfer_pool_shards))
1894+
return self._next_transfer_client()
18891895
return self._client
18901896

18911897
def is_closed(self) -> bool:

src/runloop_api_client/_client.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ def __init__(
9898
# Set to False to create a private connection pool (old behavior).
9999
shared_http_pool: bool = True,
100100
# Separate H2 connection pools for long-polls (/wait_for_status) and file
101-
# transfers. Each shard ≈ one H2 connection, selected by hash(resource_id).
101+
# transfers. Each shard ≈ one H2 connection; requests round-robin per client.
102102
background_pool_shards: int = DEFAULT_BACKGROUND_POOL_SHARDS,
103103
transfer_pool_shards: int = DEFAULT_TRANSFER_POOL_SHARDS,
104104
# Enable or disable schema validation for data returned by the API.
@@ -406,7 +406,7 @@ def __init__(
406406
# Set to False to create a private connection pool (old behavior).
407407
shared_http_pool: bool = True,
408408
# Separate H2 connection pools for long-polls (/wait_for_status) and file
409-
# transfers. Each shard ≈ one H2 connection, selected by hash(resource_id).
409+
# transfers. Each shard ≈ one H2 connection; requests round-robin per client.
410410
background_pool_shards: int = DEFAULT_BACKGROUND_POOL_SHARDS,
411411
transfer_pool_shards: int = DEFAULT_TRANSFER_POOL_SHARDS,
412412
# Enable or disable schema validation for data returned by the API.

src/runloop_api_client/sdk/async_.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1349,9 +1349,9 @@ def __init__(
13491349
:type default_query: Mapping[str, object] | None, optional
13501350
:param http_client: Custom ``httpx.AsyncClient`` instance to reuse, defaults to None
13511351
:type http_client: httpx.AsyncClient | None, optional
1352-
:param background_pool_shards: H2 shards for long-polls, defaults to 2
1352+
:param background_pool_shards: H2 shards for long-polls (round-robin), defaults to 2
13531353
:type background_pool_shards: int, optional
1354-
:param transfer_pool_shards: H2 shards for upload/download, defaults to 2
1354+
:param transfer_pool_shards: H2 shards for upload/download (round-robin), defaults to 2
13551355
:type transfer_pool_shards: int, optional
13561356
"""
13571357
self.api = AsyncRunloop(

src/runloop_api_client/sdk/sync.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1374,9 +1374,9 @@ def __init__(
13741374
:type default_query: Mapping[str, object] | None, optional
13751375
:param http_client: Custom ``httpx.Client`` instance to reuse, defaults to None
13761376
:type http_client: httpx.Client | None, optional
1377-
:param background_pool_shards: H2 shards for long-polls, defaults to 2
1377+
:param background_pool_shards: H2 shards for long-polls (round-robin), defaults to 2
13781378
:type background_pool_shards: int, optional
1379-
:param transfer_pool_shards: H2 shards for upload/download, defaults to 2
1379+
:param transfer_pool_shards: H2 shards for upload/download (round-robin), defaults to 2
13801380
:type transfer_pool_shards: int, optional
13811381
"""
13821382
self.api = Runloop(

tests/test_transfer_client.py

Lines changed: 25 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,7 @@
1414
import runloop_api_client._base_client as _base_mod
1515
from runloop_api_client import Runloop, AsyncRunloop
1616
from runloop_api_client._base_client import (
17-
_shard_index,
1817
_is_transfer_path,
19-
_pool_affinity_key,
2018
_is_background_path,
2119
)
2220

@@ -77,15 +75,6 @@ def test_path_classification() -> None:
7775
assert _is_transfer_path("/v1/objects/obj_1/download") is False
7876

7977

80-
def test_affinity_key_and_shard() -> None:
81-
assert _pool_affinity_key("/v1/devboxes/dbx_1/upload_file") == "dbx_1"
82-
assert _pool_affinity_key("/v1/devboxes/dbx_1/wait_for_status") == "dbx_1"
83-
assert _pool_affinity_key("/v1/devboxes/dbx_1/executions/ex_9/wait_for_status") == "ex_9"
84-
assert _shard_index("dbx_1", 1) == 0
85-
assert _shard_index("dbx_1", 2) in (0, 1)
86-
assert _shard_index("dbx_1", 2) == _shard_index("dbx_1", 2)
87-
88-
8978
def test_api_background_transfer_use_distinct_transports() -> None:
9079
client = _make_client(shared_http_pool=True, background_pool_shards=2, transfer_pool_shards=2)
9180
try:
@@ -108,26 +97,24 @@ def test_api_background_transfer_use_distinct_transports() -> None:
10897
client.close()
10998

11099

111-
def test_same_resource_affinity_uses_same_shard() -> None:
100+
def test_round_robin_spreads_requests_across_shards() -> None:
112101
client = _make_client(background_pool_shards=2, transfer_pool_shards=2)
113102
try:
114-
w1 = client._send_client_for_request(httpx.Request("POST", f"{base_url}/v1/devboxes/dbx_same/wait_for_status"))
115-
w2 = client._send_client_for_request(httpx.Request("POST", f"{base_url}/v1/devboxes/dbx_same/wait_for_status"))
116-
assert w1 is w2
117-
finally:
118-
client.close()
119-
120-
121-
def test_different_resources_can_land_on_different_shards() -> None:
122-
client = _make_client(background_pool_shards=2)
123-
try:
124-
seen: set[int] = set()
125-
for i in range(40):
126-
req = httpx.Request("POST", f"{base_url}/v1/devboxes/dbx_{i}/wait_for_status")
127-
c = client._send_client_for_request(req)
128-
seen.add(id(c._transport)) # type: ignore[attr-defined]
129-
assert len(seen) == 2
103+
wait_req = httpx.Request("POST", f"{base_url}/v1/devboxes/dbx_same/wait_for_status")
104+
w0 = client._send_client_for_request(wait_req)
105+
w1 = client._send_client_for_request(wait_req)
106+
w2 = client._send_client_for_request(wait_req)
107+
assert w0 is not w1
108+
assert w0 is w2
109+
assert set(client._background_clients) == {0, 1}
110+
111+
upload_req = httpx.Request("POST", f"{base_url}/v1/devboxes/dbx_same/upload_file")
112+
t0 = client._send_client_for_request(upload_req)
113+
t1 = client._send_client_for_request(upload_req)
114+
assert t0 is not t1
115+
assert set(client._transfer_clients) == {0, 1}
130116
assert len(_base_mod._shared_sync_background_transports) == 2
117+
assert len(_base_mod._shared_sync_transfer_transports) == 2
131118
finally:
132119
client.close()
133120

@@ -145,15 +132,24 @@ def test_custom_http_client_skips_isolation() -> None:
145132
custom.close()
146133

147134

148-
def test_shards_shared_across_sdk_clients() -> None:
135+
def test_round_robin_is_per_client_while_transports_are_shared() -> None:
149136
c1 = _make_client(background_pool_shards=2)
150137
c2 = _make_client(background_pool_shards=2)
151138
try:
152139
req = httpx.Request("POST", f"{base_url}/v1/devboxes/dbx_shared/wait_for_status")
140+
# Each SDK client has its own counter; both start at shard 0.
153141
t1 = c1._send_client_for_request(req)
154142
t2 = c2._send_client_for_request(req)
155143
assert t1 is not t2
156144
assert t1._transport is t2._transport # type: ignore[attr-defined]
145+
assert c1._background_next == 1
146+
assert c2._background_next == 1
147+
148+
# Advancing one client does not affect the other.
149+
t1b = c1._send_client_for_request(req)
150+
assert t1b is not t1
151+
assert c1._background_next == 2
152+
assert c2._background_next == 1
157153
finally:
158154
c1.close()
159155
c2.close()

0 commit comments

Comments
 (0)