Skip to content

feat(model): add shared NGC fetching support with vLLM/SGLang integration - #15441

Merged
rmccorm4 merged 10 commits into
mainfrom
sunil1511/ngc-model-fetching
Oct 6, 2026
Merged

rmccorm4 merged 10 commits into
mainfrom
sunil1511/ngc-model-fetching

Conversation

@sunil1511

@sunil1511 sunil1511 commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Overview / Summary

Add shared NGC model fetching through ModelExpress and integrate it into vLLM and SGLang worker startup.

This builds on ai-dynamo/modelexpress#232. NGC sources are resolved to local directories before engine initialization, while existing Hugging Face ModelExpress acquisition and P2P paths are preserved.

Details

  • Route ngc:// sources through ModelExpress’s NGC provider.
  • Reuse cached NGC models when the required configuration, tokenizer, and weight files are present.
  • Resolve NGC sources before vLLM and SGLang construct their engine configurations, including when ModelExpress weight loaders are enabled.
  • Pass the resolved local directory to engine initialization and model registration.
  • Require an explicit, colon-free --served-model-name for NGC models in SGLang.
  • Add regression tests for provider routing, cache completeness, and backend source handling.

Existing Hugging Face behavior

The new full-download and local-path rewrite behavior applies only to NGC sources.

HF models retain their source identifiers. With ModelExpress loaders enabled, vLLM and SGLang continue to skip Dynamo’s full weight prefetch, and model registration continues to request metadata only.

Scope and NGC P2P limitation

The fetching implementation is shared, but this PR integrates it only into the standard vLLM and SGLang worker startup paths. TensorRT-LLM and other entry points require separate integration and validation.

NGC weights are downloaded or found in the local cache before the ModelExpress weight loader runs. Consequently, peer loading cannot avoid the initial NGC download on a cold-cache worker.

P2P is not explicitly disabled, but peer matching currently depends on the resolved local path and other compatibility fields. Optimized NGC P2P startup requires follow-up work on metadata-only preparation, stable source identity, and NGC-aware fallback.

Where should the reviewer start?

  1. lib/llm/src/hub.rs
    • Provider selection, provider-specific cache lookup, and required-file validation.
  2. components/src/dynamo/vllm/args.py and components/src/dynamo/sglang/args.py
    • NGC resolution before engine configuration and preservation of HF source handling.
  3. components/src/dynamo/vllm/main.py and components/src/dynamo/vllm/worker_factory.py
    • Resolved source propagation and existing ModelExpress prefetch behavior.

Validation

  • Seven targeted HF regression tests passed across vLLM and SGLang.
  • Backend startup and inference were exercised with Qwen3-0.6B weights seeded into an NGC cache layout.
  • ModelExpress-enabled startup was exercised through local fallback.
  • Authenticated NGC downloads and actual two-worker P2P transfers have not yet been validated.
  • Other backend entry points were not validated.

Related Issues

Summary by CodeRabbit

  • New Features
    • Added support for downloading and running models identified by ngc:// across supported inference engines.
    • Cached NGC models can be reused without network access when required configuration, tokenizer, and weight files are available.
    • NGC models require a valid served-model name; provided names and aliases are preserved.
    • Model-source reporting now reflects the local path used by the engine for downloaded models.

@sunil1511
sunil1511 requested review from a team as code owners September 30, 2026 18:54
@copy-pr-bot

copy-pr-bot Bot commented Sep 30, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions Bot added feat backend::vllm Relates to the vllm backend backend::sglang Relates to the sglang backend labels Sep 30, 2026
@sunil1511 sunil1511 self-assigned this Sep 30, 2026
@sunil1511
sunil1511 marked this pull request as draft September 30, 2026 18:55
@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: ai-dynamo/dynamo/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 5d30b118-c676-4c5d-ae88-669ea05f2ddc

📥 Commits

Reviewing files that changed from the base of the PR and between df8ad66 and 256f2e9.

