Skip to content

[https://nvbugs/6464991][fix] Drop the isinstance check and gate the register() call only on len(status)==3…#16489

Closed
trtllm-agent wants to merge 1 commit into
NVIDIA:mainfrom
tensorrt-cicd:repair-bot-bug6464991
Closed

[https://nvbugs/6464991][fix] Drop the isinstance check and gate the register() call only on len(status)==3…#16489
trtllm-agent wants to merge 1 commit into
NVIDIA:mainfrom
tensorrt-cicd:repair-bot-bug6464991

Conversation

@trtllm-agent

@trtllm-agent trtllm-agent commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Root cause: session-reuse hook monkey-patches proxy.MpiPoolSession to a function, so isinstance(self.mpi_session, MpiPoolSession) raises TypeError on the second and later tests sharing the worker.
  • Fix: Drop the isinstance check and gate the register() call only on len(status)==3; keep the MpiPoolSession import (still used at construction).
  • Automated fix generated by repair-bot

Test plan

  • Verify fix on the same GPU type as the original failure
  • Check for regressions in related tests

Links

Summary by CodeRabbit

  • Bug Fixes
    • Improved worker initialization monitoring across supported MPI session types.
    • Worker process identities are now registered consistently when initialization status data is available.

… proxy

The isinstance(self.mpi_session, MpiPoolSession) check in
_start_executor_workers is fragile: session-reuse test infrastructure
(tests/test_common/session_reuse.py) monkey-patches the module-level
tensorrt_llm.executor.proxy.MpiPoolSession symbol with a plain function,
which makes isinstance() raise "TypeError: isinstance() arg 2 must be a
type". It also fails to recognize the _ReusableSession wrapper handed out
by the reuse cache.

The check gates WorkerProcessMonitor.register(), which already
self-filters non-local identities by hostname/pid_namespace, so it is
safe to invoke for any session type. Rely on len(status) == 3 alone as
the authoritative signal that the worker sent identities.

Signed-off-by: trtllm-agent <296075020+trtllm-agent@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: da2d2716-5dd7-48af-8984-25cf4842cfc5

📥 Commits

Reviewing files that changed from the base of the PR and between 274043a and dbde8f2.

📒 Files selected for processing (1)
  • tensorrt_llm/executor/proxy.py

📝 Walkthrough

Walkthrough

The worker initialization logic now registers worker_process_identities whenever the status payload has three elements, without requiring an MpiPoolSession.

Changes

Worker identity registration

Layer / File(s) Summary
Status payload identity handling
tensorrt_llm/executor/proxy.py
_start_executor_workers now processes and registers worker process identities when status has length 3, regardless of MPI session type.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

Suggested reviewers: junyixu-nv, zhaoyangwang-nvidia

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the bug fix and follows the required ticket/type pattern.
Description check ✅ Passed The description includes a summary, test plan, and bug link, which mostly matches the template.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@dc3671

dc3671 commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

close for it's just fixed in main #16444

@dc3671 dc3671 closed this Jul 16, 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.

2 participants