Skip to content

Fix ray mp duplicate output - #4833

Open
RunningLeon wants to merge 2 commits into
InternLM:mainfrom
RunningLeon:fix-ray-mp-dup
Open

Fix ray mp duplicate output#4833
RunningLeon wants to merge 2 commits into
InternLM:mainfrom
RunningLeon:fix-ray-mp-dup

Conversation

@RunningLeon

Copy link
Copy Markdown
Collaborator

Motivation

Fix ray mp duplicate output

Modification

Please briefly describe what modification is made in this PR.

BC-breaking (Optional)

Does the modification introduce changes that break the backward-compatibility of the downstream repositories?
If so, please describe how it breaks the compatibility and how the downstream projects should modify their code to keep compatibility with this PR.

Use cases (Optional)

If this PR introduces a new feature, it is better to list some use cases here, and update the documentation.

Checklist

  1. Pre-commit or other linting tools are used to fix the potential lint issues.
  2. The modification is covered by complete unit tests. If not, please add more unit tests to ensure the correctness.
  3. If the modification has a dependency on downstream projects of a newer version, this PR should be tested with all supported versions of downstream projects.
  4. The documentation has been modified accordingly, like docstring or example tutorials.

Copilot AI lite review requested due to automatic review settings August 6, 2026 02:51

Copilot AI 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.

Pull request overview

Fixes a Ray MP streaming edge case where the final EngineOutput could be yielded twice (once when produced, and again when the stream task terminates), resulting in duplicated final token output to callers.

Changes:

  • Track whether the stream produced any output and avoid replaying an already-consumed terminal EngineOutput when the remote async generator finishes.
  • Clear the per-stream pending slot after a consumer reads an item so the stream task can detect “already consumed” terminal output.
  • Add unit tests covering: (1) no duplicate final output after cleanup delay, (2) final output returned once when unconsumed, (3) failure-before-first-yield returns INTERNAL_ENGINE_ERROR.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/pytorch/engine/test_ray_mp_engine.py Adds regression tests to ensure Ray MP streaming does not duplicate the final output and preserves error behavior.
lmdeploy/pytorch/engine/mp_engine/ray_engine.py Adjusts stream task finalization and consumption tracking to prevent repeating already-consumed terminal outputs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@lvhan028 lvhan028 added the Bug:P0 label Aug 6, 2026
init_event.set()
self._stream_aiter[stream_id][1] = (result, True)
stream_out = self._stream_aiter[stream_id]
pending = stream_out[1]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible that stream_out[1] is out of index?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to find a way to deal with these magic index in the future. Another example is the following terminal_result = pending[0] cc @grimoire

@lvhan028
lvhan028 requested a review from grimoire August 6, 2026 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants