Skip to content

feat(vllm): add aggregated text-embedding worker (DIS-2092) - #9713

Merged
tzulingk merged 2 commits into
mainfrom
feat/vllm-embedding-worker-mvp
May 21, 2026
Merged

feat(vllm): add aggregated text-embedding worker (DIS-2092)#9713
tzulingk merged 2 commits into
mainfrom
feat/vllm-embedding-worker-mvp

Conversation

@tzulingk

@tzulingk tzulingk commented May 18, 2026

Copy link
Copy Markdown
Contributor

Overview:

MVP for DIS-2092 — adds an aggregated text-embedding worker shape to Dynamo's vLLM backend. Users can move an existing Qwen3-Embedding-0.6B vLLM deployment onto Dynamo+vLLM without changing engine args. (Builds on the --runner pooling flag preservation fix in #9710, now merged.)

The Rust frontend's /v1/embeddings route and ModelType::Embedding plumbing have existed since the SGLang embedding backend shipped. This PR registers a vLLM worker on the same endpoint.

Details:

Architecture. A new --embedding-worker flag in backend_args.py selects a different worker shape: pooling-mode AsyncLLM fronted by a new EmbeddingWorkerHandler that calls AsyncLLM.encode(prompt, pooling_params, request_id) and converts the PoolingRequestOutput to an OpenAI-shaped response. Dispatch happens in WorkerFactory.create() — the embedding branch is checked first, before the existing disaggregation_mode switch, because it's a different worker shape rather than a variant of decode.

EmbeddingWorkerHandler is a standalone class — it does NOT inherit BaseWorkerHandler. That base does generation-only init (media loaders, KV-block lookup via get_dp_range_for_worker, embedding cache manager) that would either fail or be meaningless on a pooling engine. Pooling inference is a single forward pass with no KV cache, no multimodal, no streamed decode — a separate class is clearer than overriding most of the base's behavior.

What's intentionally skipped on the embedding path:

  • KV-events publisher — no KV cache, nothing to publish.
  • Forward-pass-metrics relay — relays decode-phase ZMQ metrics; no decode here.
  • InstrumentedScheduler — hard-codes pooling_params=None (would silently disable the pooling pass) and emits decode-shaped metrics that don't apply. It's only installed when --benchmark-mode is set, which is rejected for embedding workers via _validate_embedding_worker_exclusivity(). See DIS-2092 appendix for the full discussion.
  • P/D disaggregation — rejected at parse time. --embedding-worker combined with any non-agg --disaggregation-mode raises ValueError.

Validation. DynamoVllmConfig._validate_embedding_worker_exclusivity() rejects --embedding-worker combined with prefill/decode/encode disaggregation, with any multimodal flag, or with --enable-multimodal.

Launch script. examples/backends/vllm/launch/agg_embed.sh defaults to Qwen/Qwen3-Embedding-0.6B with the standard pooling args: --runner pooling --dtype float32 --pooler-config '{"pooling_type":"MEAN","use_activation":false}'.

Unit tests (test_vllm_unit.pyTestEmbeddingWorkerFlag):

  • test_default_false
  • test_flag_sets_true
  • test_rejects_prefill_disagg
  • test_rejects_decode_disagg
  • test_rejects_multimodal_combo

A dispatch test (test_embedding_worker_takes_priority) was added to test_vllm_worker_factory.py to verify --embedding-worker is checked first and the other _create_*_worker methods are not invoked.

Note on benchmarking

This PR rejects --benchmark-mode for embedding workers, but that flag is Dynamo's in-process self-profiling — synthetic-request sweeps run at startup before serving traffic, used for capacity planning, engine-arg tuning, and planner input. It is separate from external HTTP load benchmarking via vllm bench serve (DIS-2095's methodology), which works against /v1/embeddings today and continues to work with this PR.

Decision: not adding --benchmark-mode embed now. Embedding workloads are roughly (batch_size × ISL → latency) — far simpler than decode workloads, with fewer interacting knobs. External HTTP load benchmarking (DIS-2095) covers the capacity-planning and engine-tuning needs. The one remaining wedge is Dynamo planner integration for auto-scaling; revisit when the planner needs embedding-aware metrics. See DIS-2092's appendix for the full reasoning.

