Skip to content

[Bugfix] Register statistics for dynamically added replicas - #7298

Merged
Gaohan123 merged 3 commits into
vllm-project:mainfrom
leegangtoe:fix/dynamic-replica-stats
Sep 19, 2026
Merged

Gaohan123 merged 3 commits into
vllm-project:mainfrom
leegangtoe:fix/dynamic-replica-stats

Conversation

@leegangtoe

Copy link
Copy Markdown
Contributor

Purpose

Fixes #7296.

Adding a headless replica to a running distributed stage can terminate the orchestrator
with KeyError: (0, 1) when log statistics are enabled. Both the orchestrator's engine-index
map and the Prometheus per-engine metric tables contain only the initial replicas.

Register a new replica before recording its first statistics. Bind new metric children to
the existing families, preserving the counters of replicas already serving requests.
Registration covers nested label tables and helper collectors, including speculative
position counters. Heads with no initial replica initialize collectors on their first join.
Repeated registration preserves the same index and counters; new replicas start awake.

This branch is based directly on main and is independent of the multimodal cache fix #7297.
Detached replicas retain their metric series; this change does not remove historical series.

Test Plan

vLLM Version: 0.28.0; PyTorch 2.13.0+cu130, CUDA runtime 13.0, Transformers 5.14.1.

vLLM-Omni Commit: efb39d72ff82dcd88ca31a02a1413e003e93878c; base main 5e4baea64d0f4cd1b6ce147e4f7a28f0f41d995f.

CPU regression tests use the real upstream logger and Prometheus registry. Model configuration
loading is mocked. Coverage includes zero initial replicas, multiple stages, sparse replica IDs,
repeat registration, existing counter preservation, new gauges/histograms, speculative position
counters, sleep-state preservation, and the shared orchestrator output-processing path.

From the repository root, with compatible vLLM/PyTorch, pytest and pytest-asyncio installed
(no model weights or GPU execution needed):

python -m pytest -q -o addopts='' tests/metrics/test_stat_logger.py
python -m pytest -q -o addopts='' -m 'core_model and cpu' \
  tests/metrics/test_stat_logger.py tests/engine/test_orchestrator.py \
  tests/engine/test_orchestrator_event_driven.py

The existing Buildkite L1 CPU sweeps collect these modules; no pipeline change is needed.

For serving validation, use the head/headless commands in #7296 with default log statistics.
Start one replica, send text requests, attach a second replica, then send concurrent text
requests and inspect /metrics. Text requests isolate this fix from the separate audio-cache
bug. Tested on two H200 GPUs with Qwen3-Omni-30B-A3B-Instruct thinker-only, TP=1 per replica,
memory utilization 0.60, prefix caching enabled, eager execution disabled, round-robin routing.

Test Result

  • CPU suites: 124 passed.
  • Negative control: the two late-replica orchestrator tests fail on unmodified main source
    with KeyError: (0, 3) (initial replica counts 0 and 1).
  • GPU serving with log stats enabled: 8/8 warmup requests and 64/64 requests after
    joining passed (concurrency 8), with no KeyError or output-processing failures.
  • Prometheus generation-token counter for replica 0: 16 → 80. Replica 1: 64.
    The existing counter was preserved and the new replica's awake gauge was 1.
  • Changed-file pre-commit checks pass except for 17 pre-existing mypy errors in
    orchestrator.py. Running the same hook on the same files in the base reports 20
    errors; comparison after normalizing line numbers shows no new diagnostics.

These are correctness checks, not throughput benchmarks. Optional KV-transfer connector
configurations and distributed multi-node serving were not exercised by the GPU run.

Signed-off-by: leegangtoe <leegangtoe@gmail.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@vllm-omni-review-bot

Copy link
Copy Markdown

This PR appears to belong to: docs/design/module/observability.md.

Module owners: @lishunyang12 @vraiti @tzhouam

Routing: @lishunyang12 via module of the changed files, semantic router, CODEOWNERS; @vraiti via module of the changed files, semantic router; @tzhouam via CODEOWNERS

@leegangtoe, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer.

Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment.

@vllm-omni-review-bot

vllm-omni-review-bot commented Sep 8, 2026 •

Copy link
Copy Markdown

Omni ReviewBot triage note

Automated triage of commit 47dd25c58890 produced:

  • Priority: high. Prompt maintainer attention is suggested.

These are automated triage suggestions only — the final decision belongs to the maintainers.

@hsliuustc0106 hsliuustc0106 added bug Something isn't working benchmark/profiler/metrics/logger Codes related to benchmarks, profiler, metrics and logger system labels Sep 8, 2026
@leegangtoe

Copy link
Copy Markdown
Contributor Author

Self-review completed on head efb39d7:

  • Reviewed the complete three-file diff against the base and matched the committed files to the source hashes from the validation runs. This branch does not contain the cache changes from [Bugfix] Preserve multimodal cache data across stage-0 replicas #7297.
  • Checked the shared output-processing path used by both orchestration loops: a late replica is registered before record(). Registration and index insertion contain no await, and retained indices are not deleted/reused by this change. Disabled statistics still bypass this path.
  • Reviewed the metric-table extension: it binds children to existing families rather than reconstructing collectors, preserves extra labels and speculative position counters, and leaves existing counters/sleep state intact. Empty heads initialize collectors on the first registration; repeated registration is idempotent and conflicting index reuse is rejected. New replicas start awake.
  • Rechecked 124 passing CPU tests, including the real logger/registry assertions. Running the two late-replica orchestration regressions with unmodified main source fails with KeyError: (0, 3) for initial replica counts 0 and 1.
  • Rechecked two-H200 serving with log stats enabled: 8/8 warmup and 64/64 post-join text requests passed. Replica 0's generation counter increased 16 → 80, replica 1 recorded 64, and its awake gauge was 1, with no KeyError. Text requests isolate the statistics fix from the audio-cache issue.
  • Checked validation limits: local pre-commit retains 17 pre-existing mypy errors in the orchestrator (20 on the same base files; no new diagnostics). GitHub pre-commit, both wheel builds, DCO, and docs checks pass. Connector-specific configurations and multi-node serving remain unvalidated; detached metric series are retained intentionally.

@Gaohan123 Gaohan123 added this to the v0.30.0 milestone Sep 19, 2026

@Gaohan123 Gaohan123 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM. Thanks

@Gaohan123 Gaohan123 added the ready label to trigger buildkite CI label Sep 19, 2026
@Gaohan123
Gaohan123 enabled auto-merge (squash) September 19, 2026 16:37
@Gaohan123
Gaohan123 merged commit fa506e0 into vllm-project:main Sep 19, 2026
6 of 9 checks passed
mlaneuville pushed a commit to mlaneuville/vllm-omni that referenced this pull request Sep 22, 2026
…ject#7298)

Signed-off-by: leegangtoe <leegangtoe@gmail.com>
Signed-off-by: Matthieu Laneuville <matthieu.laneuville@surf.nl>
khairulkabir1661 pushed a commit to khairulkabir1661/vllm-omni that referenced this pull request Sep 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

benchmark/profiler/metrics/logger Codes related to benchmarks, profiler, metrics and logger system bug Something isn't working ready label to trigger buildkite CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Adding a headless replica crashes output processing with log stats enabled

4 participants