Skip to content

fix(mx): fall back to MODEL_EXPRESS_URL env var for Dynamo integration - #1

Closed
KavinKrishnan wants to merge 5 commits into
chienchunhung:dynamo-integration-prototypefrom
KavinKrishnan:kavink/mx-compat-fixes
Closed

fix(mx): fall back to MODEL_EXPRESS_URL env var for Dynamo integration#1
KavinKrishnan wants to merge 5 commits into
chienchunhung:dynamo-integration-prototypefrom
KavinKrishnan:kavink/mx-compat-fixes

Conversation

@KavinKrishnan

Copy link
Copy Markdown

Summary

When the Dynamo engine sets --model-express-url, it propagates the URL via MODEL_EXPRESS_URL environment variable before TRT-LLM LLM() init. The MXCheckpointLoader is then constructed via BaseCheckpointLoader.get("MX") factory, which doesn't pass mx_server_url directly.

This fix reads MODEL_EXPRESS_URL from the environment when mx_server_url is not passed to the constructor, enabling the Dynamo → TRT-LLM integration path.

Changes

  • mx/checkpoint_loader.py: Add os.environ.get("MODEL_EXPRESS_URL") fallback in __init__

Tested

Validated on GCP GB200 with Kimi K2.5 (TP=8, 2 nodes). Source DGD auto-detected source mode, loaded from disk, published to MX server. MX P2P weight transfer succeeded on target (confirmed in logs: MX P2P weight transfer succeeded).

Made with Cursor

chienchunhung and others added 3 commits April 13, 2026 20:52
…haring

Implement the two-axis integration model (checkpoint_format × LoadFormat)
for MX (Model eXchange P2P transfer) and GMS (GPU Memory Service) into
TRT-LLM's PyTorch backend. MX replaces the weight source (P2P instead of
disk) while GMS replaces memory management (shared GPU pool with RW/RO
dual paths). The two axes compose independently, enabling four modes:
pure TRT-LLM, MX-only, GMS-only, and MX+GMS.

Key changes:
- MXCheckpointLoader: P2P weight transfer via modelexpress SDK with
  automatic disk fallback, identity-based source discovery, and pre-
  post_load_weights publish timing
- GMSBackend: Protocol-based abstraction at _torch/memory/ with RW path
  (load under GMS mem pool, commit for readers) and RO path (zero-copy
  materialize with correct post_load_weights ordering)
- _weights_presharded flag on Linear modules to skip TP slicing for
  pre-sharded weights from MX P2P or GMS RO
- New config fields: mx_server_url, gms_socket_path, gms_mode, gms_tag

Signed-off-by: Chien-Chun Hung <chienchunh@gmail.com>
Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
…#7575)

Close API consistency gaps between our GMSBackend and the GMS library's
integration from merged dynamo PR NVIDIA#7575:

- finalize_write() now does register+commit+unmap+disconnect+reconnect-RO+remap,
  matching finalize_gms_write() from gpu_memory_service.integrations.common.utils
- connect() applies patch_empty_cache() to prevent segfault when
  torch.cuda.empty_cache() encounters VMM-backed GMS allocations
- Add move_untracked_params() to relocate stray parameters (allocated outside
  use_mem_pool context) into the GMS pool before finalize_write()
- Add move_untracked_params() to GPUMemoryBackend protocol
- RW path in model_loader calls move_untracked_params() before finalize_write()
- RO path documents MoE load balancer compatibility with GMS materialization

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
…t passed

