(feat) Support multi-node aggregate jobs with direct vllm frontend - #296
Merged
Conversation
arpera
requested review from
alec-flowers,
csahithi,
ishandhanani and
nlevin-ui
as code owners
August 7, 2026 17:52
|
hi @arpera thanks for upstreaming this & improving upon it! |
kyleliang-nv
force-pushed
the
vllm-forntend-for-multinode
branch
from
August 11, 2026 08:20
18c1866 to
a7d57bc
Compare
Signed-off-by: Artem Perevedentsev <aperevedents@nvidia.com>
Signed-off-by: Artem Perevedentsev <aperevedents@nvidia.com>
Signed-off-by: Artem Perevedentsev <aperevedents@nvidia.com>
Signed-off-by: Kyle Liang <kylliang@nvidia.com>
Signed-off-by: Kyle Liang <kylliang@nvidia.com>
Signed-off-by: Kyle Liang <kylliang@nvidia.com>
Signed-off-by: Kyle Liang <kylliang@nvidia.com>
kyleliang-nv
force-pushed
the
vllm-forntend-for-multinode
branch
from
August 11, 2026 13:16
a7d57bc to
9a74c2f
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #296 +/- ##
=======================================
Coverage ? 72.61%
=======================================
Files ? 77
Lines ? 10698
Branches ? 0
=======================================
Hits ? 7768
Misses ? 2930
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Signed-off-by: Artem Perevedentsev <aperevedents@nvidia.com>
alec-flowers
approved these changes
Aug 11, 2026
…tinode Signed-off-by: Artem Perevedentsev <aperevedents@nvidia.com>
Signed-off-by: Kyle Liang <kylliang@nvidia.com>
nv-yna
added a commit
to nv-yna/srt-slurm
that referenced
this pull request
Aug 14, 2026
main's e32e591 (direct vLLM frontend, NVIDIA#296) split _public_api_node out of _orchestrator_node: for a single-worker direct-vLLM aggregated job the agg leader serves FRONTEND_PUBLIC_PORT, not the orchestrator. This branch was written before that split and paired the orchestrator node with that port, so on such a job the scraper would poll a node that is not listening and the run would come back with no frontend rows at all. Every other FRONTEND_PUBLIC_PORT pairing in the file already uses _public_api_node; this makes the scrape target agree. No behaviour change on the Dynamo path, where _public_api_node delegates to _orchestrator_node. Test asserts the frontend target lands on the agg leader and pins that the orchestrator would have been the wrong answer, so the two cannot silently converge again. Signed-off-by: Yuewei Na <nv-yna@users.noreply.github.com>
nv-yna
added a commit
to nv-yna/srt-slurm
that referenced
this pull request
Aug 14, 2026
Worker targets are the leaders' DYN_SYSTEM_PORT, which only Dynamo serves. main's NVIDIA#296 made the divergence explicit: _logical_worker_endpoints now selects sys_port / frontend_port / http_port by frontend.type, while this capture always assumed the first. Scoping rather than generalising, because the scope was already there implicitly -- expand_observability sets publish_events_and_metrics and the DYN_LOGGING_* span env, none of which a non-Dynamo frontend consumes, so such a run has no worker surface for this to find. Say so in the docstring and point at _logical_worker_endpoints for the mapping if it ever needs one. A non-Dynamo run now returns the frontend target with a warning instead of worker URLs nothing is listening on. Emitting them would have produced a run that looks instrumented and comes back with no worker rows -- the exact failure this knob exists to prevent -- with only a rate-limited scraper warning to explain it. Signed-off-by: Yuewei Na <nv-yna@users.noreply.github.com>
nv-yna
added a commit
to nv-yna/srt-slurm
that referenced
this pull request
Aug 17, 2026
main's e32e591 (direct vLLM frontend, NVIDIA#296) split _public_api_node out of _orchestrator_node: for a single-worker direct-vLLM aggregated job the agg leader serves FRONTEND_PUBLIC_PORT, not the orchestrator. This branch was written before that split and paired the orchestrator node with that port, so on such a job the scraper would poll a node that is not listening and the run would come back with no frontend rows at all. Every other FRONTEND_PUBLIC_PORT pairing in the file already uses _public_api_node; this makes the scrape target agree. No behaviour change on the Dynamo path, where _public_api_node delegates to _orchestrator_node. Test asserts the frontend target lands on the agg leader and pins that the orchestrator would have been the wrong answer, so the two cannot silently converge again. Signed-off-by: Yuewei Na <nv-yna@users.noreply.github.com>
nv-yna
added a commit
to nv-yna/srt-slurm
that referenced
this pull request
Aug 17, 2026
Worker targets are the leaders' DYN_SYSTEM_PORT, which only Dynamo serves. main's NVIDIA#296 made the divergence explicit: _logical_worker_endpoints now selects sys_port / frontend_port / http_port by frontend.type, while this capture always assumed the first. Scoping rather than generalising, because the scope was already there implicitly -- expand_observability sets publish_events_and_metrics and the DYN_LOGGING_* span env, none of which a non-Dynamo frontend consumes, so such a run has no worker surface for this to find. Say so in the docstring and point at _logical_worker_endpoints for the mapping if it ever needs one. A non-Dynamo run now returns the frontend target with a warning instead of worker URLs nothing is listening on. Emitting them would have produced a run that looks instrumented and comes back with no worker rows -- the exact failure this knob exists to prevent -- with only a rate-limited scraper warning to explain it. Signed-off-by: Yuewei Na <nv-yna@users.noreply.github.com>
nv-yna
added a commit
that referenced
this pull request
Aug 17, 2026
* feat(observability): in-job RAW /metrics scraper
Worker and frontend /metrics endpoints are served by the live processes.
When the SLURM job ends they disappear and the data is gone -- there is
no after-the-fact scrape. Anything wanting a time series of KV-cache
occupancy, router queue depth or frontend event-loop pressure has to
capture it DURING the run.
Adds a daemon thread, started with the benchmark stage and gated on
observability.scrape_metrics (which defaults to observability.enabled),
that appends each response body VERBATIM to raw_prometheus.jsonl:
{"timestamp_ns":..., "endpoint_url":..., "role":..., "worker_id":...,
"text": "<raw Prometheus exposition, UNPARSED>"}
Capture emits RAW, the processor parses -- so a parser fix never requires
re-running the job. timestamp_ns is wall-epoch, aligning with client
records and spans on the same clock.
Best-effort by construction: an unreachable endpoint costs one line and
a rate-limited warning, never the benchmark. It stops on its own signal
as well as the shared stop_event, because the latter only fires on abort
-- relying on it alone would leave the scraper polling endpoints that are
being torn down.
Targets are the frontend plus every worker LEADER; follower ranks would
duplicate rows under a misleading worker_id.
Verified on Lyris jobs 2674508 and 2674375: 7625 lines across all three
roles, worker_id set on 6100/6100 worker rows, trtllm_kv_cache_* present
in the raw text, 4572 s scrape window.
Signed-off-by: Yuewei Na <nv-yna@users.noreply.github.com>
* fix(observability): gate RAW scrape-target resolution on the opt-in
try_start_raw_scraper checks observability.scraper_enabled internally, but
the target list was built inside its argument list, so Python evaluated it
before the guard could ever run. Two consequences:
- _analytics_scrape_targets() reaches self.backend_processes, which
BenchmarkStageMixin declares as a NotImplementedError seam. Any caller
that does not implement it now crashes in _run_benchmark_script --
including the TestBenchmarkChildReaping harness on main, which is what
turned CI red.
- Every target costs a get_hostname_ip() srun round-trip inside a Slurm
job. That was paid on each benchmark run even with the scraper switched
off, and it sat outside the callee's best-effort try/except, so a
resolution failure was fatal to a run that never wanted the scraper.
Check the opt-in at the call site so targets are resolved only when the
scraper actually starts. The check is `is True` rather than plain
truthiness on purpose: scraper_enabled is a bool property, but this mixin
is routinely driven with a mocked config whose every attribute is truthy,
and truthiness would silently switch the scraper on for those callers.
Signed-off-by: Yuewei Na <nv-yna@users.noreply.github.com>
* style: apply ruff format to test_metrics_scraper
Signed-off-by: Yuewei Na <nv-yna@users.noreply.github.com>
* fix(observability): scrape the frontend on the public API node
main's e32e591 (direct vLLM frontend, #296) split _public_api_node out of
_orchestrator_node: for a single-worker direct-vLLM aggregated job the agg
leader serves FRONTEND_PUBLIC_PORT, not the orchestrator. This branch was
written before that split and paired the orchestrator node with that port,
so on such a job the scraper would poll a node that is not listening and
the run would come back with no frontend rows at all.
Every other FRONTEND_PUBLIC_PORT pairing in the file already uses
_public_api_node; this makes the scrape target agree. No behaviour change on
the Dynamo path, where _public_api_node delegates to _orchestrator_node.
Test asserts the frontend target lands on the agg leader and pins that the
orchestrator would have been the wrong answer, so the two cannot silently
converge again.
Signed-off-by: Yuewei Na <nv-yna@users.noreply.github.com>
* docs(observability): declare the RAW worker capture Dynamo-scoped
Worker targets are the leaders' DYN_SYSTEM_PORT, which only Dynamo serves.
main's #296 made the divergence explicit: _logical_worker_endpoints now
selects sys_port / frontend_port / http_port by frontend.type, while this
capture always assumed the first.
Scoping rather than generalising, because the scope was already there
implicitly -- expand_observability sets publish_events_and_metrics and the
DYN_LOGGING_* span env, none of which a non-Dynamo frontend consumes, so
such a run has no worker surface for this to find. Say so in the docstring
and point at _logical_worker_endpoints for the mapping if it ever needs one.
A non-Dynamo run now returns the frontend target with a warning instead of
worker URLs nothing is listening on. Emitting them would have produced a run
that looks instrumented and comes back with no worker rows -- the exact
failure this knob exists to prevent -- with only a rate-limited scraper
warning to explain it.
Signed-off-by: Yuewei Na <nv-yna@users.noreply.github.com>
* feat(observability): default the scrape interval to 1s
3s was a conservative first guess. The panels this feeds are per-request
timescale -- KV-cache occupancy and queue depth move within a single
decode burst -- so a 3s cadence aliases exactly the transients the capture
exists to show.
Changed in all three places the default is spelled: the schema field, the
RawMetricsScraper constructor, and try_start_raw_scraper's getattr fallback
for configs that omit the field. A test pins them together so they cannot
drift apart again.
Cost is bounded by construction: the 0.5s floor still applies, and the
drift-free pacing means a sweep slower than the interval runs back-to-back
rather than queueing. Volume scales with it -- the 76-minute Lyris run
produced 7625 lines at 3s, so expect roughly 3x that at 1s.
Signed-off-by: Yuewei Na <nv-yna@users.noreply.github.com>
---------
Signed-off-by: Yuewei Na <nv-yna@users.noreply.github.com>
Co-authored-by: Yuewei Na <nv-yna@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add vllm frontend support for aggregated multi-node vllm jobs. Initial work on adding support vllm frontend for aggregated single-node vllm jobs has been done in #278. Then in srt-slurm fork @functionstackx added support for multi-node case functionstackx#1 (which is the base for this PR but it has been extended a lot; also improves docs).
This vllm frontend support is required to make reproducible Kimi-K3 multi-node recipes uploaded to SA here SemiAnalysisAI/InferenceX#2391