Where should the reviewer start?

  1. components/src/dynamo/vllm/worker_factory.py_create_embedding_worker() (new) and the dispatch branch in create(). That's the structural change.
  2. components/src/dynamo/vllm/handlers.pyEmbeddingWorkerHandler (new class at the end of the file). Note the standalone (non-BaseWorkerHandler) design — see the docstring for the reasoning.
  3. components/src/dynamo/vllm/backend_args.py--embedding-worker flag, embedding_worker config field, and _validate_embedding_worker_exclusivity().
  4. examples/backends/vllm/launch/agg_embed.sh → reference launch config for the new worker shape.

Closes DIS-2092 (MVP scope).

@github-actions github-actions Bot added feat backend::vllm Relates to the vllm backend labels May 18, 2026
@tzulingk
tzulingk force-pushed the feat/vllm-embedding-worker-mvp branch from 6973e6d to 8a977a1 Compare May 19, 2026 00:10
@tzulingk
tzulingk force-pushed the feat/vllm-embedding-worker-mvp branch from 289f0e9 to 0a3c73e Compare May 19, 2026 00:35
@tzulingk
tzulingk force-pushed the feat/vllm-embedding-worker-mvp branch from 3d98cb1 to dc25694 Compare May 19, 2026 14:34
@tzulingk
tzulingk marked this pull request as ready for review May 19, 2026 16:30
@tzulingk
tzulingk requested a review from a team as a code owner May 19, 2026 16:30
@tzulingk
tzulingk requested a review from a team May 19, 2026 16:30
@tzulingk
tzulingk requested review from a team as code owners May 19, 2026 16:30
@tzulingk
tzulingk requested review from biswapanda and nnshah1 May 19, 2026 16:31
Comment thread components/src/dynamo/vllm/backend_args.py
Comment thread components/src/dynamo/vllm/handlers.py Outdated
Comment thread components/src/dynamo/vllm/handlers.py Outdated
Comment thread components/src/dynamo/vllm/handlers.py
tzulingk added a commit that referenced this pull request May 19, 2026
Four fixes flagged by dynamo-ops review on the standalone embedding worker:

1. Reject --benchmark-mode when --embedding-worker is set
   (components/src/dynamo/vllm/backend_args.py): --benchmark-mode injects
   InstrumentedScheduler, which hard-codes pooling_params=None and would
   silently disable pooling. Surface the conflict at validation time
   instead of producing a broken engine.

2. Handle pre-tokenized inputs correctly
   (components/src/dynamo/vllm/handlers.py): the OpenAI /v1/embeddings
   spec accepts input as str, list[str], list[int], or list[list[int]].
   The previous str(item) coercion turned [1, 2, 3] into three text
   prompts ("1", "2", "3"). Add a _classify_embedding_input helper that
   distinguishes the four shapes, wraps token-id arrays in
   vllm.inputs.TokensPrompt, and rejects mixed lists with a clear error.

3. Add an abort monitor around each encode call
   (components/src/dynamo/vllm/handlers.py): client cancellation or
   shutdown_event were leaving GPU work running until encode completed.
   Inline a trimmed version of BaseWorkerHandler._abort_monitor (no
   is_prefill, no abort_guard) that calls engine_client.abort(request_id)
   on cancellation and raises EngineShutdown on shutdown_event.

4. Wire VllmEngineMonitor so dead pooling engines are detected
   (components/src/dynamo/vllm/handlers.py,
   components/src/dynamo/vllm/worker_factory.py): a crashed AsyncLLM was
   leaving the endpoint registered serving failures. Construct
   VllmEngineMonitor in EmbeddingWorkerHandler.__init__, plumbing the
   runtime handle through worker_factory.

Tests:
- components/src/dynamo/vllm/tests/test_backend_args.py:
  TestEmbeddingWorkerExclusivity covers the new benchmark-mode rejection
  alongside the existing disagg/multimodal exclusions.
