Skip to content
Merged
1 change: 1 addition & 0 deletions vllm/executor/ray_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from vllm.utils import get_ip
from vllm.worker.worker_base import WorkerWrapperBase


if TYPE_CHECKING:
from vllm.v1.core.sched.output import SchedulerOutput
from vllm.v1.outputs import ModelRunnerOutput
Expand Down
118 changes: 110 additions & 8 deletions vllm/v1/executor/ray_distributed_executor.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
# SPDX-License-Identifier: Apache-2.0
# SPDX-FileCopyrightText: Copyright contributors to the vLLM project

from concurrent.futures import Future
from typing import Union
from collections import defaultdict
from concurrent.futures import CancelledError, Future
from typing import Optional, Sequence, Union, cast

from vllm.executor.ray_distributed_executor import ( # noqa
RayDistributedExecutor as RayDistributedExecutorV0)
from vllm.v1.executor.abstract import Executor
from vllm.v1.outputs import ModelRunnerOutput
from vllm.logger import init_logger, logger

init_logger("vllm")

class FutureWrapper(Future):
"""A wrapper around a Ray output reference to meet the interface
Expand All @@ -28,6 +31,19 @@ def result(self, timeout=None):
class RayDistributedExecutor(RayDistributedExecutorV0, Executor):
"""Ray distributed executor using Ray Compiled Graphs."""

def _init_executor(self) -> None:
super()._init_executor()

# KV connector setup
self.has_connector = self.vllm_config.kv_transfer_config is not None

# Complete transfer tracker. Used by to track finished requests
# [req_id -> n_finished_workers]
self._recv_remaining_count = defaultdict[str,
int](lambda: self.parallel_config.world_size)
self._send_remaining_count = defaultdict[str,
int](lambda: self.parallel_config.world_size)

@property
def max_concurrent_batches(self) -> int:
"""Ray distributed executor supports pipeline parallelism,
Expand All @@ -37,6 +53,80 @@ def max_concurrent_batches(self) -> int:
return 2
return self.parallel_config.pipeline_parallel_size

def _aggregate_workers_output(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we move these to utility functions (shared with multiproc executor), or maybe a mixin superclass also containing the _send_remaining_count and _recv_remaining_count fields?

self, outputs: list[ModelRunnerOutput]) -> ModelRunnerOutput:
# aggregate finished_sending, finished_recving from all workers

finished_sending = set[str]()
finished_recving = set[str]()
for output in outputs:
# update finished_sending
for req_id in output.finished_sending or []:
new_count = self._send_remaining_count[req_id] - 1
if new_count == 0:
# got response from all workers, report back to scheduler
finished_sending.add(req_id)
del self._send_remaining_count[req_id]
else:
self._send_remaining_count[req_id] = new_count

# update finished_recving
for req_id in output.finished_recving or []:
new_count = self._recv_remaining_count[req_id] - 1
if new_count == 0:
# got response from all workers, report back to scheduler
finished_recving.add(req_id)
del self._recv_remaining_count[req_id]
else:
self._recv_remaining_count[req_id] = new_count

# select output of the worker specified by output_rank
output = outputs[0]

# set the aggregated finished_sending / finished_recving
if finished_sending:
output.finished_sending = finished_sending
if finished_recving:
output.finished_recving = finished_recving

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
if finished_sending:
output.finished_sending = finished_sending
if finished_recving:
output.finished_recving = finished_recving
if finished_sending:
output.finished_sending = finished_sending
if finished_recving:
output.finished_recving = finished_recving

See also #21048 which has a fix for this that we should get in.


return output

def _async_aggregate_workers_output(
self, output_futures: Sequence[Union[Future[ModelRunnerOutput], FutureWrapper]]
) -> Future[ModelRunnerOutput]:
"""Takes a list of futures and returns a single future which resolves
to the respective list of outputs."""
result_future: Future[ModelRunnerOutput] = Future()

outputs: list[Optional[ModelRunnerOutput]] = [None
] * len(output_futures)

def make_callback(idx):

def callback(fut):
if result_future.done():
return

try:
outputs[idx] = fut.result()
except CancelledError:
result_future.cancel()
except Exception as e:
result_future.set_exception(e)

