mudler / mudler/vllm.cpp

fix(SERVE-METRICS): a zero-output step records no scheduler stats, so the gauges freeze

Open
#2,651 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
423
Forks
53
Avg merge
20h 26m
Merged PRs (30d)
310

Description

Row: SERVE-METRICS

Found by #2647 (PORT-NOW wave
PORTQ-3, tranche 81-120). Nothing was executed: this is a static reading of the
tree at e24ec8bfd against upstream dedbf6be8b (vllm#48668).

What is missing

Upstream dedbf6be8b records scheduler_stats on every engine step and
narrows the "outputs are non-empty" condition so it gates only the
IterationStats construction and the interval log. Prefix-cache and queue gauges
therefore keep moving on a step that produced no request output.

This tree still gates the whole Record call on non-empty outputs, on both
frontends:

  • src/vllm/v1/engine/llm_engine.cpp:219
    if (stat_logger_ != nullptr && !engine_core_outputs.outputs.empty()),
    with the comment at :217 saying "Guarded by outputs>0 exactly as upstream" —
    which was true before this commit and is now false;
  • src/vllm/v1/engine/async_llm.cpp:580
    if (logger != nullptr && !outputs.outputs.empty()).

One level down the dependency is structural: src/vllm/v1/engine/core.cpp:95
only puts an EngineCoreOutputs into outputs_by_client when
!engine_core_outputs.outputs.empty(), so a zero-output step produces no client
entry at all and the stats never reach a frontend to be dropped.

The IterationStats half already matches post-commit upstream on the async path
(async_llm.cpp:547-548 builds it only for non-empty outputs).

Why it is user-visible

SERVE-METRICS is the Prometheus endpoint a real deployment scrapes. A busy
engine whose steps produce no output — a long prefill, a step where every request
is still accumulating — currently freezes running, waiting and the
prefix-cache gauges instead of reporting them. A frozen gauge reads as an idle
engine, which is the same failure shape #277 already fixed once on this row for
the async frontend.

Size

Roughly 20-40 product lines over three files: emit the client entry (or a
stats-only channel) on zero-output steps in core.cpp, and drop the two Record
guards. Plus a RED-first test that drives a zero-output step and asserts the
gauges move — without it the change is unobservable.

The interval-log half of the commit is not applicable: do_log_stats_with_interval
has no counterpart here, the tree has no interval logger.

Care needed

src/vllm/v1/engine/core.cpp:83-86 records the DP and finished_requests
signalling around that map as deferred. Emitting an entry on a zero-output step
touches that path, so the port owes a reading of what else consumes the map. That
affects the size estimate above, not the defect.

Not in scope here

This issue does not advance the parity pin. dedbf6be8b is inside the
5559679229..e126687a9a window that #2611
owns.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading the outputs_by_client flow in src/vllm/v1/engine/core.cpp:83-95, then compare the Record guards in src/vllm/v1/engine/llm_engine.cpp:217-219 and src/vllm/v1/engine/async_llm.cpp:547-580. Add a RED-first test for a zero-output step and verify that the running, waiting, and prefix-cache gauges update while deferred DP and finished-request signalling remain correct.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, prometheus
Domain
observability
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
56/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.