- components/src/dynamo/vllm/tests/test_vllm_worker_handler.py:
  TestClassifyEmbeddingInput covers all four input shapes plus the
  rejection paths (mixed lists, bools, empty lists, unsupported types).

DIS-2092

Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>
tzulingk added a commit that referenced this pull request May 20, 2026
Mirrors what PR #9722 did for SGLang's embedding handler: read the
optional `dimensions` field from the request and slice each embedding
to the first N floats before responding.

Validates `1 <= dimensions <= model_hidden_dim`; otherwise raises
`ValueError` which the Rust frontend surfaces as HTTP 400. Matches the
OpenAI hosted API's behavior of rejecting out-of-range `dimensions`
values rather than silently zero-padding or clamping.

Test plan additions in `tests/serve/test_vllm.py::embedding_agg`:

- New payload with `dimensions=128` against Qwen3-Embedding-0.6B
  (hidden dim 1024) → response must report dimension exactly 128.

`EmbeddingPayload` is constructed inline here because the
`extra_body` kwarg on `embedding_payload()` lives in PR #9722 and
isn't in this branch's base. Once both PRs land we can simplify this
test to use `embedding_payload(..., extra_body={"dimensions": 128})`.

Stacks on: #9713 (vLLM embedding worker MVP — adds the handler being
modified here). Refs DIS-2093, parallel to #9722 for SGLang.

Note: `encoding_format=base64` is NOT included. Same reason as the
SGLang side: the Rust frontend's response type is `Vec<f32>` via
the upstream `async_openai::types::embeddings` re-export and rejects
base64 strings. Tracked in DIS-2099 (requires owning the embedding
response type in lib/protocols/).

Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>
tzulingk added a commit that referenced this pull request May 20, 2026
Adds end-to-end + unit coverage for the two routing properties promised
by the embedding worker pool:

1. Same-model load balancing — two workers serving Qwen3-Embedding-0.6B
   weighted-randomly absorb a burst of /v1/embeddings traffic. Asserts
   both workers' dynamo_component_requests_total counters move > 0 over
   the burst window.

2. Multi-model dispatch — two workers, two different models
   (Qwen3-Embedding-0.6B + BAAI/bge-small-en-v1.5). Each model's burst
   must only land on its registered worker; the wrong-model worker
   observes zero delta. Verifies the name-keyed
   get_embeddings_engine(model) → select_worker_set_with() path
   specifically for embedding traffic (chat-completions exercises the
   same code, but DIS-2098 asks for explicit embedding-side coverage).

Pieces:
- examples/backends/vllm/launch/agg_embed_multiworker.sh — frontend +
  N embedding workers (one per GPU, distinct DYN_SYSTEM_PORT each).
  Takes two model args; mirrors agg_embed.sh's pooler config.
- tests/utils/payloads.py — new EmbeddingMultiWorkerDispatchPayload that
  snapshots /metrics before and after a burst and asserts the delta-vs-
  expected-indices pattern. Diff semantics matter because back-to-back
  bursts in the multi-model test share absolute counters; only deltas
  prove "wrong-model traffic stayed out".
- tests/serve/test_vllm.py — two gpu_2 / pre_merge test functions
  driving the script + payload.
- tests/utils/test_embedding_dispatch_payload.py — unit tests for the
  payload's baseline+delta logic. Monkeypatches requests.get with a
  canned per-port /metrics sequence; no GPU needed.

Stacked on PR #9713 (DIS-2092 vLLM embedding worker MVP). The launch
script and tests can't run until the embedding worker handler in #9713
lands; this PR will rebase to main once #9713 merges.

Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>
tzulingk added a commit that referenced this pull request May 20, 2026
Five issues raised on PR #9713 by CodeRabbit and dynamo-ops:

1. handlers.py: vLLM request_id was derived from context.trace_id or
   id(context), both of which can collide across concurrent embedding
   requests in the same distributed trace. Switch to context.id() (the
   per-request id already used elsewhere in this file) so AsyncLLM never
   sees colliding request ids.

