[None][fix] Fix RPCStreamingError ('LlmResponse' object is not iterable) with num_postprocess_workers > 0 - #17469
Conversation
Signed-off-by: Shiki Wu <shikiw@nvidia.com>
|
/bot run |
Walkthrough
ChangesResponse queue handling
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tensorrt_llm/executor/rpc_worker_mixin.py (1)
76-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for bare
LlmResponseentries.The existing tests cover batched responses, but this change adds support for a bare response. Enqueue one
LlmResponseand verify thatfetch_responses()returns a one-element list. Retain a batch case to cover both queue shapes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tensorrt_llm/executor/rpc_worker_mixin.py` around lines 76 - 85, Add regression coverage for the response-queue handling near fetch_responses: retain the existing batched-response test and add a case that enqueues one bare LlmResponse, then verifies fetch_responses() returns a one-element list containing it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tensorrt_llm/executor/rpc_worker_mixin.py`:
- Around line 76-85: Add regression coverage for the response-queue handling
near fetch_responses: retain the existing batched-response test and add a case
that enqueues one bare LlmResponse, then verifies fetch_responses() returns a
one-element list containing it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 438d5035-f740-47ef-b02c-7975168bbcb1
📒 Files selected for processing (1)
tensorrt_llm/executor/rpc_worker_mixin.py
|
PR_Github #65010 [ run ] triggered by Bot. Commit: |
|
PR_Github #65010 [ run ] completed with state
|
|
/bot run --stage-list "DGX_B200-PyTorch-5" |
|
PR_Github #65026 [ run ] triggered by Bot. Commit: |
|
PR_Github #65026 [ run ] completed with state |
Dev Engineer Review
RpcWorker.fetch_responsesto accept both individualLlmResponseobjects and batched responses.extend().RPCStreamingError: 'LlmResponse' object is not iterablefailure when postprocess workers are enabled.QA Engineer Review
No test changes.
Description
With
num_postprocess_workers > 0, generation crashes at start withRPCStreamingError: 'LlmResponse' object is not iterable: the classic_send_rsppath enqueues a bare response (worker.result_queue.put(response)), butRpcWorker.fetch_responsesassumes every queue item is a batch and callsextend()on it. Accept both shapes on the consumer side.Test Coverage
Reproduced deterministically with
num_postprocess_workers=4(RPC/Ray orchestrator); no longer crashes with this patch. Existing executor unit tests cover the batched path.PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
If PR introduces API changes, an appropriate PR label is added - either
api-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin the PR title.Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.