Repository navigation
fix(frontend): mint internal request ids on the server for HTTP and gRPC - #17032
Conversation
Concurrent HTTP requests that shared a client-chosen id collided in the frontend's in-flight response map, so callers failed with DetachedStreamSender/DetachedStreamReceiver or received another caller's stream. - HTTP ingress spans always mint the internal request id; inbound `request-id` / `x-dynamo-request-id` are ignored there (with a deprecation warning). The worker-side transport read is unchanged. - `/inference/v1/generate` and SGLang `/generate` keep `x-request-id`, body `request_id` and `rid` as correlation values echoed to the client, while the request context always uses the server-minted id. - `Frontend::generate` refuses a duplicate in-flight id instead of replacing the existing sender, and drops its entry if dispatch fails. - Sidecar testkit binds plans to the next unregistered request so HTTP-driven tests no longer rely on choosing the worker request id. Closes ai-dynamo#17010 Signed-off-by: svc-glamr@nvidia.com <svc-glamr@nvidia.com>
Use a drop guard in Frontend::generate so a caller that is dropped before the request reaches the backend no longer leaves its id in the sink map, where it would refuse every later request with that id. Replace the distinct-id pipeline test with one that aborts a parked request and reuses its id. Record the ignored request id header as a structured log field. Refs ai-dynamo#17010 Signed-off-by: svc-glamr@nvidia.com <svc-glamr@nvidia.com>
Signed-off-by: svc-glamr@nvidia.com <svc-glamr@nvidia.com>
|
factory: Initial validation snapshot. Validation status: complete
The SGLang Detailed evidence — validation completeLeft out of the table because CI repeats them: building the changed source and importing its compiled extension (Check 1), lint and formatting with Evidence tally: [4/4 validated] Advisory AI assessment: sound. This is an AI judgment, not an approval. Validation result: pass — on Evidence audit: complete [4/4 validated] — the command report comes from recorded runs. The head Commands and results [4/4 validated]Generated from the commands recorded during this run. Check 1Builds the changed Dynamo source and confirms that Python can import its compiled extension. Result: Passed ( Command: Not shown because the exact command contained private run data. Check 2Checks the changed files with the repository's fast lint and formatting commands. Result: Passed ( Command: pre-commit run --files docs/fern/pages/use-cases/reinforcement-learning/operations-and-simulation.md lib/llm/src/http/service/generate.rs lib/llm/src/http/service/openai.rs lib/llm/src/http/service/sglang_generate.rs lib/llm/tests/http-service.rs lib/runtime/src/logging.rs lib/runtime/src/pipeline/error.rs lib/runtime/src/pipeline/nodes/sources/base.rs lib/runtime/tests/pipeline.rs lib/sidecar/testkit/src/control.rs lib/sidecar/testkit/tests/router_sidecar_mocker_integration.rs lib/sidecar/testkit/tests/support/process.rs tests/frontend/test_request_tracing_logs.pyCheck 3Checks the changed Rust crates with Result: Passed ( Command:
Check 4Starts Dynamo with vLLM on one GPU and sends a real request. Result: Passed ( Command:
|
|
No description provided. |
Signed-off-by: svc-glamr@nvidia.com <svc-glamr@nvidia.com>
|
factory: @dynamo-ops please run full CI for 8081718 |
|
/ok to test 8081718 |
…rkers Signed-off-by: svc-glamr@nvidia.com <svc-glamr@nvidia.com>
|
factory: @dynamo-ops please run full CI for bb563a5 |
|
/ok to test bb563a5 |
|
nursery: @dynamo-ops please run full CI for bd3bd12 |
|
/ok to test bd3bd12 |
Signed-off-by: GLAMR <svc-glamr@nvidia.com>
|
nursery: @dynamo-ops please run full CI for a2fb5bf |
|
/ok to test a2fb5bf |
Signed-off-by: GLAMR <svc-glamr@nvidia.com>
|
nursery: @dynamo-ops please run full CI for 971ce6b |
|
/ok to test 971ce6b |
Signed-off-by: GLAMR <svc-glamr@nvidia.com>
|
nursery: @dynamo-ops please run full CI for fe4eea1 |
|
/ok to test fe4eea1 |
Signed-off-by: GLAMR <svc-glamr@nvidia.com>
|
nursery: @dynamo-ops please run full CI for 060b6a7 |
|
/ok to test 060b6a7 |
nnshah1
left a comment
There was a problem hiding this comment.
Approving for frontend, runtime and observability. Verified on head 060b6a7: every route now mints its own id and a duplicate key is refused instead of replacing the in-flight entry; migration still re-dispatches through the router and a worker-side Conflict stays migratable; the dispatch spans keep x_request_id so client correlation is unchanged. All required checks green, no open threads. Still needs a docs codeowner for the RL page change. Follow-ups tracked separately: vLLM reusing the Dynamo id as its engine id, and gRPC ingress span/correlation header.
— Neelay + 🤖
Signed-off-by: GLAMR <svc-glamr@nvidia.com>
|
nursery: @dynamo-ops please run full CI for af4f31c |
|
/ok to test af4f31c |
|
Feedback for GLAMR |
Closes #17010
Overview
Concurrent requests with the same client ID can overwrite the frontend’s in-flight state and fail or receive another caller’s response. This fix gives each HTTP and gRPC request a server-generated internal ID while preserving client IDs for correlation and responses.
Failure behavior and recorded reproduction
Two concurrent requests that send the same client request id break each other. On
/inference/v1/generate, two requests withx-request-id: 1both fail with HTTP 500, and the frontend logsUnlinked response; response task was dropped or cancelledorUnlinked request; initiating request task was dropped or cancelled. A caller can also receive the other caller's stream.The frontend tracks each in-flight request by its id, so it can route the response back to the caller. On the OpenAI, Anthropic and Responses routes, that id came from the
x-dynamo-request-idorrequest-idheader. On/inference/v1/generateand SGLang/generate, it came fromx-request-idor the bodyrequest_id/rid. A second request with the same id silently replaced the first request's entry, so one response lost its caller. The defect is in the Dynamo frontend./inference/v1/generaterequests sharingx-request-id: 1succeeded, with HTTP 500 and the detached-stream errors. With the fix, 60 of 60 concurrent requests on the generate and chat routes returned 200 with their own output, in each of two runs.Key Changes
Core logic: approximately 90 added lines, excluding tests, documentation, comments, and whitespace.
base.rsrefuses duplicate in-flight IDs and releases entries when callers are dropped.logging.rsand HTTP/gRPC handlers generate internal IDs while native generate routes keep client IDs in responses andx_request_idtrace fields.push_handler.rspreserves duplicate refusal asConflict, allowing migration without marking a healthy worker down.Request ID behavior and migration scope
The frontend now generates its own internal request id for every HTTP and gRPC request and never takes it from the client. Client ids are correlation values only:
x-request-idis still logged and echoed, and the two native generate routes still return the client'srequest_idorridin the response. The frontend also refuses a second in-flight request with an id it already holds, instead of replacing the first one, and frees the id when a caller goes away.x-request-idx-dynamo-request-idx-dynamo-request-idorrequest-idx-request-idIf a migration retry reaches the same worker while the first attempt is still waiting for its engine to create a response stream, the worker refuses the retry's duplicate request id. The refusal is typed
Conflict, so the frontend retries the request on another worker and does not mark the refusing worker as down. The reinforcement learning operations page now describes client ids as correlation values, except theridof native SGLang session requests, which SGLang uses as the session key.The gRPC completion and tensor handlers also generate internal request ids. KServe still copies the client’s proto request id into unary and streaming responses independently of the internal context id.
Start reviewing at
lib/runtime/src/pipeline/nodes/sources/base.rs.Validation
This PR adds and updates tests in
lib/runtime/tests/pipeline.rs,lib/runtime/src/logging.rs,lib/runtime/src/pipeline/network/ingress/push_handler.rs,lib/runtime/src/pipeline/network/egress/push_router.rs,lib/llm/src/http/service/generate.rs,lib/llm/src/http/service/sglang_generate.rs,lib/llm/tests/http-service.rs,lib/llm/tests/kserve_service.rs,lib/sidecar/testkit/tests/router_sidecar_mocker_integration.rsandtests/frontend/test_request_tracing_logs.py. The Rust tests run without a GPU, against echo engines or the mocker behind a real frontend pipeline. The Python test is an end-to-end run of the frontend with a mocker worker, also without a GPU.base.rsrequest-idandx-dynamo-request-id/inference/v1/generaterequests sharingx-request-ideach get their own tokens, and the body echoes the client id/inference/v1/generatemints the internal id and keeps vLLM'sx-request-idbefore bodyrequest_idprecedence for the public id/generaterequests sharingrideach get their own output, and the worker envelope keeps the clientrid/generatekeeps client ids as correlation onlyx-dynamo-request-idandrequest-ideach get their own contentrequest_idannotation is a fresh UUID, not the client'sx-dynamo-request-idx-dynamo-request-idlogs one deprecation WARN with aheaderfield, and the logged request id is a fresh UUIDmust be a valid UUIDWARNThe existing KServe tests
test_infer_successandtest_tensor_infernow submit concurrent requests with the same valid client UUID. Their engines check that the internal context id is a UUID different from the client id, and the tests check that each response still echoes the client id.The tables were recorded on commit
453394da18. Later commits change how a worker's duplicate-id refusal is typed, add one unit test for it, and edit comments and docs. Locally, those changes were checked only bycargo test -p dynamo-runtime --libon the newduplicate_request_id_is_a_conflicttest and the changedpre_stream_invalid_request_does_not_quarantine_healthy_workertest (both pass) and bycargo clippy -p dynamo-runtime --all-targets -- -D warnings; everything else was checked only by CI.The base merge keeps main’s tracing interest guard in
generate_dispatch_span_uses_resolved_request_idand retains its separate internal and client ID assertions. The two runtime pipeline regression tests now bound their backend-arrival waits to five seconds so an early dispatch failure cannot hang them.Issues and Ongoing Work
Pull Request Checklist
/pr-checklistskill. The review used separate design/performance, validation/style, and adversarial reviews. This maintenance pass used static inspection and remote CI; it did not run local tests, builds, or benchmarks.Summary by CodeRabbit
New Features
Bug Fixes
Documentation