2. handlers.py: the embedding _monitor_abort caught Exception and only
   logged, masking cancellation/shutdown failures and leaving encode
   work unmanaged. Re-raise after logging so _abort_monitor.__aexit__
   surfaces the failure via task.result(). The fast-path
   asyncio.CancelledError + EngineShutdown branches are unchanged.

3. worker_factory.py: rewrote the benchmark-mode rationale comment so
   it no longer references internal ticket ids.

4. agg_embed.sh: removed internal ticket id references from the
   default-model / max-model-len / pooler-config comments.

5. test_vllm.py: added pytest.mark.core to the embedding_agg config so
   it satisfies the "exactly one component marker per
   framework-marked test" repository policy.

Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>
tzulingk added a commit that referenced this pull request May 20, 2026
Mirrors what PR #9722 did for SGLang's embedding handler: read the
optional `dimensions` field from the request and slice each embedding
to the first N floats before responding.

Validates `1 <= dimensions <= model_hidden_dim`; otherwise raises
`ValueError` which the Rust frontend surfaces as HTTP 400. Matches the
OpenAI hosted API's behavior of rejecting out-of-range `dimensions`
values rather than silently zero-padding or clamping.

Test plan additions in `tests/serve/test_vllm.py::embedding_agg`:

- New payload with `dimensions=128` against Qwen3-Embedding-0.6B
  (hidden dim 1024) → response must report dimension exactly 128.

`EmbeddingPayload` is constructed inline here because the
`extra_body` kwarg on `embedding_payload()` lives in PR #9722 and
isn't in this branch's base. Once both PRs land we can simplify this
test to use `embedding_payload(..., extra_body={"dimensions": 128})`.

Stacks on: #9713 (vLLM embedding worker MVP — adds the handler being
modified here). Refs DIS-2093, parallel to #9722 for SGLang.

Note: `encoding_format=base64` is NOT included. Same reason as the
SGLang side: the Rust frontend's response type is `Vec<f32>` via
the upstream `async_openai::types::embeddings` re-export and rejects
base64 strings. Tracked in DIS-2099 (requires owning the embedding
response type in lib/protocols/).

Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>
tzulingk added a commit that referenced this pull request May 20, 2026
Adds end-to-end + unit coverage for the two routing properties promised
by the embedding worker pool:

1. Same-model load balancing — two workers serving Qwen3-Embedding-0.6B
   weighted-randomly absorb a burst of /v1/embeddings traffic. Asserts
   both workers' dynamo_component_requests_total counters move > 0 over
   the burst window.

2. Multi-model dispatch — two workers, two different models
   (Qwen3-Embedding-0.6B + BAAI/bge-small-en-v1.5). Each model's burst
   must only land on its registered worker; the wrong-model worker
   observes zero delta. Verifies the name-keyed
   get_embeddings_engine(model) → select_worker_set_with() path
   specifically for embedding traffic (chat-completions exercises the
   same code, but DIS-2098 asks for explicit embedding-side coverage).

Pieces:
- examples/backends/vllm/launch/agg_embed_multiworker.sh — frontend +
  N embedding workers (one per GPU, distinct DYN_SYSTEM_PORT each).
  Takes two model args; mirrors agg_embed.sh's pooler config.
- tests/utils/payloads.py — new EmbeddingMultiWorkerDispatchPayload that
  snapshots /metrics before and after a burst and asserts the delta-vs-
  expected-indices pattern. Diff semantics matter because back-to-back
  bursts in the multi-model test share absolute counters; only deltas
  prove "wrong-model traffic stayed out".
- tests/serve/test_vllm.py — two gpu_2 / pre_merge test functions
  driving the script + payload.
- tests/utils/test_embedding_dispatch_payload.py — unit tests for the
  payload's baseline+delta logic. Monkeypatches requests.get with a
  canned per-port /metrics sequence; no GPU needed.

Stacked on PR #9713 (DIS-2092 vLLM embedding worker MVP). The launch
script and tests can't run until the embedding worker handler in #9713
lands; this PR will rebase to main once #9713 merges.

Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>
tzulingk added a commit that referenced this pull request May 20, 2026
Apply the same review feedback from PR #9713 to this stacked PR:
- Strip ``DIS-`` ticket references from the multi-worker test docstring,
  launch script header, and payload helper docstring (the explanations
  are self-contained without them, per repository policy on internal
  ticket ids in source).
