[tinker] Load harness for the API server's sampling path at 131k concurrency - #2166
Draft
avigyabb wants to merge 1 commit into
Draft
[tinker] Load harness for the API server's sampling path at 131k concurrency#2166avigyabb wants to merge 1 commit into
avigyabb wants to merge 1 commit into
Conversation
…urrency skyrl/benchmarks/load_test_tinker_sampling.py isolates the Tinker API server (FastAPI/uvicorn, SQLite, in-memory future store, forwarding client) from the engine, CPU only: the real skyrl.tinker.api app under uvicorn configured as the non-colocated megatron server with the engine subprocess stubbed and the router URL seeded into EngineStateDB; a fake vLLM router (barrier or fixed-latency mode, optionally multi-process); and a load client that behaves like the Tinker SDK (400 concurrent submits, sample_max_concurrent_requests cap, 45s re-polls, 16 connection retries with backoff) or drives the real SDK. Workers bind distinct loopback source IPs so one IP's ~28k ephemeral ports do not cap the run. Reports completed/failed by class, latency percentiles, the router's peak in-flight count, and the server's peak RSS, fds and /healthz latency; TINKER_LOADTEST_PROFILE=<path> cProfiles the server. Structure follows Chuck Tang's SQLite QueuePool repro gist (https://gist.github.com/j316chuck/f44f35572ffb8584519d13b943f99ef8). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Avi Basnet <avigyabb@stanford.edu>
This was referenced Sep 5, 2026
avigyabb
added a commit
that referenced
this pull request
Sep 5, 2026
#2160) Stack 1/7. Independent of the rest. RichHandler renders each access-log record through a rich Table, about 1.5 ms of event-loop CPU per HTTP request. Under rollout load that was 63% of the Tinker API server's CPU: profiling a 4096-sample run showed 12.5 s of its 19.9 s of server CPU inside `rich.logging.emit`. With a plain `StreamHandler` for `uvicorn.access` the same run takes 9.7 s and server throughput goes from 337 to 568 samples/s. Startup and error logging keep the Rich handler. Measured with the load harness in stack 7/7 (`skyrl/benchmarks/load_test_tinker_sampling.py`). **Stack** (each PR retargets to `main` as the one below merges) 1. #2160 2. #2161 3. #2162 4. #2163 5. #2164 6. #2165 7. #2166 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Logging configuration only; no request handling, auth, or data path changes—access output format may look slightly plainer on stderr. > > **Overview** > **HTTP access logging** for the Tinker API (via `get_uvicorn_log_config`) no longer goes through `RichHandler`. A dedicated **`access`** `StreamHandler` on stderr uses the same text formatter, while **`uvicorn`** and **`uvicorn.error`** still use Rich for startup, errors, and tracebacks. > > This targets per-request access log volume: Rich’s table rendering was a major event-loop CPU cost under high QPS. Access lines should read the same format string but without Rich styling. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 254499a. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Signed-off-by: Avi Basnet <avigyabb@stanford.edu> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
avigyabb
added a commit
that referenced
this pull request
Sep 5, 2026
…g, keep-alive) (#2163) Stack 4/7. `uvicorn.run`: `backlog=SKYRL_HTTP_CONNECTION_LIMIT` (50k, as on Chuck's branch; the effective value is capped by `net.core.somaxconn`, raise it to match) and `timeout_keep_alive=75`. With 131072 outstanding samples and 212k-token results, each 2048-result completion burst kept the event loop busy ~16 s; uvicorn's 5 s keep-alive then closed every idle client connection, all clients reconnected at once and the 2048-entry accept backlog overflowed, refusing 109k of 131072 requests. With these settings the same run completed 130917 of 131072 with zero forwarding errors and zero reconnects (earlier runs on the 5 s keep-alive showed hundreds to thousands). Neither setting is needed for correctness: the SDK retries refused or dropped connections. They avoid the reconnect storm rather than fix a failure, and with the SDK's per-client in-flight cap the burst that overflowed the backlog does not occur. An earlier revision of this PR also exposed `sample_max_concurrent_requests` from `EngineConfig` via `/client/config`; that was dropped as unnecessary (its default equalled the SDK default, and no measured run depended on it). **Stack** (each PR retargets to `main` as the one below merges) 1. #2160 2. #2161 3. #2162 4. #2163 5. #2164 6. #2165 7. #2166 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Server-only uvicorn listen/keep-alive defaults; no API or auth behavior change, with SDK retries as a fallback. > > **Overview** > The Tinker API server now passes **uvicorn** socket tuning so large completion bursts do not trigger mass client reconnects and accept-queue overflows. > > **`timeout_keep_alive`** is set to **75s** (via `HTTP_KEEP_ALIVE_TIMEOUT_SECONDS`) instead of uvicorn’s **5s** default, so idle SDK connections stay open while the event loop is busy for many seconds during bursts. > > **`backlog`** is set to **`SKYRL_HTTP_CONNECTION_LIMIT`** (default **50k**, overridable by env), so pending connections queue in the kernel instead of being refused when the loop cannot accept fast enough (effective cap is **`net.core.somaxconn`**). > > These are **reliability/performance** knobs, not correctness fixes—the SDK already retries refused or dropped connections. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 27c838e. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Signed-off-by: Avi Basnet <avigyabb@stanford.edu> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
avigyabb
added a commit
that referenced
this pull request
Sep 5, 2026
…2162) Stack 3/7. Fixes the 128x128 `404 Future not found` seen against j316chuck#18. **Chain.** The SDK polls `retrieve_future` with a 45 s client timeout and gives up; the result lands afterwards; the abandoned handler wakes, builds a response nobody receives (uvicorn drops the send to a dead client silently) and starts the short retrieved-TTL clock; the sweeper evicts the result 120 s later; the SDK's retry of the same request_id gets 404, which the SDK treats as fatal. **Fix.** Start the retrieved clock only if `request.is_disconnected()` is false, and raise the retrieved TTL to 300 s so it outlasts the SDK's worst-case re-poll gap (45 s timeout + up to 30 s backoff, twice). `tests/tinker/test_retrieve_future_lost_response.py` reproduces the chain under a real uvicorn socket with shortened TTLs; it fails on `main` and passes here. A second test checks a delivered result still expires on the short clock, so memory stays bounded. Alternative considered: j316chuck#19 drops the retrieved clock and keeps every result for 2048 s. That also fixes the 404 but retains ~35 minutes of results regardless of delivery; with long-output rollouts (hundreds of KB per result) that is tens of GB. Verified at scale: 131072 requests with 5 s engine queueing and 224k SDK-style abandoned polls completed with zero 404s. **Stack** (each PR retargets to `main` as the one below merges) 1. #2160 2. #2161 3. #2162 4. #2163 5. #2164 6. #2165 7. #2166 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes async polling/delivery semantics and HTTP server tuning on the hot `retrieve_future` path; behavior is covered by new integration tests but affects SDK retry reliability under load. > > **Overview** > Fixes fatal **`404 Future not found`** when the SDK abandons a long `retrieve_future` poll (45s client timeout) and retries the same `request_id` after the result is ready. > > **`retrieve_future`** now calls **`mark_retrieved`** (starting the post-delivery eviction clock) only when the client is still connected (`not await req.is_disconnected()`). If the handler finishes building a response after the client disconnected, the short retrieved TTL no longer starts, so the in-memory store keeps the result for a real retry. > > **`ExternalFutureStore`** raises **`_RETRIEVED_TTL_SECONDS`** from 120s to 300s so delivered results still get a grace window that covers worst-case SDK re-poll gaps (timeout + backoff, twice). > > **Uvicorn** startup sets **`timeout_keep_alive=75`** (vs 5s default) and **`backlog=SKYRL_HTTP_CONNECTION_LIMIT`** to reduce idle disconnects and accept-queue overflows during completion bursts. > > Adds **`test_retrieve_future_lost_response.py`** (real socket on Linux) plus test stubs for **`is_disconnected`** on existing API tests. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 36188da. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Signed-off-by: Avi Basnet <avigyabb@stanford.edu> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
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.
Stack 7/7. The harness that produced every number in stacks 1-6.
skyrl/benchmarks/load_test_tinker_sampling.pyisolates the Tinker API server (FastAPI/uvicorn, SQLite, in-memory future store, forwarding client) from the engine, CPU only: the realskyrl.tinker.apiapp under uvicorn configured as the non-colocated megatron server with the engine subprocess stubbed and the router URL seeded intoEngineStateDB; a fake vLLM router (barrier or fixed-latency mode, optionally multi-process for multi-MB results); and a load client that behaves like the Tinker SDK (400 concurrent submits,sample_max_concurrent_requestscap, 45 s re-polls, 16 connection retries with backoff) or drives the real SDK (--client sdk). Workers bind distinct loopback source IPs so one IP's ~28k ephemeral ports do not cap the run. Reports completed/failed by class, latency percentiles, the router's peak in-flight count, and the server's peak RSS, fds and/healthzlatency;TINKER_LOADTEST_PROFILE=<path>cProfiles the server process.uv run --extra tinker python skyrl/benchmarks/load_test_tinker_sampling.py \ --num-requests 131072 --forwarding-max-connections 2048 # realistic engine queueing, long outputs, SDK cap uv run --extra tinker python skyrl/benchmarks/load_test_tinker_sampling.py \ --num-requests 131072 --max-tokens 8192 --proto --forwarding-max-connections 2048 \ --vllm-mode latency --max-num-seqs 2048 --gen-seconds 5 --max-outstanding 16384 # against a real server with the public SDK uv run --extra tinker python skyrl/benchmarks/load_test_tinker_sampling.py \ --role load --client sdk --url http://HOST:8000 --num-requests 4096Structure follows Chuck Tang's SQLite QueuePool repro gist (https://gist.github.com/j316chuck/f44f35572ffb8584519d13b943f99ef8). Run against pre-#2097
main(59d4daed) with 262k-token results it reproduces his 32x64 collapse: 18 of 2048 completed in 39 min with 4218QueuePoolerrors.What it does not model: real generation time (production is far gentler on the server than the harness), engine and weight-sync stalls, training traffic on the same SQLite file, and a real network between SDK and server.
Stack (each PR retargets to
mainas the one below merges)🤖 Generated with Claude Code