[tinker] Hold 131k concurrent samples incl. long outputs: aiohttp forwarding, proto-once results, cheap access log, undelivered-result fix, load harness - #9
Closed
avigyabb wants to merge 2 commits into
Conversation
…ss log, undelivered-result fix, load harness Measured with a CPU-only harness that runs the real API server (backend=megatron, colocate_all=false, engine stubbed, fake vLLM router) and a client that behaves like the Tinker SDK (400 concurrent submits, 45s re-polls, retries on reset). - Forward samples with aiohttp instead of httpx. httpcore's pool rescans every connection for every request, so per-request CPU grew with the number of in-flight samples (14ms at 512, 27ms at 2048, 38ms at 4096); at 2048 in flight the server needed 119s to forward 2048 instant requests. aiohttp stays at 0.34ms. The connector limit is forwarding_inference_max_connections and queued requests wait with no deadline. Connect-phase errors and 5xx rejections (TransientInferenceError) are retried once; read failures stay final since vLLM may still be generating. - Route uvicorn's access log to a plain handler. RichHandler rendered each record through a rich Table at ~1.5ms of event-loop CPU per HTTP request, 63% of server CPU under load. Throughput went from 337 to 568 samples/s. - Start the retrieved-TTL clock only for a client that is still connected. A poll the SDK abandoned after its 45s timeout woke on a dead connection, marked the result retrieved, and the 120s sweeper evicted it before the SDK's retry, which then got 404 "Future not found". uvicorn drops the send silently, so check request.is_disconnected() first, and raise the retrieved TTL to 300s to outlast the SDK's worst-case re-poll gap. - forwarding_inference_timeout_sec default 300s -> 2048s (bursts wait inside vLLM's queue when connections are unlimited); new EngineConfig sample_max_concurrent_requests served by /api/v1/client/config. - Add skyrl/benchmarks/load_test_tinker_sampling.py and a regression test for the abandoned-poll -> 404 chain under a real uvicorn socket. Results with a 2048-connection cap: 131072/131072 samples complete in barrier mode (568/s, 1.2GB RSS) and with 5s engine queueing plus 224k SDK-style re-polls (0 failures, 4.9GB RSS); holding all 131072 outstanding at once costs 133k fds and 6.4GB. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Avi Basnet <avigyabb@stanford.edu>
… as-is Long-output rollouts made the per-result payload work the API server's main cost. For a 32k-token result (356KB JSON) the forwarding path spent 4ms (orjson decode, pydantic validate, pydantic JSON dump) and the proto path the SDK >= 0.25 uses on retrieve_future spent another 7ms (stdlib json.loads plus proto build) inside a single global lock, capping proto delivery near 150 results/s regardless of concurrency; the proto build holds the GIL, so the thread hop bought nothing. - The forwarding client decodes the vLLM body once and encodes straight to SampleResponse wire bytes (serialize_sample_output, shared with the validated path and pinned to it byte for byte by tests). No pydantic model or JSON text is built for the result. - ExternalFutureStore keeps the proto bytes (8 bytes/token, 26% smaller than the JSON text); retrieve_future passes them through for proto clients and derives JSON lazily, cached, for pre-proto clients (sample_output_json_from_proto). Results stored as JSON (DB path, errors) keep the existing encode-in-thread path, now cached per entry. - Pending entries no longer retain the request body (never read back on this path; ~100KB per entry for long prompts). - Store TTLs are EngineConfig fields (external_future_retrieved_ttl_sec, external_future_completed_ttl_sec): retention after delivery is the dominant memory term, roughly completion rate x result size x window. 512 concurrent 32k-token results, proto client: server CPU 7.5s -> 3.5s, 73 -> 167 results/s. 131072 requests with 8k-token results, 2048-way engine queueing and SDK-style 45s re-polls: 131072/131072, 0 failures, 267/s, peak RSS 8.9GB. 32768 requests with 32k-token results: 32768/32768, 163/s, 9.6GB. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Avi Basnet <avigyabb@stanford.edu>
avigyabb
commented
Sep 3, 2026
| # Tinker's stop_reason is Literal["stop", "length"]; vLLM emits a wider set. | ||
| finish_reason = choice.get("finish_reason") | ||
| stop_reason = "stop" if finish_reason in ("stop", "stop_token") else "length" | ||
| sequences.append( |
Owner
Author
There was a problem hiding this comment.
we don't want to convert tokens, logprobs, stop_reason to python objects
Owner
Author
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
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.
Goal: make sure the Tinker API server (FastAPI/uvicorn/SQLite/in-memory store/forwarding client) is not the bottleneck at 131072 concurrent sample requests on the non-colocated megatron path, and give the Trajectory team a fast, GPU-free way to check that. Based on upstream
main(345ce86).Harness
skyrl/benchmarks/load_test_tinker_sampling.pyruns the realskyrl.tinker.apiapp under uvicorn withbackend=megatron,colocate_all=false, the engine subprocess stubbed and the router URL seeded intoEngineStateDB, a fake vLLM router (barrier or fixed-latency mode), and a multi-process client that behaves like the Tinker SDK (400 concurrent submits, 45s re-polls, retries on reset) and binds several loopback source IPs so it is not capped by one IP's ~28k ephemeral ports.uv run --extra tinker python skyrl/benchmarks/load_test_tinker_sampling.py \ --num-requests 131072 --forwarding-max-connections 2048 # realistic engine queueing: 2048 concurrent generations of 5s each uv run --extra tinker python skyrl/benchmarks/load_test_tinker_sampling.py \ --num-requests 131072 --forwarding-max-connections 2048 \ --vllm-mode latency --max-num-seqs 2048 --gen-seconds 5 # 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 4096What it found and what this changes
httpx forwarding pool is quadratic in in-flight requests. httpcore's
_assign_requests_to_connectionsrescans every pooled connection for every request; profiled at 27.5Mis_idlecalls for 512 requests. Per-request CPU: 14ms at 512 in flight, 27ms at 2048, 38ms at 4096. At 2048 in flight the server needed 119s to forward 2048 instant requests. aiohttp is flat at 0.34ms, soSkyRLTrainInferenceForwardingClientnow uses aiohttp:TCPConnector(limit=forwarding_inference_max_connections),sock_read=forwarding_inference_timeout_sec, no total deadline so queued requests wait for the engine instead of hitting the old 300s pool timeout.Access log through RichHandler cost ~1.5ms of event-loop CPU per HTTP request, 63% of server CPU under load.
get_uvicorn_log_confignow sendsuvicorn.accessto a plain handler. Server throughput went 337 -> 568 samples/s.Undelivered results were evicted, so SDK retries got
404 Future not found(the 128x128 failure Chuck hit).retrieve_futurestarted the short retrieved-TTL clock when the response object was built, even for a poll the SDK had already abandoned after its 45s client timeout; uvicorn drops the send silently, the sweeper evicted the result, and the SDK's retry of the same request_id 404'd (fatal in the SDK). Now the clock starts only ifrequest.is_disconnected()is false, and the retrieved TTL is 300s to outlast the SDK's worst-case re-poll gap (45s timeout + 30s backoff, twice).tests/tinker/test_retrieve_future_lost_response.pyreproduces the chain under a real uvicorn socket; it fails onmainand passes here.Config.
forwarding_inference_timeout_secdefault 300s -> 2048s (with unlimited connections a large burst waits inside vLLM's queue and 128x128 bursts exceed 300s there), and a newsample_max_concurrent_requests(default 2000, the SDK's default) served by/api/v1/client/configso operators can raise the SDK's in-flight cap.Retry semantics. Connect-phase errors and 5xx rejections (
TransientInferenceError) are retried once after refreshing the proxy URL. Read/write failures stay final (NovaSky-AI#2118 stance): vLLM may still be executing the request.Results (2048 outbound connection cap, SDK-style client, 16 vCPU, no GPU)
main, 2048 requestsLong-output rollouts (second commit)
With long outputs the per-result payload work becomes the server's main cost. For a 32k-token result (356KB JSON) the forwarding path spent 4ms (orjson decode, pydantic validate, pydantic JSON dump) and the proto path the SDK >= 0.25 uses on
retrieve_futurespent another 7ms (stdlibjson.loads+ proto build) inside a single global lock, capping proto delivery near 150 results/s regardless of concurrency. The proto build holds the GIL, so the thread hop bought nothing.Now the forwarding client decodes the vLLM body once and encodes straight to
SampleResponsewire bytes (serialize_sample_output, pinned byte-for-byte to the validated path by tests). The store keeps the proto bytes (8 bytes/token, 26% smaller than the JSON text);retrieve_futurepasses them through for proto clients and derives JSON lazily and cached for pre-0.25 clients. Pending entries no longer retain the request body. Store TTLs areEngineConfigfields (external_future_retrieved_ttl_sec,external_future_completed_ttl_sec) because retention after delivery is the dominant memory term: roughly completion rate x result size x window.Remaining per-result cost is
orjson.loads(0.8ms) plus list-to-numpy conversion (1.9ms) for 32k tokens, so one API process tops out around 165 results/s at 32k tokens and ~270/s at 8k. The 5s stalls at 100% CPU cause some keep-alive resets that the SDK retries; on this branch a re-sentasampleis a duplicate generation (Chuck's branch dedups by(model_id, sampling_session_id, seq_id)), which is the next thing worth porting.Notes
retrieve_futurerequests/s, above what one uvicorn process handles (~1200-2000 HTTP req/s; the in-memory store is single-process). Advertising an 8k-16ksample_max_concurrent_requestsand letting the SDK queue the rest is the pragmatic setting; the inference engines bound throughput anyway.--forwarding-inference-max-connectionsnear engine capacity.tests/tinker/+tests/utils/CPU suite, 107 passed.🤖 Generated with Claude Code