- Add ``pytest.mark.core`` to both multi-worker test functions so they
  satisfy the "exactly one component marker per framework-marked test"
  policy that CodeRabbit flagged on the related ``embedding_agg``
  config in #9713.

Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>
tzulingk added a commit that referenced this pull request May 20, 2026
Per the repository policy on internal Linear ticket ids in source code
(also flagged on PR #9713), drop the ``DIS-`` reference from the module
docstring. The cross-reference to PR #9713 already communicates the
"vLLM side will observe these same metric names" intent.

Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>
tzulingk added a commit that referenced this pull request May 20, 2026
Mirrors what PR #9722 did for SGLang's embedding handler: read the
optional `dimensions` field from the request and slice each embedding
to the first N floats before responding.

Validates `1 <= dimensions <= model_hidden_dim`; otherwise raises
`ValueError` which the Rust frontend surfaces as HTTP 400. Matches the
OpenAI hosted API's behavior of rejecting out-of-range `dimensions`
values rather than silently zero-padding or clamping.

Test plan additions in `tests/serve/test_vllm.py::embedding_agg`:

- New payload with `dimensions=128` against Qwen3-Embedding-0.6B
  (hidden dim 1024) → response must report dimension exactly 128.

`EmbeddingPayload` is constructed inline here because the
`extra_body` kwarg on `embedding_payload()` lives in PR #9722 and
isn't in this branch's base. Once both PRs land we can simplify this
test to use `embedding_payload(..., extra_body={"dimensions": 128})`.

Stacks on: #9713 (vLLM embedding worker MVP — adds the handler being
modified here). Refs DIS-2093, parallel to #9722 for SGLang.

Note: `encoding_format=base64` is NOT included. Same reason as the
SGLang side: the Rust frontend's response type is `Vec<f32>` via
the upstream `async_openai::types::embeddings` re-export and rejects
base64 strings. Tracked in DIS-2099 (requires owning the embedding
response type in lib/protocols/).

Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>
tzulingk added a commit that referenced this pull request May 20, 2026
Adds end-to-end + unit coverage for the two routing properties promised
by the embedding worker pool:

1. Same-model load balancing — two workers serving Qwen3-Embedding-0.6B
   weighted-randomly absorb a burst of /v1/embeddings traffic. Asserts
   both workers' dynamo_component_requests_total counters move > 0 over
   the burst window.

2. Multi-model dispatch — two workers, two different models
   (Qwen3-Embedding-0.6B + BAAI/bge-small-en-v1.5). Each model's burst
   must only land on its registered worker; the wrong-model worker
   observes zero delta. Verifies the name-keyed
   get_embeddings_engine(model) → select_worker_set_with() path
   specifically for embedding traffic (chat-completions exercises the
   same code, but DIS-2098 asks for explicit embedding-side coverage).

Pieces:
- examples/backends/vllm/launch/agg_embed_multiworker.sh — frontend +
  N embedding workers (one per GPU, distinct DYN_SYSTEM_PORT each).
  Takes two model args; mirrors agg_embed.sh's pooler config.
- tests/utils/payloads.py — new EmbeddingMultiWorkerDispatchPayload that
  snapshots /metrics before and after a burst and asserts the delta-vs-
  expected-indices pattern. Diff semantics matter because back-to-back
  bursts in the multi-model test share absolute counters; only deltas
  prove "wrong-model traffic stayed out".
- tests/serve/test_vllm.py — two gpu_2 / pre_merge test functions
  driving the script + payload.
- tests/utils/test_embedding_dispatch_payload.py — unit tests for the
  payload's baseline+delta logic. Monkeypatches requests.get with a
  canned per-port /metrics sequence; no GPU needed.