📒 Files selected for processing (10)
  • components/src/dynamo/common/model_fetch.py
  • components/src/dynamo/common/tests/test_model_fetch.py
  • components/src/dynamo/sglang/args.py
  • components/src/dynamo/sglang/tests/test_sglang_unit.py
  • components/src/dynamo/vllm/args.py
  • components/src/dynamo/vllm/main.py
  • components/src/dynamo/vllm/tests/test_vllm_model_source.py
  • components/src/dynamo/vllm/worker_factory.py
  • lib/llm/src/hub.rs
  • lib/llm/src/local_model.rs

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.


Walkthrough

The change adds NGC provider selection for model fetching and cache lookup. SGLang and vLLM resolve NGC model URIs to local paths and pass those paths into their model setup flows.

Changes

NGC model resolution

Layer / File(s) Summary
Provider and cache resolution
lib/llm/src/hub.rs, lib/llm/src/local_model.rs
from_hf selects NGC for ngc:// model names and uses the selected provider for cache lookup and downloads. Cache validation checks for configuration and tokenizer files, plus weights when required. Tests cover provider selection and NGC cache cases; documentation names NGC as a download source.
SGLang local-path resolution
components/src/dynamo/common/model_fetch.py, components/src/dynamo/common/tests/test_model_fetch.py, components/src/dynamo/sglang/args.py, components/src/dynamo/sglang/tests/test_sglang_unit.py
A helper identifies NGC model URIs. SGLang fetches models that need local paths, rejects missing or invalid served names before fetching, and uses the fetched path. Tests cover NGC arguments and served-name validation.
vLLM model path and source propagation
components/src/dynamo/vllm/args.py, components/src/dynamo/vllm/main.py, components/src/dynamo/vllm/worker_factory.py, components/src/dynamo/vllm/tests/test_vllm_model_source.py
An asynchronous parsing path fetches models that need local paths and sets the engine model path. vLLM uses the resolved model source for registration and worker paths. Tests cover NGC load formats, explicit served names, and Hugging Face model handling.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to 256f2

No actionable merge-blocking issue was identified. NGC cache-layout compatibility remains unverified, and authenticated downloads and two-worker transfers still need validation.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 47.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 36 functions across 10 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: shared NGC fetching support integrated with vLLM and SGLang.
Description check ✅ Passed The description covers the overview, implementation details, reviewer starting points, validation results, scope limitations, and related issue information. It is mostly complete and directly describe…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Comment @coderabbitai help to get the list of available commands.

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR

@sunil1511
sunil1511 force-pushed the sunil1511/ngc-model-fetching branch from 2c82022 to 991eaba Compare September 30, 2026 23:19
@sunil1511
sunil1511 marked this pull request as ready for review September 30, 2026 23:19
Comment thread components/src/dynamo/common/tests/test_model_fetch.py Outdated
Comment thread components/src/dynamo/sglang/tests/test_sglang_unit.py Outdated
Comment thread components/src/dynamo/sglang/tests/test_sglang_unit.py Outdated
Comment thread components/src/dynamo/sglang/tests/test_sglang_unit.py Outdated
Comment thread components/src/dynamo/vllm/args.py Outdated
Comment thread lib/llm/src/hub.rs Outdated
Comment thread lib/llm/src/hub.rs Outdated
Comment thread lib/llm/src/hub.rs Outdated
Comment thread lib/llm/src/hub.rs Outdated
Comment thread lib/llm/src/hub.rs Outdated
@rmccorm4

rmccorm4 commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

/ok to test 5ce7124

@devin-ai-integration

devin-ai-integration Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

❌ Dynamo PR CI failed — run 37377438887 (attempt 3) on 6875f90814

Gate checks: ❌ backend-status-check · ✅ deploy-status-check · ❌ dynamo-status-check

Other Jobs
dynamo-runtime ❌ 2 ⏹️ 1 ✅ 5
frontend ⏹️ 1

Failure details