The Dynamo engine integration sets MODEL_EXPRESS_URL as an environment
variable before LLM init. When MXCheckpointLoader is constructed via
BaseCheckpointLoader.get() factory (which doesn't pass mx_server_url),
the loader needs to read the URL from the environment.

Signed-off-by: Kavin Krishnan <kavink@nvidia.com>
Made-with: Cursor
…sh_model_params

Replace stub _try_p2p_transfer() and publish_as_source() with proven
implementations from modelexpress.trtllm_live_transfer. Add source
probe via _has_existing_sources() to avoid 1-hour timeout on source
instances. Use MODEL_EXPRESS_TARGET env var for explicit target mode.

Made-with: Cursor
Signed-off-by: Kavin Krishnan <kavink@nvidia.com>
Made-with: Cursor
@chienchunhung
chienchunhung force-pushed the dynamo-integration-prototype branch from 84dfb2a to 62ac40f Compare April 20, 2026 21:22
chienchunhung added a commit that referenced this pull request Apr 21, 2026
… timeout, model_name plumbing

Three discrete improvements to the MX side of PR NVIDIA#13045 driven by
review feedback from MX team's downstream PR
(chienchunhung/TensorRT-LLM #1) — three orchestration ergonomics fixes
landed as one focused commit so reviewers see them as a clean slice
on top of the prototype.

(1) MODEL_EXPRESS_URL env-var fallback — at validator level

  TorchLlmArgs.validate_mx_config now honors the upstream
  ``MODEL_EXPRESS_URL`` env var when ``checkpoint_format='MX'`` and
  ``mx_server_url`` is unset. Resolution happens at validator time so
  the value ends up on ``llm_args.mx_server_url`` (visible to
  logging, /startup_metrics, downstream code) instead of being
  silently re-read from env by the loader.

  Lets orchestrators (Dynamo) configure MX via the environment
  without plumbing every CLI knob, while keeping resolution in one
  place. Explicit ``mx_server_url=`` always wins. The env-var
  fallback only fires when MX is the active checkpoint format
  (so HF-only configs aren't surprised by an unrelated env var).
  Empty string in env is treated as unset.

(2) MX_SOURCE_QUERY_TIMEOUT defensive default

  MXCheckpointLoader.__init__ calls
  ``os.environ.setdefault("MX_SOURCE_QUERY_TIMEOUT", "30")`` whenever
  an MX server URL is configured. Caps cold-cluster first-replica
  startup at 30 s instead of upstream's 1-hour default (the polling
  in MxLiveWeightLoader._query_source). setdefault semantics preserve
  any explicit user value. HF-only loads (no MX URL) don't touch
  the env at all.

  The proper upstream-side fix is a non-blocking source-query API
  (tracked as MX-4 in §15 of the design doc); this defensive default
  caps the worst case until that lands.

(3) model_name plumbing with HF-snapshot-aware resolver

  Plumbs ``llm_args.model → MXCheckpointLoader(model_name=...)`` so
  upstream's ``publish_model_params()`` publishes under the
  user-supplied Hub ID (e.g. "Qwen/Qwen2.5-72B-Instruct") instead of
  the "unknown" sentinel.

  - MXCheckpointLoader takes a new optional ``model_name``
    constructor arg (Union[str, Path]). Coerced to str at
    construction time.
  - publish_as_source() now sets BOTH MODEL_EXPRESS_URL and
    MODEL_NAME env vars (resolving identity via the priority order
    below) and restores both env vars in finally.
    publish_model_params() reads them via env, as documented.
  - Identity resolution order: explicit constructor arg →
    MODEL_NAME env → checkpoint_dir basename (with HF-snapshot path
    unmangling) → "unknown".
  - HF cache layout (".../models--<org>--<name>/snapshots/<sha>/")
    is unmangled back to "<org>/<name>" instead of returning the
    commit hash.
  - _construct_checkpoint_loader plumbs ``mx_model_name`` through;
    py_executor_creator.py extracts it from llm_args.model.

  Both env-var dances (MODEL_EXPRESS_URL + MODEL_NAME) collapse into
  one direct call when MX-2 (public build_identity) lands upstream.

Tests for these three additions are in the next commit.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
Made-with: Cursor
@chienchunhung
chienchunhung force-pushed the dynamo-integration-prototype branch from 62ac40f to b78dafc Compare April 21, 2026 20:30
@chienchunhung
chienchunhung force-pushed the dynamo-integration-prototype branch from b78dafc to edb4f0f Compare April 21, 2026 22:00
chienchunhung added a commit that referenced this pull request Apr 21, 2026
… timeout, model_name plumbing

Three discrete improvements to the MX side of PR NVIDIA#13045 driven by
review feedback from MX team's downstream PR
(chienchunhung/TensorRT-LLM #1) — three orchestration ergonomics fixes
landed as one focused commit so reviewers see them as a clean slice
on top of the prototype.

(1) MODEL_EXPRESS_URL env-var fallback — at validator level

  TorchLlmArgs.validate_mx_config now honors the upstream
  ``MODEL_EXPRESS_URL`` env var when ``checkpoint_format='MX'`` and
  ``mx_server_url`` is unset. Resolution happens at validator time so
  the value ends up on ``llm_args.mx_server_url`` (visible to
  logging, /startup_metrics, downstream code) instead of being
  silently re-read from env by the loader.

  Lets orchestrators (Dynamo) configure MX via the environment
  without plumbing every CLI knob, while keeping resolution in one
  place. Explicit ``mx_server_url=`` always wins. The env-var
  fallback only fires when MX is the active checkpoint format
  (so HF-only configs aren't surprised by an unrelated env var).
  Empty string in env is treated as unset.

(2) MX_SOURCE_QUERY_TIMEOUT defensive default

  MXCheckpointLoader.__init__ calls
  ``os.environ.setdefault("MX_SOURCE_QUERY_TIMEOUT", "30")`` whenever
  an MX server URL is configured. Caps cold-cluster first-replica
  startup at 30 s instead of upstream's 1-hour default (the polling
  in MxLiveWeightLoader._query_source). setdefault semantics preserve
  any explicit user value. HF-only loads (no MX URL) don't touch
  the env at all.

  The proper upstream-side fix is a non-blocking source-query API
  (tracked as MX-4 in §15 of the design doc); this defensive default
  caps the worst case until that lands.

(3) model_name plumbing with HF-snapshot-aware resolver

  Plumbs ``llm_args.model → MXCheckpointLoader(model_name=...)`` so
  upstream's ``publish_model_params()`` publishes under the
  user-supplied Hub ID (e.g. "Qwen/Qwen2.5-72B-Instruct") instead of
  the "unknown" sentinel.

  - MXCheckpointLoader takes a new optional ``model_name``
    constructor arg (Union[str, Path]). Coerced to str at
    construction time.
  - publish_as_source() now sets BOTH MODEL_EXPRESS_URL and
    MODEL_NAME env vars (resolving identity via the priority order
    below) and restores both env vars in finally.
    publish_model_params() reads them via env, as documented.
  - Identity resolution order: explicit constructor arg →
    MODEL_NAME env → checkpoint_dir basename (with HF-snapshot path
    unmangling) → "unknown".
  - HF cache layout (".../models--<org>--<name>/snapshots/<sha>/")
    is unmangled back to "<org>/<name>" instead of returning the
    commit hash.
  - _construct_checkpoint_loader plumbs ``mx_model_name`` through;
    py_executor_creator.py extracts it from llm_args.model.

  Both env-var dances (MODEL_EXPRESS_URL + MODEL_NAME) collapse into
  one direct call when MX-2 (public build_identity) lands upstream.

Tests for these three additions are in the next commit.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
Made-with: Cursor
chienchunhung added a commit that referenced this pull request Apr 21, 2026
… timeout, model_name plumbing

Three discrete improvements to the MX side of PR NVIDIA#13045 driven by
review feedback from MX team's downstream PR
(chienchunhung/TensorRT-LLM #1) — three orchestration ergonomics fixes
landed as one focused commit so reviewers see them as a clean slice
on top of the prototype.

(1) MODEL_EXPRESS_URL env-var fallback — at validator level

  TorchLlmArgs.validate_mx_config now honors the upstream
  ``MODEL_EXPRESS_URL`` env var when ``checkpoint_format='MX'`` and
  ``mx_server_url`` is unset. Resolution happens at validator time so
  the value ends up on ``llm_args.mx_server_url`` (visible to
  logging, /startup_metrics, downstream code) instead of being
  silently re-read from env by the loader.

  Lets orchestrators (Dynamo) configure MX via the environment
  without plumbing every CLI knob, while keeping resolution in one
  place. Explicit ``mx_server_url=`` always wins. The env-var
  fallback only fires when MX is the active checkpoint format
  (so HF-only configs aren't surprised by an unrelated env var).
  Empty string in env is treated as unset.

(2) MX_SOURCE_QUERY_TIMEOUT defensive default

  MXCheckpointLoader.__init__ calls
  ``os.environ.setdefault("MX_SOURCE_QUERY_TIMEOUT", "30")`` whenever
  an MX server URL is configured. Caps cold-cluster first-replica
  startup at 30 s instead of upstream's 1-hour default (the polling
  in MxLiveWeightLoader._query_source). setdefault semantics preserve
  any explicit user value. HF-only loads (no MX URL) don't touch
  the env at all.

  The proper upstream-side fix is a non-blocking source-query API
  (tracked as MX-4 in §15 of the design doc); this defensive default
  caps the worst case until that lands.

(3) model_name plumbing with HF-snapshot-aware resolver

  Plumbs ``llm_args.model → MXCheckpointLoader(model_name=...)`` so
  upstream's ``publish_model_params()`` publishes under the
  user-supplied Hub ID (e.g. "Qwen/Qwen2.5-72B-Instruct") instead of
  the "unknown" sentinel.

  - MXCheckpointLoader takes a new optional ``model_name``
    constructor arg (Union[str, Path]). Coerced to str at
    construction time.
  - publish_as_source() now sets BOTH MODEL_EXPRESS_URL and
    MODEL_NAME env vars (resolving identity via the priority order
    below) and restores both env vars in finally.
    publish_model_params() reads them via env, as documented.
  - Identity resolution order: explicit constructor arg →
    MODEL_NAME env → checkpoint_dir basename (with HF-snapshot path
    unmangling) → "unknown".
  - HF cache layout (".../models--<org>--<name>/snapshots/<sha>/")
    is unmangled back to "<org>/<name>" instead of returning the
    commit hash.
  - _construct_checkpoint_loader plumbs ``mx_model_name`` through;
    py_executor_creator.py extracts it from llm_args.model.

  Both env-var dances (MODEL_EXPRESS_URL + MODEL_NAME) collapse into
  one direct call when MX-2 (public build_identity) lands upstream.

Tests for these three additions are in the next commit.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
Made-with: Cursor
@chienchunhung
chienchunhung force-pushed the dynamo-integration-prototype branch 2 times, most recently from 0a2a950 to 6bdcfd8 Compare April 22, 2026 22:48
chienchunhung added a commit that referenced this pull request Apr 22, 2026
… timeout, model_name plumbing

Three discrete improvements to the MX side of PR NVIDIA#13045 driven by
review feedback from MX team's downstream PR
(chienchunhung/TensorRT-LLM #1) — three orchestration ergonomics fixes
landed as one focused commit so reviewers see them as a clean slice
on top of the prototype.

(1) MODEL_EXPRESS_URL env-var fallback — at validator level

  TorchLlmArgs.validate_mx_config now honors the upstream
  ``MODEL_EXPRESS_URL`` env var when ``checkpoint_format='MX'`` and
  ``mx_server_url`` is unset. Resolution happens at validator time so
  the value ends up on ``llm_args.mx_server_url`` (visible to
  logging, /startup_metrics, downstream code) instead of being
  silently re-read from env by the loader.

  Lets orchestrators (Dynamo) configure MX via the environment
  without plumbing every CLI knob, while keeping resolution in one
  place. Explicit ``mx_server_url=`` always wins. The env-var
  fallback only fires when MX is the active checkpoint format
  (so HF-only configs aren't surprised by an unrelated env var).
  Empty string in env is treated as unset.

(2) MX_SOURCE_QUERY_TIMEOUT defensive default

  MXCheckpointLoader.__init__ calls
  ``os.environ.setdefault("MX_SOURCE_QUERY_TIMEOUT", "30")`` whenever
  an MX server URL is configured. Caps cold-cluster first-replica
  startup at 30 s instead of upstream's 1-hour default (the polling
  in MxLiveWeightLoader._query_source). setdefault semantics preserve
  any explicit user value. HF-only loads (no MX URL) don't touch
  the env at all.

  The proper upstream-side fix is a non-blocking source-query API
  (tracked as MX-4 in §15 of the design doc); this defensive default
  caps the worst case until that lands.

(3) model_name plumbing with HF-snapshot-aware resolver

  Plumbs ``llm_args.model → MXCheckpointLoader(model_name=...)`` so
  upstream's ``publish_model_params()`` publishes under the
  user-supplied Hub ID (e.g. "Qwen/Qwen2.5-72B-Instruct") instead of
  the "unknown" sentinel.

  - MXCheckpointLoader takes a new optional ``model_name``
    constructor arg (Union[str, Path]). Coerced to str at
    construction time.
  - publish_as_source() now sets BOTH MODEL_EXPRESS_URL and
    MODEL_NAME env vars (resolving identity via the priority order
    below) and restores both env vars in finally.
    publish_model_params() reads them via env, as documented.
  - Identity resolution order: explicit constructor arg →
    MODEL_NAME env → checkpoint_dir basename (with HF-snapshot path
    unmangling) → "unknown".
  - HF cache layout (".../models--<org>--<name>/snapshots/<sha>/")
    is unmangled back to "<org>/<name>" instead of returning the
    commit hash.
  - _construct_checkpoint_loader plumbs ``mx_model_name`` through;
    py_executor_creator.py extracts it from llm_args.model.

  Both env-var dances (MODEL_EXPRESS_URL + MODEL_NAME) collapse into
  one direct call when MX-2 (public build_identity) lands upstream.

Tests for these three additions are in the next commit.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
Made-with: Cursor
chienchunhung added a commit that referenced this pull request Apr 22, 2026
… timeout, model_name plumbing

Three discrete improvements to the MX side of PR NVIDIA#13045 driven by
review feedback from MX team's downstream PR
(chienchunhung/TensorRT-LLM #1) — three orchestration ergonomics fixes
landed as one focused commit so reviewers see them as a clean slice
on top of the prototype.

(1) MODEL_EXPRESS_URL env-var fallback — at validator level

  TorchLlmArgs.validate_mx_config now honors the upstream
  ``MODEL_EXPRESS_URL`` env var when ``checkpoint_format='MX'`` and
  ``mx_server_url`` is unset. Resolution happens at validator time so
  the value ends up on ``llm_args.mx_server_url`` (visible to
  logging, /startup_metrics, downstream code) instead of being
  silently re-read from env by the loader.

  Lets orchestrators (Dynamo) configure MX via the environment
  without plumbing every CLI knob, while keeping resolution in one
  place. Explicit ``mx_server_url=`` always wins. The env-var
  fallback only fires when MX is the active checkpoint format
  (so HF-only configs aren't surprised by an unrelated env var).
  Empty string in env is treated as unset.

(2) MX_SOURCE_QUERY_TIMEOUT defensive default

  MXCheckpointLoader.__init__ calls
  ``os.environ.setdefault("MX_SOURCE_QUERY_TIMEOUT", "30")`` whenever
  an MX server URL is configured. Caps cold-cluster first-replica
  startup at 30 s instead of upstream's 1-hour default (the polling
  in MxLiveWeightLoader._query_source). setdefault semantics preserve
  any explicit user value. HF-only loads (no MX URL) don't touch
  the env at all.

  The proper upstream-side fix is a non-blocking source-query API
  (tracked as MX-4 in §15 of the design doc); this defensive default
  caps the worst case until that lands.

(3) model_name plumbing with HF-snapshot-aware resolver

  Plumbs ``llm_args.model → MXCheckpointLoader(model_name=...)`` so
  upstream's ``publish_model_params()`` publishes under the
  user-supplied Hub ID (e.g. "Qwen/Qwen2.5-72B-Instruct") instead of
  the "unknown" sentinel.

  - MXCheckpointLoader takes a new optional ``model_name``
    constructor arg (Union[str, Path]). Coerced to str at
    construction time.
  - publish_as_source() now sets BOTH MODEL_EXPRESS_URL and
    MODEL_NAME env vars (resolving identity via the priority order
    below) and restores both env vars in finally.
    publish_model_params() reads them via env, as documented.
  - Identity resolution order: explicit constructor arg →
    MODEL_NAME env → checkpoint_dir basename (with HF-snapshot path
    unmangling) → "unknown".
  - HF cache layout (".../models--<org>--<name>/snapshots/<sha>/")
    is unmangled back to "<org>/<name>" instead of returning the
    commit hash.
  - _construct_checkpoint_loader plumbs ``mx_model_name`` through;
    py_executor_creator.py extracts it from llm_args.model.

  Both env-var dances (MODEL_EXPRESS_URL + MODEL_NAME) collapse into
  one direct call when MX-2 (public build_identity) lands upstream.

Tests for these three additions are in the next commit.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
Made-with: Cursor
@chienchunhung
chienchunhung force-pushed the dynamo-integration-prototype branch from 6bdcfd8 to 06491b6 Compare April 22, 2026 23:54
TRT-LLM's custom logger.warning() doesn't accept exc_info.

Signed-off-by: Kavin Krishnan <kavink@nvidia.com>
Made-with: Cursor
chienchunhung added a commit that referenced this pull request Apr 30, 2026
…rmanent wedge

Document the multi-signature disaggregated-serving wedge surfaced by the
rc11 deployment. The report covers the 1P1D reproducer harness, the six
labelled failure signatures (sender-side broken-promise after ready,
trie cascade-prune assertion, decode-side bad optional access, gen-side
checkGenTransferStatus blocking on at_least_num=1, receiver-side queued
cancel broken-promise, and the suspected control-path send stall),
their mapping to chained test/fix PR pairs (NVIDIA#13571/NVIDIA#13572 for sig #2,
NVIDIA#13639/NVIDIA#13640 for sig #1), the in-flight fixes for sig #4 and sig #5,
and the relationship to the unrelated companion fixes NVIDIA#12718 and NVIDIA#13119
which are not in rc11. Includes an investigation timeline that explains
why each signature surfaced only after the previous one was fixed, and
a test-coverage analysis of why the existing unit and integration tests
did not catch any of these bugs.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
Made-with: Cursor
chienchunhung added a commit that referenced this pull request Apr 30, 2026
…wedge to NIXL/UCX

Document the run8 end-to-end validation result for the signature NVIDIA#6
fix and the resulting pivot in the wedge-driver picture:

- Signature NVIDIA#6 fix verified at the C++ trace level: 33/33 requests
  walk gen_send_assign_buffer_begin → step → end cleanly, and the new
  gen_request_sync_not_ready_buffers_freed marker fires on every
  cancelled-after-ready request (3/3 in run8).
- The harness still reports NO RECOVERY. Post-mortem py-spy and gdb
  stack dumps on the ctx-side mpi4py-spawned executor worker show the
  surviving wedge is one architectural layer below TRT-LLM: a NIXL
  UCX-internal pthread_mutex_lock deadlock inside
  nixlUcxThreadEngine::getNotifs() blocks
  CacheSender::recvRequestInfo() indefinitely.
- Cancel signature #7a as a real bug — drop_without_fulfill is a
  misnamed trace marker that fires immediately before the signature
  #1 cancellation handler that already fulfills the promise. The 3
  events per run are the fix path doing its job.
- Reframe signature #7b (14 stranded receiver-side futures) as a
  downstream symptom of either the NIXL/UCX deadlock or the
  C++ ↔ Python lifetime ownership debt called out in the
  Architectural Reflections section.
- Update the Executive Summary, Signature NVIDIA#6 status block, and Next
  Steps to reflect the NIXL/UCX layer as the actual terminal wedge
  driver, and add two follow-up items: enforce kv_transfer_timeout_ms
  as a hard deadline on the C++ blocking entry points, and file the
  NIXL/UCX bug with the run8 ctx-worker stack as the canonical
  reproducer.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
Made-with: Cursor
chienchunhung added a commit that referenced this pull request Apr 30, 2026
…DIA#7; reposition deadline as fallback

Restructure the report around a cleaner, more honest framing of the
final terminal wedge driver:

- Promote the NIXL UCX-internal pthread_mutex_lock deadlock (previously
  documented loosely as #7a / #7b in the run8 post-mortem) into a
  proper Signature NVIDIA#7 in the canonical signature list, with its own
  detailed section in Failure Signatures including the gdb stack
  evidence, mechanism, reproducer, and the explicit "this is NOT a
  TRT-LLM bug" framing.
- Retire #7a as a real bug (misnamed trace marker; counts match the
  signature #1 fix path firing correctly) and fold #7b's symptom into
  NVIDIA#7's mechanism description.
- Reposition the kv_transfer_timeout_ms deadline work (Next Steps
  item 7 + the Effort Estimate subsection) explicitly as a TRT-LLM-side
  fallback / mitigation for signature NVIDIA#7, NOT the ultimate fix. The
  ultimate fix is the NIXL/UCX root-cause bug (Next Steps item 8).
- Update the Executive Summary to say "six TRT-LLM bugs plus a seventh
  that lives one architectural layer below in NIXL/UCX" instead of
  "five distinct bugs plus a caveat".
- Add a row for NVIDIA#7 to the Signature ↔ PR Map: status "identified,
  classified, documented, not a TRT-LLM bug"; ultimate fix tracked
  via the NIXL/UCX bug; TRT-LLM-side fallback tracked via the
  deadline work.
- Update Phase 10 narrative and the Signature NVIDIA#6 status block to use
  the cleaner NVIDIA#7 framing.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
Made-with: Cursor
chienchunhung added a commit that referenced this pull request Apr 30, 2026
…13673/NVIDIA#13674 into the investigation report

The four sig #4 / #5 / NVIDIA#6 PRs are now open against NVIDIA/TensorRT-LLM:

- Sig #4: chained pair NVIDIA#13674 (test) -> NVIDIA#13671 (fix; carries 2
  commits including the test, both PRs target main so they can be
  merged in order)
- Sig #5: combined test + fix in NVIDIA#13672 (independent of the #1 chain)
- Sig NVIDIA#6: combined test + fix in NVIDIA#13673, chained on top of NVIDIA#13640
  (the #1 fix is a prerequisite for the !isReady early-return path)

Update the Signature <-> PR Map, the per-signature Status / PRs blocks,
and the Next Steps items 2 and 3 to reference these PR numbers and
mark the corresponding follow-up items as done.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
Made-with: Cursor
chienchunhung added a commit that referenced this pull request Apr 30, 2026
… section

Fold two related pieces of analysis into the report as a new section
between the Investigation Timeline and "Why the Existing Tests Did
Not Catch This":

(1) Signature taxonomy refining the naive "burst -> timeout ->
cancellation -> bug" framing. Four-of-seven signatures (#1, #3, #5, NVIDIA#6)
are direct cancellation-handling bugs; #4 is a structural latent
blocking bug that cancellations expose; #2 is an eviction-driven bug
that burst traffic exposes via memory pressure; NVIDIA#7 is a NIXL-internal
contention bug that the same load shape happens to trigger but which
is not strictly a cancellation bug. Includes a refined trigger chain
diagram and two precise corrections (burst alone is not the trigger;
"cancellation" is one of several entry points to cleanup paths).

(2) Cascade map distinguishing two kinds of inter-signature tangling:
- Type 1 (a fix produces a new signature): only one case, the #1 fix
  produces NVIDIA#6 by making the receiver-side !isReady early-return path
  reachable in production where a latent recv-buffer leak existed.
  This is why NVIDIA#6 PR (NVIDIA#13673) is explicitly chained on #1 fix PR
  (NVIDIA#13640).
- Type 2 (a fix exposes a pre-existing signature): three cases where
  #4 fix exposes #5 / NVIDIA#6 and NVIDIA#6 fix exposes NVIDIA#7 because the upstream
  fix removes the masking effect on the downstream bug. These are not
  regressions of the fixes; they were latent pre-existing issues.
- Subtler third relationship: #4 is structurally a defensive catcher
  for any upstream bug that produces a never-resolving receiver
  future. The #4 fix is independently valuable as defence in depth,
  not just a symptomatic patch.

Also includes a fix-to-file mapping showing that the fixes do not
overlap in code; the only structural dependency is the NVIDIA#6 -> #1 chain
enforced by the PR base.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
Made-with: Cursor
chienchunhung added a commit that referenced this pull request Apr 30, 2026
…cess retrospective

Add a new section between "Why the Existing Tests Did Not Catch This"
and "Architectural Reflections" that answers the natural follow-up
question after the cascade map: with the e2e view in hand, what would
the cleaner approach have been from the start?

Two halves:

(1) What was not actually possible at T0 — explains why a "design one
comprehensive fix" approach (e.g. a TransferSession-like abstraction
introduced in PR #0) was strictly impossible given the information
state at T0: you cannot design an abstraction to fix bugs you have not
found yet, and the field was wedged under P0 urgency that did not
allow a multi-thousand-line refactor of a critical path.

(2) What we should have done first, in priority order, to change the
meta-process rather than the fix:
  - PR #0: deadline enforcement (Next Steps item 7) as a containment
    layer. Converts every cleanup-path bug from a silent wedge into a
    per-request error, gives every subsequent bug an attributable
    failure point, gives orchestration a real signal so customer
    deployments self-heal via pod restart.
  - Write down the seven invariants from the Architectural Reflections
    section. Would have caught #5 and NVIDIA#6 at #1's PR review (Type 1
    cascade prevented at design time) and made #4 visible to any
    code search.
  - Add the cancel-during-transfer integration test for the customer
    load shape. Would have surfaced all six TRT-LLM signatures as CI
    test failures instead of as a customer field hit.

Plus a clarifying "What this section is not arguing" subsection to
prevent over-reading: not advocating for a TransferSession rewrite as
PR #0, not claiming one PR could fix all seven signatures, and not
claiming the sequential discovery was avoidable given the
meta-process we actually had at T0.

The key insight is that the bottleneck of the investigation was
observability and attribution, not fix complexity. The three
meta-process changes attack that bottleneck directly.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
Made-with: Cursor
chienchunhung added a commit that referenced this pull request May 1, 2026
NVIDIA#7 confirmed independent of TRT-LLM-side fix strategy

Add Phase 11 to the Investigation Timeline documenting the
pr13056_run1 experiment: ran the same 1P1D long-prompt burst harness
against an independent fix stack (a comprehensive end-to-end
shared_ptr<LlmRequest> + BufferIndexHolder RAII + deadline-enforcement
refactor) on the same rc11 base. Same outcome as run8: NO RECOVERY
after 180s idle, with a gdb-confirmed pthread_mutex_lock frame in
CacheSender::Impl::response on the ctx worker, alongside the same
NIXL plugin threads in the same process.

Coverage map shows the two stacks converge on the same TRT-LLM-side
bugs (#1, #4, #5, NVIDIA#6) via different mechanisms (surgical patches vs
comprehensive refactor), eliminating two alternative hypotheses:
- our chained PRs introduced a regression that masquerades as NVIDIA#7
- comprehensive deadline enforcement alone clears the field reproducer

Both refuted by the experiment. The independent stack's defensive
diagnostics ([buf] CANCEL: 0, [buf] STILL_WAITING: 0, kNETWORK_ERROR:
0, broken-promise: 0, deadline-driven failures: 0) confirm the wedge
is below where any TRT-LLM-side deadline can reach.

Update Next Steps item 8 to reference the pr13056_run1 stack dump as
a second independent reproducer for the NIXL/UCX bug filing — much
stronger evidence than run8 alone because it shows the deadlock is
independent of any TRT-LLM fix strategy.

Update Phase 10 title to remove the (current) qualifier since
Phase 11 is now the latest phase.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
Made-with: Cursor
chienchunhung added a commit that referenced this pull request May 1, 2026
…ifestations broaden sig NVIDIA#7 to a CacheSender::Impl::* bug class

run9 (rc11 + our fixes + UCX) and run10 (PR NVIDIA#13056 + UCX) both used a
gdb capture loop on the dataTransResp thread to pin down the exact mutex
behind sig NVIDIA#7. The wedge changed character in both runs:

- run9 ctx mpi worker SIGSEGVs at iter 92 of the burst inside
  _PyObject_GenericGetAttrWithDict (Python C-API), downstream of the
  sig #1 fix path firing cleanly.
- run10 ctx mpi worker SIGSEGVs synchronously inside
  CacheSender::Impl::handleAsyncSend(AsyncSendResource&) on the very
  first sanity-probe request - no concurrency, no burst, no
  cancellation. Zero UCX/NIXL frames in the stack; root cause is most
  plausibly a null shared_ptr<LlmRequest> deref at line 594 since
  PR NVIDIA#13056's ownership model is necessary but does not enforce
  Response::mRequest non-null at producer or consumer.

Both findings are cleanly TRT-LLM-internal and broaden sig NVIDIA#7 from a
single pthread_mutex_lock deadlock to a class of CacheSender::Impl::*
bugs with at least four observed manifestations across two transports
and three fix bundles.

Touched sections: Status block, caveat block, signature table (sig NVIDIA#7
row), sig NVIDIA#7 section variants/fix/status, Phase 12 cross-reference,
new Phase 13 section, Next Steps items 7 / 7a / 7b.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
Made-with: Cursor
chienchunhung added a commit that referenced this pull request May 3, 2026
…le structure

Split the 3193-line single-file report into 13 reader-friendly files
under docs/investigations/nvbug-6104831-disagg-permanent-wedge/:

- README.md: top-level orientation, status, navigation, suggested
  reading paths.
- 01-background.md: architecture diagrams, request lifecycle
  walkthrough, state machine, cancellation flow.
- 02-failure-signatures.md: the seven failure signatures (#1-NVIDIA#7).
- 03-defect-class-stack.md: NEW - the L1-L8 defect-class layering
  that frames the four-approach comparison.
- 04-reproduction.md: how to reproduce locally, load shape, run
  archive index.
- 05-investigation-timeline.md: chronological story (Phases 0-14).
- 06-fix-approaches/: side-by-side comparison plus one file per
  approach (A chained, B PR NVIDIA#13056, C PR NVIDIA#13495, D combo).
- 07-architectural-reflections.md: seven invariants + retrospective.
- 08-next-steps-and-pr-map.md: PR map, outstanding work, deadline
  effort estimate, run archive index.

The L1-L8 defect class stack is the new framework introduced here.
It re-frames the seven signatures as the visible faces of eight
underlying invariant gaps and is used to explain why the combo
approach (D) is the only stack that recovers cleanly across the
test matrix. The five Mermaid diagrams from the request walkthrough
move into 01-background.md as the prerequisite reading for the rest
of the investigation.

The single-file form was hard to navigate at this size; the split
follows the user's reading-path needs (cold reader, single-PR
reviewer, fix-path picker, retrospective reader) called out in the
README.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
chienchunhung added a commit that referenced this pull request May 3, 2026
… colored fix components

Two improvements to 00-tldr.md based on review feedback:

1. Add a 7-row table after the architecture section that gives a
   one-line "where it lives" + "symptom" for each signature #1-NVIDIA#7.
   The TL;DR previously mentioned the seven signatures by number
   without describing them, forcing the reader to jump to
   02-failure-signatures.md to make sense of references like
   "sig #4" or "sig NVIDIA#7".

2. Color-code the four fix components in the combo diagram:
   - PR NVIDIA#13056: blue (lifetime + cancel-flag + RAII)
   - PR NVIDIA#13495: orange (NIXL TransferStatus::release)
   - eval-order fix: green
   - Python idempotency guards: purple

   Each L1-L8 layer node is tinted with the lighter shade of
   whichever fix closes it, so the "closes" arrows are reinforced
   by colour-matching. Makes the fix-to-layer mapping visually
   scannable at a glance.

Note added underneath the diagram explaining the colour scheme.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
chienchunhung added a commit that referenced this pull request May 20, 2026
…view

Apply 5 review-driven edits to §16 to address residual concerns:

Walk ordering (concern NVIDIA#8): change GMS RO and MX-receiver alias walks
from per-module to top-level model.setup_aliases(). Matches the §7
mitigation contract from ai-dynamo/dynamo PR NVIDIA#7053 ("Call
model.post_load_weights() (top-level only) before
materialize_module_from_gms()"). transform_weights() and
cache_derived_state() walks remain per-module since those bodies live
on submodules. New "Why setup_aliases() is top-level-only" callout
documents the asymmetry.

Lifecycle of _weights_transformed (concern #4): new subsection
specifying explicit set/reset/orthogonality rules. Includes a 2x2
truth table showing _weights_removed and _weights_transformed track
different lifecycles and can take any combination. Reset is the
orchestrator's responsibility (e.g., ModelLoader.reload() resets the
flag before re-binding tensors); subclasses do not manage reset.

Hard preconditions (concern #5): promote MX source-identity
completeness from "open question" to "hard precondition P1." Lists
transform-affecting parameters that MX identity must cover
(attn_backend, quant backend list, FP8/NVFP4 fusion strategy, TP/EP
layout, model revision). Specifies an in-tree backend-fingerprint
fail-safe as the fallback if upstream MX cannot guarantee
completeness. P2 documents that orchestrator owns _weights_transformed
reset. Removes redundant open question NVIDIA#6 from the table.

Cosmetic fixes (concerns #2, NVIDIA#6): "four stages" -> "three per-module
stages plus orchestrator-managed per-process finalization."
cache_derived_state description softened to "reserved for
data-dependent state where it exists; many existing modules will have
empty bodies."

Scope clarifications (concerns #1, #3, NVIDIA#7):
- Tiny PR scope reframed as "duck-typed helpers, not inheritance"
  with citations to existing model_loader.py walker pattern. Lists
  4 walker helpers (_setup_aliases, _walk_transform, _walk_cache_state,
  _walk_full_post_load).
- Migration callout: when migrating a subclass, the old
  post_load_weights() override MUST be removed; otherwise the new
  staged calls silently no-op.
- Family PR #2 (Linear/Attention) gains a "quant-method callback
  decision" note with default = keep quant_method.post_load_weights
  callback name (no rename).

No code changes. Drives Tiny prep PR scope and family-PR migration
sequence. References:
- TRT-LLM PR NVIDIA#13926 (GMS-only)
- TRT-LLM PR NVIDIA#14151 (MX shim refactor)
- ai-dynamo/dynamo PR NVIDIA#7053 (upstream GMS prototype)

Signed-off-by: Chien-Chun Hung <chienchunh@nvidia.com>
Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
chienchunhung added a commit that referenced this pull request Jun 8, 2026
… cancel-poison design

Adds a new investigation doc 19 capturing the external forensic A/B
(fengyul/dynamo-disagg exp 4) that decomposes the decode-side wedge
into three independent failures: F1 (Broken promise UAF), F2 (engine-
loop freeze on unbounded future.get()), and F3 (eager-free poisons UCX
progress thread → permanent wedge). Maps F1/F2/F3 onto the existing
layer model (L1, L3, L4+L5) and signatures (#1, #4), and onto the
design doc's C4 invariant.

Strategic conclusions surfaced from the experiment:

* PR NVIDIA#14979's shared_ptr lifetime port closes F1 (a co-occurring crash
  class) but does not recover the field wedge on NIXL by itself.
* Bounded polling alone (F2 fix) does not progress a stuck transfer on
  NIXL because UCX runs its own background progress thread —
  engine-freeze and transfer-stall are separate concerns.
* The load-bearing fix is F3 done safely = quiescence-gated freeing,
  which is structurally tangled across py_executor.py and the transfer
  manager API and not cleanly cherry-pickable. The deployable that
  passes the reproducer is PR NVIDIA#13713 in full.

Updates to investigation README:
* Adds doc 19 to the 'How to read' index.
* Adds a 'Can we ship a small subset of NVIDIA#13713?' suggested reading path.

Updates to disagg-inflight-cancel-poison design README:
* Adds the exp 4 evidence to 'Empirical evidence motivating this work'.
* Adds a 'load-bearing for recovery, not optional polish' note on
  Phase 2 (deferred un-poison) — the polling-until-quiescence mechanism
  realizes C4 on the V1 + C++ path and is direct empirical answer to
  the field wedge, not just operability improvement on top of the
  existing fail-closed surface.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
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.

2 participants