Stacked on PR #9713 (DIS-2092 vLLM embedding worker MVP). The launch
script and tests can't run until the embedding worker handler in #9713
lands; this PR will rebase to main once #9713 merges.

Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>
tzulingk added a commit that referenced this pull request May 20, 2026
Apply the same review feedback from PR #9713 to this stacked PR:
- Strip ``DIS-`` ticket references from the multi-worker test docstring,
  launch script header, and payload helper docstring (the explanations
  are self-contained without them, per repository policy on internal
  ticket ids in source).
- Add ``pytest.mark.core`` to both multi-worker test functions so they
  satisfy the "exactly one component marker per framework-marked test"
  policy that CodeRabbit flagged on the related ``embedding_agg``
  config in #9713.

Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>
@tzulingk
tzulingk force-pushed the feat/vllm-embedding-worker-mvp branch from 8ad9834 to 3d974aa Compare May 20, 2026 15:23
tzulingk added a commit that referenced this pull request May 20, 2026
Adds two Prometheus histograms that capture embedding-specific
request shape, observed from the SGLang embedding worker:

  dynamo_embedding_batch_size    (model)    histogram
  dynamo_embedding_input_tokens  (model)    histogram

Both are recorded once per request in `_transform_response`.

Why this exists
===============
The SGLang publisher today emits chat-shaped metrics (KV gauges,
prefill/decode counters) that are always zero on a pooling engine.
An operator monitoring an embedding fleet sees noise instead of
signal. These two histograms let you actually answer "how big are
batches?" and "how many tokens per request?" — the basis for capacity
planning and saturation detection.

Buckets are documented in metrics.py:
- batch size: powers of two up to 2048 (OpenAI hosted's per-request
  limit)
- input tokens: dense in 1..8192 since typical sentence-level
  embedding ISLs are 60-200