# Check if all outputs are ready
if all(outputs):
result_future.set_result(
self._aggregate_workers_output(
cast(list[ModelRunnerOutput], outputs)))

return callback

for i, output_future in enumerate(output_futures):
output_future.add_done_callback(make_callback(i))

return result_future

def execute_model(
self,
scheduler_output,
Expand All @@ -55,10 +145,22 @@ def execute_model(

refs = self.forward_dag.execute(scheduler_output) # type: ignore

# When PP is not used, we block here until the result is available.
if self.max_concurrent_batches == 1:
return refs[0].get()
if not self.has_connector:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the changes below here in this file should be all that's needed in this PR (apart from moving those other two functions so that they can be shared).

# get output only from a single worker (output_rank)
# When PP is not used, we block here until the result is available.
if self.max_concurrent_batches == 1:
return refs[0].get()

# When PP is used, we return a FutureWrapper immediately so that
# the scheduler can yield to the next batch.
return FutureWrapper(refs[0])
# When PP is used, we return a FutureWrapper immediately so that
# the scheduler can yield to the next batch.
return FutureWrapper(refs[0])

# get output from all workers when connector is present
if self.max_concurrent_batches == 1:
# Block and get results from all workers
outputs = [ref.get() for ref in refs]
return self._aggregate_workers_output(outputs)
else:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

redundant else

# Return a future that will aggregate outputs from all workers
output_futures = [FutureWrapper(ref) for ref in refs]
return self._async_aggregate_workers_output(output_futures)
44 changes: 38 additions & 6 deletions vllm/v1/worker/gpu_model_runner.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: Apache-2.0
# SPDX-FileCopyrightText: Copyright contributors to the vLLM project

import copy
import gc
import time
import weakref
Expand Down Expand Up @@ -1233,6 +1234,8 @@ def _pool(
hidden_states: torch.Tensor,
num_scheduled_tokens: int,
num_scheduled_tokens_np: np.ndarray,
finished_sending: Optional[set[str]],

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why are all of these changes to gpu_model_runner.py and gpu_worker.py needed? I don't think they affect what's being fixed by this PR and so would be best to revert (unless I'm missing something).

@kouroshHakha kouroshHakha Jul 16, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

so ray distributed executor runs the runner directly (not gpu_worker), it's better for the model_runner to own the logic of filling in the finished_sending and finished_recving fields in the output. So I basically reverted the changes to gpu_model_runner introduced in #19555. In that PR, for some reason that I don't understand, the logic is taken out of the model_runner and is put into the worker.

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.

This change was made to support PP.
Notice these lines in GPUModelRunner::execute_model:

        if not get_pp_group().is_last_rank:
            # For mid-pipeline stages, return the hidden states.
            if not broadcast_pp_output:
                return hidden_states

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

so ray distributed executor runs the runner directly

@kouroshHakha could you point to where this is the case? From a quick look it appears that it's still a bit entangled with V0 logic but ultimately uses a RayWorkerWrapper which wraps a WorkerBase which should in this case resolve to a vllm.v1.worker.gpu_worker.Worker.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

So here is the flow with Ray as the distributed backend in v1:

v1.RayDistributedExecutor.execute_model() --> RayWrapper.execute_model_ray() (or RayWrapper.execute_model_spmd()) --> worker.model_runner.execute_model()

Therefore the worker.execute_model() logic is skipped and it directly interacts with model_runner.execute_model().

@kouroshHakha kouroshHakha Jul 17, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@orozery Is there a test / script that I should target to make sure the intended pp logic still works with my changes?

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.

FWIW, there should be no regression for test_pipeline_parallel.py , not sure if there is a test for PP + P/D

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@orozery I see what you are saying. Basically for PP, each pp rank (except the last one) will forward an empty output back to the scheduler. You want those empty outputs to have the correct finished/recved attributes. I added that logic back but leaving the main model_runner logic intact. Let's get this merged asap so that it can catch the 0.10.0 train since it's a massive regression in the ray behavior. We can follow up with better solutions after.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

not sure if there is a test for PP + P/D

Based on my research there is none. I also couldn't get the master run PP + P/D so not sure if at any point that combo was supported and now it's regressed? Or was it not supported at all.

finished_recving: Optional[set[str]],
) -> ModelRunnerOutput:
assert self.input_batch.num_reqs ==\
len(self.input_batch.pooling_params), \
Expand Down Expand Up @@ -1267,6 +1270,8 @@ def _pool(
logprobs=None,
prompt_logprobs_dict={},
pooler_output=pooler_output,
finished_sending=finished_sending,
finished_recving=finished_recving,
)

@torch.inference_mode()
Expand All @@ -1277,12 +1282,12 @@ def execute_model(
) -> Union[ModelRunnerOutput, IntermediateTensors]:
self._update_states(scheduler_output)
if not scheduler_output.total_num_scheduled_tokens:
if has_kv_transfer_group():
with set_forward_context(None, self.vllm_config):
self.maybe_setup_kv_connector(scheduler_output)
if not has_kv_transfer_group():
# Return empty ModelRunnerOutput if there's no work to do.
return EMPTY_MODEL_RUNNER_OUTPUT

return self.kv_connector_no_forward(scheduler_output)

# Return empty ModelRunnerOutput if there's no work to do.
return EMPTY_MODEL_RUNNER_OUTPUT

# Prepare the decoder inputs.
(attn_metadata, attention_cuda_graphs, logits_indices,
Expand Down Expand Up @@ -1375,6 +1380,8 @@ def execute_model(
)

self.maybe_wait_for_kv_save()
finished_sending, finished_recving = (
self.get_finished_kv_transfers(scheduler_output))

if self.use_aux_hidden_state_outputs:
hidden_states, aux_hidden_states = model_output
Expand All @@ -1400,7 +1407,7 @@ def execute_model(
else:
if self.input_batch.pooling_params:
return self._pool(hidden_states, num_scheduled_tokens,
num_scheduled_tokens_np)
num_scheduled_tokens_np, finished_sending, finished_recving)

sample_hidden_states = hidden_states[logits_indices]
logits = self.model.compute_logits(sample_hidden_states, None)
Expand Down Expand Up @@ -1694,6 +1701,31 @@ def maybe_wait_for_kv_save() -> None:
if has_kv_transfer_group():
get_kv_transfer_group().wait_for_save()

@staticmethod
def get_finished_kv_transfers(
scheduler_output: "SchedulerOutput",
) -> tuple[Optional[set[str]], Optional[set[str]]]:
if has_kv_transfer_group():
return get_kv_transfer_group().get_finished(
scheduler_output.finished_req_ids)
return None, None

def kv_connector_no_forward(
self, scheduler_output: "SchedulerOutput") -> ModelRunnerOutput:
# KV send/recv even if no work to do.
with set_forward_context(None, self.vllm_config):
self.maybe_setup_kv_connector(scheduler_output)
finished_sending, finished_recving = (
self.get_finished_kv_transfers(scheduler_output))

if not finished_sending and not finished_recving:
return EMPTY_MODEL_RUNNER_OUTPUT

output = copy.copy(EMPTY_MODEL_RUNNER_OUTPUT)
output.finished_sending = finished_sending
output.finished_recving = finished_recving
return output

def propose_ngram_draft_token_ids(
self,
sampled_token_ids: list[list[int]],
Expand Down
16 changes: 0 additions & 16 deletions vllm/v1/worker/gpu_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -332,22 +332,6 @@ def execute_model(
output = EMPTY_MODEL_RUNNER_OUTPUT

assert isinstance(output, ModelRunnerOutput)
if has_kv_transfer_group():

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The changes in gpu_model_runner.py and gpu_worker.py are revert of what was done in #19555

finished_sending, finished_recving = (
get_kv_transfer_group().get_finished(
scheduler_output.finished_req_ids))
if finished_sending or finished_recving:
if output is EMPTY_MODEL_RUNNER_OUTPUT:
output = copy.copy(EMPTY_MODEL_RUNNER_OUTPUT)
output.finished_sending = finished_sending
output.finished_recving = finished_recving

# Clear KVConnector state for this step.
get_kv_transfer_group().clear_connector_metadata()

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.

Why was clear_connector_metadata removed here?


# with a connector, the scheduler expects output from all workers
return output

# return output only from the driver worker
return output if self.is_driver_worker else None

Expand Down
Loading