Repository navigation
Conversation
🏷️ CI GuideRuns automatically on every eligible PR before approval:
Heavy model tests:
|
Jasen2201
left a comment
There was a problem hiding this comment.
Thanks for the refactor. Sharing EndpointSpec / IngressRouting between HTTP and ext-proc is a clear improvement, and several fixes here are valuable: exactly-once outcome accounting in ProxyBody, no longer stripping content-encoding (reqwest is built without the gzip feature, so compressed bodies were previously forwarded without the header), preserving repeated response headers, and HTTP-mode prefix_hash now actually receiving tokens (previously always tokens: None -> PolicyReturnedNone). The new HTTP/ext-proc contract tests are good too.
Main items (details inline):
- The PD path was not updated for the new
x-api-keyforwarding, so the client key now reaches P/D alongside the worker Bearer. - Messages/Responses routing text is serialized in alphabetical key order (
BTreeMap), so cache_aware prefix matching seesmessages/inputbeforesystem/tools/instructions, the opposite of prompt order. - HTTP ingress now tokenizes synchronously on a tokio worker thread (ext-proc uses
spawn_blockingwith bounded slots). - Both READMEs link
docs/mesh_multi_api_guide.md, which is not in this PR or on main; the ext-proc README also overstates Responses support (resource operations are rejected in ext-proc mode).
Questions / notes inline: the typed route_* path that is no longer reachable from the server, PD now forwarding the raw body, item_reference state detection, resource fan-out filtering, DP data_parallel_rank injection into Messages/Responses, and ingress-side schema strictness.
Note: reviewed by reading the diff and cross-checking against the PR head (9a7d7f8); I did not build or run the tests locally.
1. Support Messages and Responses JSON/SSE through shared endpoint routing 2. Forward Responses resource operations over HTTP, preserving queries and worker credentials while deduplicating DP backend origins 3. Fix JSON completion accounting and stream resource responses directly 4. Consolidate handlers, organize tests, and simplify comments
- Treat Responses input item_reference entries as state references - Return 500 worker_request_preparation_failed in HTTP and ext-proc - Document backend-specific DP rank compatibility requirements - Remove stale resource fan-out test references
9a7d7f8 to
2526ca5
Compare
Jasen2201
left a comment
There was a problem hiding this comment.
Thanks for the quick turnaround. I checked the replies against the five new commits (78aa57d..a5c01ac; the first commit is unchanged from 9a7d7f8). These look good: the PD x-api-key fix plus its tests, the prompt-order routing view, item_reference detection, the 500 classification for prepare_request failures, and --prepare-max-tokenize-bytes.
Blocking
- Prepare pool admission (inline on
ingress.rs). Every inference request now goes through a fail-fast pool. With the defaults, more than 14 overlapping parse/prepare jobs get a 503. The total size of in-flight request bodies is also capped at 32 MiB, and that budget is held through the upstream call (in PD, through the whole prefill). Both are new failure modes on the HTTP path for long-context, high-concurrency traffic. Could the pool move to a separate PR with load data? For this PR, offloading only whenneeds_tokens()is true would address the original comment. 503 ambiguous_response_credentials(inline onhttp_router.rs). This blocks Responses resource operations for the standard OpenAI and Anthropic SDKs whenever there are multiple backends without worker keys.- Ingress schema narrowing (reply: "Agreed, I'll narrow"). The code at a5c01ac is unchanged:
max_tokensis still required (inference.rs:212), Messagestemperatureis still capped at 1,top_k: -1is still rejected, and roles are still limited to user/assistant. You already confirmed these don't match ATOM, so please include the change in this PR. - Resource fan-out has no tests. 17f0ea7 adds
mod resource_fanout_test;and a README row for it, but the file was never committed, and 2526ca5 removes both as "stale". Please add tests for the filtering, credential handling, origin dedup, and error selection.
Still open from the first round
- The ext-proc README (
scripts/ext-proc/README.md:64) still says Responses works through "both ordinary HTTP and ext-proc". ext-proc still returns 405 for non-POST requests (ext_proc/request.rs:106), so GET/DELETE/cancel only work over HTTP. One sentence would fix this. - The PR description doesn't yet have the notes promised in the replies: that the legacy typed path is removed in a follow-up, and that PD also forwards the raw body. For PD raw-body forwarding, is there a PD chat smoke or GSM8K run? The GPU ATOMesh jobs were skipped in CI.
ENDPOINT_MESSAGEShasn't been added (metrics_utils.rs:13still uses"messages").- The placement-failure retry for Responses isn't implemented yet. That's fine as a follow-up if the description says so.
Nits
- There's no test that locks in the shared-prefix property of the new routing view order (same system+tools, different user turn).
item_reference: in the OpenAI schematypeis optional on item references, so an item that is only{"id": "..."}isn't detected.
Note: reviewed by reading the PR head (a5c01ac); I did not build or run the tests.
- Default preparation workers to half of available_parallelism (min 1); build the Tokio runtime with the remaining half unless TOKIO_WORKER_THREADS is set - Replace DEFAULT_PREPARE_WORKERS with core::runtime::default_prepare_workers for the prepare pool and ext-proc parser concurrency defaults - Log the Tokio worker count at startup - Build the tokenizers crate with opt-level 3 in release (CI keeps 2)
- Reject Responses resource IDs outside [A-Za-z0-9_-] (max 256 bytes) so decoded IDs cannot change the forwarded backend path or query - Return 413 prepare_input_too_large when one body exceeds the whole preparation input budget, and warn at startup when the budget is below --max-payload-size - Run /v1/tokenize and /v1/detokenize encoding on the preparation pool - Prepare routing text and tokens in the HTTP/PD parse job so each request queues for the preparation pool once - Forward data_parallel_rank to Messages and Responses only when the DP worker's mesh.apis label lists them - Send worker keys for /v1/messages as both Authorization and x-api-key - Record Mesh-local errors with their real status instead of "canceled" - Return 501 unsupported_in_ext_proc for Responses resources via ext-proc - Fail /liveness and /health when the preparation pool has failed - Remove the stale resource_fanout_test module reference
- Count each preparation job outcome once; the ticket that times out or abandons a job owns its count, and forced rejections after a pool failure report WorkerFailed - Bound only preparation work by the deadline; requests whose preparation finished are forwarded instead of failing with 504 - Cap --prepare-parse-inline-max-bytes at 64 KiB - Rank 405/501 from backends without Responses resource routes below 404, and report unreachable backends as 502 backend_unreachable without internal URLs - Accept untyped Responses item references as state references - Treat Responses error events as terminal, count refusal deltas as first output, and include Anthropic cache tokens in input usage - Do not retry configuration errors; fail Responses over only when the connection was never established; classify connect failures first - Resolve ext-proc streaming like HTTP when the response has no content-type
Routing
- Prepare routing in the parse job for every HTTP/PD deployment via
IngressRouting::for_app; the RouterManager used in production previously
bypassed it and queued each request twice
- Remove the unreachable typed inference path from the HTTP, PD and manager
routers; route_* trait methods default to 501
- Share placement preparation, DP-rank body injection and backend
credential errors between HTTP, PD and ext-proc
- Pass the PD body by value instead of re-serializing it
- Validate API, topology and model once; add a ResponseId extractor and
shared body_too_large/tokenizer_input_too_large errors
Preparation pool
- Split into prepare_pool/{mod,job,control,tests}.rs
- Reduce job states to QUEUED->RUNNING and QUEUED->CANCELLED, count each
job outcome once on the ticket side, and derive stats and gauges from one
Level per quantity
- Share shutdown/drop teardown and constructors; validate PoolConfig in one
place with defaults defined once
Configuration
- Remove --prepare-parse-inline-max-bytes and --prepare-shutdown-grace-ms
(saved configs containing them still load)
- Make legacy ext-proc parser_concurrency/max_tokenize_bytes optional and
fold them into the preparation settings in one place; drop
prepare_workers_source and the max_streams clamp
- Stop exporting unused prepare_* keys to the Python bridge
Tests
- Remove redundant or timing-sensitive tests and merge overlapping ones;
make the unreachable-backend test deterministic
ProxyBody did not report is_end_stream or an exact size_hint, so the request observation wrapping it never saw completion when hyper stopped after Content-Length, and every successful non-streaming response was recorded as a "canceled" router error.
JiaoliangYu
left a comment
There was a problem hiding this comment.
Manual re-review of the latest head (c73b493f). The previous missing-test-module and Responses ID path-validation issues are fixed. I found one remaining blocking runtime issue and one documentation cleanup item. I reviewed the code manually; I could not run Rust tests locally because this machine does not have the Rust toolchain. git diff --check passes, and the GitHub non-GPU unit tests are currently green.
… budget - Remove the preparation input byte budget (--prepare-max-retained-input-bytes, 503 prepare_input_budget, 413 prepare_input_too_large); preparation deadlines already bound waiting. Saved configs containing the field still load. - Cap `log` records at the configured level in LogTracer. LogTracer::init() raised the cap to TRACE, so tokenizers' per-character trace! calls paid for a tracing dispatch; 16 KiB encodes drop from ~4.7 ms to ~3.3 ms. - Encode with tokenizer copies shared by groups of four preparation workers. One HuggingFace BPE instance contends on its word cache across threads (about a third of throughput lost at 16 threads); copies are loaded once per group with the registration factory and replaced on re-registration.
The suite was folded into proxy_contract_test.rs; describe the Responses resource forwarding coverage there and correct the test counts of the ingress, PD load and proxy body modules.
DP-aware forwarding reparsed the full request body on Tokio after preparation finished and serialized it again for every HTTP attempt and in ext-proc, outside the pool's worker limit and deadline. The pool job now builds a rank-agnostic body once (client data_parallel_rank removed, key order kept), and forwarding only appends "data_parallel_rank":N to those bytes. Non-DP workers keep the original bytes; a DP worker registered after preparation falls back to building the body once at forwarding.
The release profile used opt-level "z", trading request throughput for a smaller binary, with only the tokenizers crate raised to opt-level 3. Request parsing, routing and tokenization are CPU-bound, so build the whole binary at opt-level 3 and drop the per-package overrides for release and CI. Fat LTO, a single codegen unit and stripping are unchanged.
Motivation
Mesh is the request frontend in front of ATOM workers, but before this PR it only served OpenAI Chat/Completions well:
GET/DELETE /v1/responses/{id}, input items, cancel) were not forwarded, and Messages/Responses SSE streams were not accounted correctly.This PR unifies the Chat, Completions, Messages and Responses APIs behind one ingress shared by HTTP, PD and ext-proc, and moves preparation onto a bounded, observable thread pool. The goal is a frontend that costs less CPU and adds less tail latency per request than a Python (FastAPI) frontend, in either deployment mode.
Technical Details
1. One ingress for every API and deployment mode
Single endpoint registry. Each inference API is declared once, with its path and sub-resources:
/v1/chat/completions/v1/messages/v1/responses, plus/{id},/{id}/canceland/{id}/input_items/v1/completions/generateThe standalone HTTP router, the PD router and the Envoy ext-proc path all resolve requests through this registry, so they parse, validate and reject requests identically.
Capability-aware placement. Each request is checked against the deployment once (API, topology, model).
mesh.apislabel) and match the topology are offered to the load-balancing policy.unsupported_apiinstead of forwarding to a backend that would fail.Responses state safety. Some requests reference state stored on a backend:
previous_response_id,conversation, oritem_referenceinput items (including items with notype). They are routed only when every candidate shares one backend, or all candidates are labeledmesh.responses_state=shared. Otherwise Mesh returns 501stateful_routing_unsupported. Unhealthy workers count in this check, so failover cannot silently move a conversation to a backend that does not hold its state.Responses resource operations. Retrieve, delete, cancel and input-item listing are forwarded over HTTP with the original query string and the worker's credentials. DP ranks that share one backend address are queried once. Resource IDs are limited to
[A-Za-z0-9_-], at most 256 bytes, so a decoded ID cannot change the forwarded path or query.Payload pass-through. Mesh parses a request only to build a routing view, and forwards the client's original bytes.
data_parallel_rankis built once during preparation, preserving key order. Each attempt only appends the selected worker's rank, so retries never reparse or reserialize the body.mesh.apis.Credential isolation. When the selected worker has an API key, the client's
Authorizationandx-api-keyheaders are dropped and the worker key is sent instead, in regular and PD dispatch alike. Messages requests carry the worker key in both headers, because Anthropic clients authenticate withx-api-key.Response accounting.
errorevents end the stream;ext-proc parity. ext-proc shares the registry, placement, DP rank handling and error mapping with HTTP. Responses resource operations, which need a direct backend call, return 501
unsupported_in_ext_proc.2. Bounded preparation pool (
core/prepare_pool/)Off the I/O threads. These steps run on a dedicated pool of OS threads, so a long prompt no longer stalls I/O for other connections:
/v1/tokenizeand/v1/detokenizeWhen the routing policy needs text or tokens (for example cache-aware or prefix-hash routing), they are produced in the same job as the parse, so each request enters the pool once.
Admission control.
--prepare-queue-capacitymay wait. Beyond that, Mesh rejects immediately with 503prepare_queue_full.--prepare-queue-timeout-ms,--prepare-timeout-ms), and exceeding it returns 504. The deadline applies only to preparation: a request whose preparation finished is always forwarded.--prepare-max-tokenize-bytesare rejected with 413.Cancellation. If the client disconnects or the deadline passes before a worker picks up a job, the job is cancelled and never runs. Every job outcome (success, timeout, cancellation, failure) is counted exactly once.
Shutdown and health.
/livenessand/healthreport unhealthy.Observability. New Prometheus metrics:
mesh_prepare_queued_jobs,mesh_prepare_dispatched_jobs,mesh_prepare_execution_reserved,mesh_prepare_busy_workers,mesh_prepare_live_workersmesh_prepare_queue_seconds,mesh_prepare_run_seconds,mesh_prepare_stage_secondsmesh_prepare_errors_totalCPU budget. By default, half of the available CPUs run preparation (
--prepare-workers, minimum 1) and the other half serve I/O, unlessTOKIO_WORKER_THREADSis set. The older ext-proc parser settings map onto the same pool.3. Faster tokenization
opt-level = 3); the rest of the binary keeps the size-optimized release profile.Benchmark: Mesh vs. a Python frontend
Question. How much CPU and latency does Mesh spend per request compared with a typical Python serving frontend? Both are measured on the same cores with the same requests and the same backends.
Setup
atomeshat237714bd2run_in_executor; aiohttp forwards the original bytestokenizers0.22.2, one copy per 4 prepare workerstokenizers0.21.0 (Python binding), one per processopt-level=3, fat LTO,target-cpu=nativevia env overrides; repo profile unchangedMethod
tasksetto the same 8 physical cores (SMT siblings excluded).atomesh-mockervirtual workers returning a fixed JSON body) use 18 other logical CPUs./v1/completionsprompts of 1 KiB, 16 KiB, 64 KiB, 128 KiB, 512 KiB and 1 MiB.--queue-size 100000,--prepare-queue-capacity 100000, 120 s preparation deadlines) to match uvicorn/asyncio.--prepare-max-tokenize-bytesis raised to 8 MiB so 1 MiB prompts are accepted.Summary. Mesh is faster at all 60 measured points.
Ranges exclude points marked † or ‡ in the detailed tables.
Throughput ratio
Each cell is Mesh throughput divided by FastAPI throughput; blue means Mesh is faster.
Latency vs. throughput (Pareto)
Both axes are logarithmic and lower-right is better. Each line runs from 1 to 256 concurrent clients.
Parse + proxy, p99 latency
Parse + tokenize + proxy, p99 latency
Mean latency vs. throughput
Detailed results
All 60 points (median of 2 rounds)
Parse + proxy (Mesh: 8 Tokio + 4 prepare threads)
Parse + tokenize + proxy (Mesh: 4 Tokio + 8 prepare threads)
† Load-generator bound for Mesh: the ratio is a lower bound and latencies are not comparable.
‡ Finite 8 s window: mean latency undercounts in-flight multi-second requests for one or both frontends.