2 jobs failed (amd64 and arm64) with the same 2 tests in tests/fault_tolerance/cancellation/test_utils.py timing out after 30s waiting for the HTTP response. The 2 cancelled jobs (dynamo-runtime / rust-gpu, frontend / Build multi-arch cpu) were cancelled within minutes of the arm64 failure, so they look like fail-fast cancels.

❌ dynamo-runtime / test / parallel cuda13.0, amd64: test_drained_stream_* requests.exceptions.Timeout (30s)

Job: dynamo-runtime / test / parallel cuda13.0, amd64 · Failed step: Run CPU-only tests (parallelized) · Logs: gh run view --job 112071996083 -R ai-dynamo/dynamo --log-failed

______________ test_drained_stream_can_require_generated_content _______________
tests/fault_tolerance/cancellation/test_utils.py:50: in test_drained_stream_can_require_generated_content
    read_streaming_responses(
tests/fault_tolerance/cancellation/utils.py:309: in read_streaming_responses
    response_raw = cancellable_req.get_response()
        drain      = True
        require_content = True
tests/fault_tolerance/cancellation/utils.py:188: in get_response
    raise requests.exceptions.Timeout(
E   requests.exceptions.Timeout: Timed out after 30.0s waiting for the HTTP response
FAILED tests/fault_tolerance/cancellation/test_utils.py::test_drained_stream_can_require_generated_content - requests.exceptions.Timeout: Timed out after 30.0s waiting for the HTTP response
FAILED tests/fault_tolerance/cancellation/test_utils.py::test_drained_stream_accepts_generated_content - requests.exceptions.Timeout: Timed out after 30.0s waiting for the HTTP response
=== 2 failed, 789 passed, 23 skipped, 5401 deselected in 1139.89s (0:18:59) ====

test_drained_stream_can_require_generated_content and test_drained_stream_accepts_generated_content both fail in CancellableRequest.get_response() (utils.py:188), which got no HTTP response within 30s (drain=True, require_content=True). Both failed again on every auto-retry. A later Upload test logs error about an * in the artifact path test_unsafe_or_nonexact_protobuf_pins[protobuf==6.33.*] is a follow-on step error, not the cause.

❌ dynamo-runtime / test / parallel cuda13.0, arm64: same test_drained_stream_* Timeout (30s)

Job: dynamo-runtime / test / parallel cuda13.0, arm64 · Failed step: Run CPU-only tests (parallelized) · Logs: gh run view --job 112071996268 -R ai-dynamo/dynamo --log-failed

tests/fault_tolerance/cancellation/utils.py:188: in get_response
    raise requests.exceptions.Timeout(
E   requests.exceptions.Timeout: Timed out after 30.0s waiting for the HTTP response
FAILED tests/fault_tolerance/cancellation/test_utils.py::test_drained_stream_can_require_generated_content - requests.exceptions.Timeout: Timed out after 30.0s waiting for the HTTP response
FAILED tests/fault_tolerance/cancellation/test_utils.py::test_drained_stream_accepts_generated_content - requests.exceptions.Timeout: Timed out after 30.0s waiting for the HTTP response
=== 2 failed, 789 passed, 23 skipped, 5401 deselected in 1472.27s (0:24:32) ====

The same 2 tests fail the same way as on amd64: no HTTP response from CancellableRequest.get_response() within 30s. All other tests passed.

For agents
{"pr": 15441, "run_id": 37377438887, "run_attempt": 3, "head_sha": "6875f908142108c41b68cc42215f0de45c2bdef6",
 "failures": [
  {"job": "dynamo-runtime / test / parallel cuda13.0, amd64", "job_id": 112071996083, "failed_step": "Run CPU-only tests (parallelized)", "signature": "requests.exceptions.Timeout: Timed out after 30.0s waiting for the HTTP response", "tests": ["tests/fault_tolerance/cancellation/test_utils.py::test_drained_stream_can_require_generated_content", "tests/fault_tolerance/cancellation/test_utils.py::test_drained_stream_accepts_generated_content"], "log_cmd": "gh run view --job 112071996083 -R ai-dynamo/dynamo --log-failed"},
  {"job": "dynamo-runtime / test / parallel cuda13.0, arm64", "job_id": 112071996268, "failed_step": "Run CPU-only tests (parallelized)", "signature": "requests.exceptions.Timeout: Timed out after 30.0s waiting for the HTTP response", "tests": ["tests/fault_tolerance/cancellation/test_utils.py::test_drained_stream_can_require_generated_content", "tests/fault_tolerance/cancellation/test_utils.py::test_drained_stream_accepts_generated_content"], "log_cmd": "gh run view --job 112071996268 -R ai-dynamo/dynamo --log-failed"}
 ]}

Posted automatically by Devin for run 37377438887. Updated on every full-CI run of this PR.

@peilii peilii 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.

SGLang changes LGTM

Comment thread components/src/dynamo/vllm/args.py
@sunil1511
sunil1511 force-pushed the sunil1511/ngc-model-fetching branch 2 times, most recently from be886a0 to b9240ee Compare October 3, 2026 00:15
…tion

Route NGC sources through ModelExpress and reuse complete cached models. Resolve local paths before vLLM and SGLang engine initialization while preserving existing Hugging Face P2P acquisition paths.

Signed-off-by: sunil1511 <sunilupare45@gmail.com>
Signed-off-by: sunil1511 <sunilupare45@gmail.com>
Signed-off-by: sunil1511 <sunilupare45@gmail.com>
…e to all consumers

Signed-off-by: sunil1511 <sunilupare45@gmail.com>
@sunil1511
sunil1511 force-pushed the sunil1511/ngc-model-fetching branch from b9240ee to 48af879 Compare October 3, 2026 00:21
Comment thread components/src/dynamo/vllm/main.py Outdated
@rmccorm4

rmccorm4 commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

/ok to test 6875f90

@rmccorm4
rmccorm4 enabled auto-merge (squash) October 5, 2026 21:40
@rmccorm4

rmccorm4 commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

/ok to test 228ded5

@rmccorm4
rmccorm4 merged commit 6c7ab96 into main Oct 6, 2026
124 checks passed
@rmccorm4
rmccorm4 deleted the sunil1511/ngc-model-fetching branch October 6, 2026 05:33
Arsene12358 added a commit that referenced this pull request Oct 9, 2026
Merges origin/main at ce45db9. Four files conflicted, all resolved
by keeping both sides:

- instrumented_scheduler.py imports: this branch's importlib.metadata
  version import and main's itertools chain (vLLM 0.31.0 bump, #15643).
- worker_factory.py imports from instrumented_scheduler: main's
  InstrumentedScheduler (#12545) and this branch's
  benchmark_content_point_key.
- test_vllm_worker_factory.py imports: main's Config (#15441),
  ENV_FPM_WORKER_ID, InstrumentedScheduler and
  FPM_SET_WORKER_ID_METHOD_NAME (#12545), next to this branch's
  FpmBenchmarkWorkerExtension, ENV_FPM_BENCHMARK_OUTPUT_PATH,
  benchmark_content_point_key and the probe and restore timeouts.
- test_vllm_instrumented_scheduler.py: both sides appended tests at the
  end of the file. This branch's engine provenance and measurement tests
  come first, then main's FPM worker_id propagation tests (#12545).

Main's vLLM 0.31.0 bump deleted test_vllm_kv_cache_metadata_compat.py,
which the realseed_prefix_cache fixture's comment named as the
importorskip probe that its function-local KVCacheManager import
protects. The comment now names test_vllm_dcp_kv_events.py, which still
probes vllm.v1.core.kv_cache_manager.

No code change for vLLM 0.31.0: the vLLM interfaces this branch uses
(collective_rpc by method name, worker extension mixing,
cudagraph_metrics, CUDAGraphStat, the stat loggers, the fields the
engine probe and the provenance capture read) are unchanged from 0.30.0.

Signed-off-by: Yiming Liu <yimingl@nvidia.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend::sglang Relates to the sglang backend 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