Metric names are intentionally NOT prefixed with `sglang:` — they
describe an OpenAI-spec workload shape, not an engine-internal signal.
Once the vLLM embedding worker (PR #9713) ships, it observes the same
names from its own handler, partitioned by `model`.

Robustness
==========
Observations are wrapped in try/except + log inside
`_transform_response`. A bad metric call must never break inference.

Tests
=====
components/src/dynamo/sglang/tests/test_sglang_embedding_metrics.py —
6 cases:
- Count + sum increment correctly across observations.
- `model` label partitions correctly for multi-model deployments.
- Buckets are monotonically non-decreasing (regression test for
  accidental bucket reorder).
- Negative values are silently dropped + logged (defensive — would
  corrupt percentile estimates).
- Handler integration: `_transform_response` actually calls the
  observe helpers with the right (model, batch_size, input_tokens)
  for the request.
- Handler robustness: a raising observe call does not break the
  OpenAI response shape.

Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>
Comment thread components/src/dynamo/vllm/backend_args.py
@tzulingk
tzulingk requested a review from dynamo-ops May 20, 2026 20:19
Comment thread components/src/dynamo/vllm/handlers.py

@biswapanda biswapanda left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall lgtm.
one comment on opportunity to improve perf

@tzulingk
tzulingk force-pushed the feat/vllm-embedding-worker-mvp branch from 3d974aa to 5be8483 Compare May 20, 2026 20:22
Comment thread components/src/dynamo/vllm/handlers.py
Adds an aggregated text-embedding worker shape to Dynamo's vLLM
backend. Users can move an existing pooling-model vLLM deployment
(e.g. Qwen3-Embedding-0.6B) onto Dynamo+vLLM without changing engine
args. The Rust frontend's `/v1/embeddings` route and
`ModelType::Embedding` plumbing have existed since the SGLang
embedding backend shipped; this commit registers a vLLM worker on the
same endpoint.

Architecture
============
A new `--embedding-worker` flag in `backend_args.py` selects a
different worker shape: pooling-mode `AsyncLLM` fronted by a new
`EmbeddingWorkerHandler` that calls
`AsyncLLM.encode(prompt, pooling_params, request_id)` and converts the
`PoolingRequestOutput` to an OpenAI-shaped response. Dispatch happens
in `WorkerFactory.create()` — the embedding branch is checked first,
before the existing `disaggregation_mode` switch, because it's a
different worker shape rather than a variant of decode.

`EmbeddingWorkerHandler` is a standalone class that does NOT inherit
`BaseWorkerHandler`. The base does generation-only init (media
loaders, KV-block lookup, embedding cache manager) that would either
fail or be meaningless on a pooling engine. Pooling inference is a
single forward pass with no KV cache, no multimodal, no streamed
decode — a separate class is clearer than overriding most of the
base's behavior.

Intentionally skipped on the embedding path
============================================
- KV-events publisher: no KV cache, nothing to publish.
- Forward-pass-metrics relay: relays decode-phase ZMQ metrics; no
  decode here.
- InstrumentedScheduler: hard-codes pooling_params=None (would
  silently disable the pooling pass) and emits decode-shaped metrics
  that don't apply. Only installed when --benchmark-mode is set,
  which is rejected for embedding workers via
  `_validate_embedding_worker_exclusivity()`.
- P/D disaggregation: rejected at parse time. `--embedding-worker`
  combined with any non-`agg` `--disaggregation-mode` raises
  `ValueError`.

Request handling
================
- Inputs accepted per the OpenAI /v1/embeddings spec: `str`,
  `list[str]`, `list[int]`, `list[list[int]]`. Mixed lists are
  rejected with a clear `TypeError`. Token-id forms are passed as
  `vllm.inputs.TokensPrompt` so the engine skips its own tokenizer.
- Per-request `request_id` is derived from `context.id()` (matches
  the chat/completion paths) so concurrent embeddings never collide
  inside `AsyncLLM`.
- Each encode call is wrapped in `_abort_monitor` so client
  cancellation or `shutdown_event` calls `engine_client.abort()` and
  propagates `EngineShutdown` rather than leaving GPU work running.
- `VllmEngineMonitor` is wired in `__init__` so a dead pooling engine
  triggers `shutdown_event` instead of leaving the endpoint registered
  serving failures.

Validation
==========
`DynamoVllmConfig._validate_embedding_worker_exclusivity()` rejects
`--embedding-worker` combined with: any non-`agg` disaggregation,
multimodal flags, `--enable-multimodal`, or `--benchmark-mode`.

Launch script
=============
`examples/backends/vllm/launch/agg_embed.sh` defaults to
`Qwen/Qwen3-Embedding-0.6B` with the standard pooling args:
`--runner pooling --dtype float32 --pooler-config
'{"pooling_type":"MEAN","use_activation":false}'`.

Tests
=====
- `test_vllm_unit.py::TestEmbeddingWorkerFlag`: parse-time flag
  acceptance + exclusion combinations.
- `test_vllm_worker_factory.py::test_embedding_worker_takes_priority`:
  dispatch precedence over decode/prefill/encode paths.
- `test_vllm_worker_handler.py::TestClassifyEmbeddingInput`: all four
  input shapes plus mixed-list / bool / empty rejection.
- `test_backend_args.py::TestEmbeddingWorkerExclusivity`: covers the
  benchmark-mode and multimodal rejections.
- `test_vllm.py` adds an `embedding_agg` config in the
  `pytest.mark.core` lane that exercises the launch script + a
  range of input shapes.

Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>
Comment thread components/src/dynamo/vllm/handlers.py

@nnshah1 nnshah1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

…ort exit

On the success path of an embedding request, ``_abort_monitor.__aexit__``
cancels the running ``_monitor_abort`` task while it's blocked in
``asyncio.wait``. The CancelledError propagates out of ``asyncio.wait``
and jumps directly to ``except asyncio.CancelledError: pass``,
short-circuiting past the ``for task in pending: task.cancel()`` cleanup
loop. The result: the ``shutdown_event.wait()`` task created inside
``_monitor_abort`` is never cancelled — one leaked task per embedding
request, accumulating for the lifetime of the worker.

Fix: add a ``finally`` block that cancels ``shutdown_task`` on every
exit path (normal completion, cancellation, EngineShutdown, unexpected
exception).

Addresses dynamo-ops review on PR #9713.

Signed-off-by: Tzu-Ling <tzulingk@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend::vllm Relates to the vllm backend feat size/XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants