Skip to content

[Bugfix] Wire iteration_stats through StagePool.process_llm_raw_outputs - #4747

Closed
amanchugh89 wants to merge 2 commits into
vllm-project:mainfrom
amanchugh89:fix/orchestrator-iteration-stats
Closed

amanchugh89 wants to merge 2 commits into
vllm-project:mainfrom
amanchugh89:fix/orchestrator-iteration-stats

Conversation

@amanchugh89

@amanchugh89 amanchugh89 commented Jun 26, 2026 •

Copy link
Copy Markdown

What

StagePool.process_llm_raw_outputs accepts an iteration_stats arg but overwrites it with a fresh
IterationStats() before calling the output processor, so the processor fills a throwaway and the
caller's object stays empty.

The orchestrator passes its own IterationStats and then records it via OmniPrometheusStatLogger
(inherits PrometheusStatLogger.record, which reads it). So per-(stage, replica) prompt/generation
token counts, preemption / corrupted-request counters, and TTFT/TPOT/ITL histograms are recorded as
zero whenever Prometheus logging is enabled.

Fix

iteration_stats = iteration_stats or IterationStats()

Allocate only when the caller passes None; otherwise the processor fills the caller's object.

Tests

tests/engine/test_stage_pool_iteration_stats.py — asserts the caller's IterationStats reaches
process_outputs by identity (fails on main, passes with the fix), plus the None path. ruff
clean; suite runs in CI.

Not a duplicate: no open PR touches this wiring; the arg was added in #3628 but never threaded
through. AI assistance was used; every line reviewed.

`process_llm_raw_outputs` accepts an `iteration_stats` argument but then
unconditionally did `iteration_stats = IterationStats()`, so the output
processor populated a throwaway local instead of the caller's object.
The orchestrator builds its own `IterationStats`, passes it in, and then
records metrics off that same object via the stat logger, so per-(stage,
replica) iteration metrics were always recorded empty.

Allocate a fresh `IterationStats` only when the caller passes None.

Co-authored-by: Claude <noreply@anthropic.com>
Signed-off-by: amanchugh89 <amanchugh.89@gmail.com>
@amanchugh89
amanchugh89 marked this pull request as ready for review June 26, 2026 14:29
@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.

@amanchugh89

Copy link
Copy Markdown
Author

Validated the regression on the actual code (vLLM 0.23.0): with the fix, tests/engine/test_stage_pool_iteration_stats.py passes (2 passed); reverting just the one-line change makes test_caller_iteration_stats_not_clobbered fail (1 failed, 1 passed) — the identity assertion catches the clobber while the None-path test is unaffected. So it's a genuine red→green regression, not only the reasoning above.

@amanchugh89

Copy link
Copy Markdown
Author

@tzhouam @yenuo26 if you guys have have a pass at this PR ?

@amanchugh89

Copy link
Copy Markdown
Author

Closing for now — hasn't attracted review and I'm focusing elsewhere. The fix + regression test are validated (red→green) if anyone wants to pick it up; happy to reopen. Thanks!

@amanchugh89 amanchugh89 closed this Jul 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant