Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions slime/backends/vllm_utils/arguments.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,11 +234,21 @@ def add_vllm_arguments(parser):
"true determinism — seed alone does not control kernel selection."
),
)
# Timeout for trainer->vLLM-engine control-plane HTTP requests. Pairs with
# --router-request-timeout-secs (requests to the router). The old name
# --vllm-weight-transfer-timeout-sec is kept as a deprecated alias for back-compat
# (it is already a merged flag); both map to dest ``vllm_engine_request_timeout_secs``.
parser.add_argument(
"--vllm-engine-request-timeout-secs",
"--vllm-weight-transfer-timeout-sec",
dest="vllm_engine_request_timeout_secs",
type=float,
default=900.0,
help="Timeout (seconds) for vLLM weight-transfer HTTP control-plane calls.",
help=(
"Timeout (seconds) for trainer->vLLM-engine control-plane HTTP requests "
"(weight-transfer init/update, /sleep, /wake_up). "
"Alias --vllm-weight-transfer-timeout-sec is deprecated."
),
)
# vime-only orchestration knob: not part of vllm's CLI but read by
# UpdateWeightFromDistributed._use_vllm_packed() to choose packed
Expand Down Expand Up @@ -368,7 +378,7 @@ def vllm_parse_args():
"router_policy",
"vllm_server_concurrency",
"vllm_enable_deterministic_inference",
"vllm_weight_transfer_timeout_sec",
"vllm_engine_request_timeout_secs",
"vllm_weight_sync_packed",
# vime-only flags for fine-grained deployment; consumed in slime/ray/rollout.py
# (start_rollout_servers / _resolve_vllm_config) and must NOT be forwarded to
Expand Down
21 changes: 12 additions & 9 deletions slime/backends/vllm_utils/vllm_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -603,8 +603,8 @@ def __init__(
def _http_base(self) -> str:
return f"http://{self.server_host}:{self.server_port}"

def _weight_transfer_http_timeout(self) -> float:
return float(self.args.vllm_weight_transfer_timeout_sec)
def _engine_request_http_timeout(self) -> float:
return float(self.args.vllm_engine_request_timeout_secs)

def init(
self,
Expand Down Expand Up @@ -780,7 +780,7 @@ def _post_vllm_update_weights_http(self, update_info: dict) -> dict:
return self._make_request(
"update_weights",
{"update_info": update_info},
timeout=self._weight_transfer_http_timeout(),
timeout=self._engine_request_http_timeout(),
)

def health_generate(self, timeout: float = 5.0) -> bool:
Expand Down Expand Up @@ -906,10 +906,13 @@ def release_memory_occupation(self, level: int = 1):
return {"ok": True, "sleep_mode": False, "note": "vLLM sleep mode disabled; no /sleep call."}
# vLLM ``POST /sleep`` reads ``level`` from query params, not JSON body
# (``vllm.entrypoints.serve.sleep.api_router.sleep``).
# Use the (tunable) engine-request timeout, not a hardcoded 30s: releasing a large model's
# weights/KV scales with model size, and under vLLM data parallelism the front API server
# (api_server_count=dp) coordinates every replica's sleep, so a 30B+ engine exceeds 30s.
response = requests.post(
f"{self._http_base()}/sleep",
params={"level": level},
timeout=30,
timeout=self._engine_request_http_timeout(),
)
Comment on lines 912 to 916

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.

medium

Using a single float for timeout sets both the connection timeout and the read timeout to that value (which defaults to 900 seconds). If the vLLM server is completely dead or unreachable, the connection attempt will hang for 15 minutes, blocking the Ray actor. Specifying a tuple like (5.0, self._weight_transfer_http_timeout()) keeps a short, sensible connection timeout while still allowing the long-running sleep operation to complete.

Suggested change
response = requests.post(
f"{self._http_base()}/sleep",
params={"level": level},
timeout=30,
timeout=self._weight_transfer_http_timeout(),
)
response = requests.post(
f"{self._http_base()}/sleep",
params={"level": level},
timeout=(5.0, self._weight_transfer_http_timeout()),
)

return _response_json(response)

Expand All @@ -926,7 +929,7 @@ def resume_memory_occupation(self, tags: list[str] | None = None):
response = requests.post(
f"{self._http_base()}/wake_up",
params=wake_params,
timeout=30,
timeout=self._engine_request_http_timeout(),
)
Comment on lines 929 to 933

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.

medium

Using a single float for timeout sets both the connection timeout and the read timeout to that value (which defaults to 900 seconds). If the vLLM server is completely dead or unreachable, the connection attempt will hang for 15 minutes, blocking the Ray actor. Specifying a tuple like (5.0, self._weight_transfer_http_timeout()) keeps a short, sensible connection timeout while still allowing the long-running wake_up operation to complete.

Suggested change
response = requests.post(
f"{self._http_base()}/wake_up",
params=wake_params,
timeout=30,
timeout=self._weight_transfer_http_timeout(),
)
response = requests.post(
f"{self._http_base()}/wake_up",
params=wake_params,
timeout=(5.0, self._weight_transfer_http_timeout()),
)

return _response_json(response)

Expand All @@ -936,7 +939,7 @@ def init_weight_transfer_engine(self, payload: dict) -> dict:
For IPC mode the payload is ``{"init_info": {}}``; for NCCL use
``init_weights_update_group`` which constructs the payload from typed args.
"""
init_timeout_s = self._weight_transfer_http_timeout()
init_timeout_s = self._engine_request_http_timeout()
last_error = None
for attempt in range(1, 4):
try:
Expand All @@ -953,7 +956,7 @@ def start_weight_update(self, is_checkpoint_format: bool = False) -> dict:
return self._make_request(
"start_weight_update",
{"is_checkpoint_format": is_checkpoint_format},
timeout=self._weight_transfer_http_timeout(),
timeout=self._engine_request_http_timeout(),
)

def finish_weight_update(self) -> dict:
Expand All @@ -963,7 +966,7 @@ def finish_weight_update(self) -> dict:
``update_weights_from_tensor`` (the IPC data-carrying RPC), matching slime's
single-RPC version-with-data semantics.
"""
return self._make_request("finish_weight_update", {}, timeout=self._weight_transfer_http_timeout())
return self._make_request("finish_weight_update", {}, timeout=self._engine_request_http_timeout())

def check_weights(self, action: str):
"""No vLLM ``weights_checker`` route; return a placeholder dict."""
Expand All @@ -985,7 +988,7 @@ def init_weights_update_group(self, master_address, master_port, rank_offset, wo
"world_size": world_size,
}
}
init_timeout_s = self._weight_transfer_http_timeout()
init_timeout_s = self._engine_request_http_timeout()
last_error = None
for attempt in range(1, 4):
try:
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/backends/vllm_utils/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def vllm_args() -> SimpleNamespace:
hf_checkpoint="/tmp/model",
vllm_router_ip=None,
vllm_router_port=None,
vllm_weight_transfer_timeout_sec=900.0,
vllm_engine_request_timeout_secs=900.0,
num_gpus_per_node=8,
rollout_num_gpus_per_engine=4,
colocate=False,
Expand Down
19 changes: 11 additions & 8 deletions tests/unit/backends/vllm_utils/test_arguments.py
Original file line number Diff line number Diff line change
Expand Up @@ -260,21 +260,24 @@ def test_orchestration_dests_use_vllm_prefix(args_mod):
assert "vllm_router_ip" in args_mod._VIME_ORCHESTRATION_DESTS
assert "vllm_router_port" in args_mod._VIME_ORCHESTRATION_DESTS
assert "router_request_timeout_secs" in args_mod._VIME_ORCHESTRATION_DESTS
assert "vllm_weight_transfer_timeout_sec" in args_mod._VIME_ORCHESTRATION_DESTS
assert "vllm_engine_request_timeout_secs" in args_mod._VIME_ORCHESTRATION_DESTS
assert "router_ip" not in args_mod._VIME_ORCHESTRATION_DESTS
assert "router_port" not in args_mod._VIME_ORCHESTRATION_DESTS
assert "vllm_router_request_timeout_secs" not in args_mod._VIME_ORCHESTRATION_DESTS


@pytest.mark.unit
def test_add_vllm_arguments_parses_weight_transfer_timeout(args_mod, monkeypatch):
def test_add_vllm_arguments_parses_engine_request_timeout(args_mod, monkeypatch):
monkeypatch.setattr(args_mod.AsyncEngineArgs, "add_cli_args", staticmethod(lambda parser: parser))
parser = argparse.ArgumentParser(add_help=False)
args_mod.add_vllm_arguments(parser)
default, _ = parser.parse_known_args([])
assert default.vllm_weight_transfer_timeout_sec == 900.0
parsed, _ = parser.parse_known_args(["--vllm-weight-transfer-timeout-sec", "123.5"])
assert parsed.vllm_weight_transfer_timeout_sec == 123.5
assert default.vllm_engine_request_timeout_secs == 900.0
parsed, _ = parser.parse_known_args(["--vllm-engine-request-timeout-secs", "123.5"])
assert parsed.vllm_engine_request_timeout_secs == 123.5
# deprecated alias --vllm-weight-transfer-timeout-sec still maps to the same dest
aliased, _ = parser.parse_known_args(["--vllm-weight-transfer-timeout-sec", "55.0"])
assert aliased.vllm_engine_request_timeout_secs == 55.0


def _realistic_add_vllm_arguments(parser):
Expand All @@ -284,8 +287,8 @@ def _realistic_add_vllm_arguments(parser):
parser.add_argument("--vllm-router-port", dest="vllm_router_port", type=int, default=None)
parser.add_argument("--vllm-server-concurrency", dest="vllm_server_concurrency", type=int, default=512)
parser.add_argument(
"--vllm-weight-transfer-timeout-sec",
dest="vllm_weight_transfer_timeout_sec",
"--vllm-engine-request-timeout-secs",
dest="vllm_engine_request_timeout_secs",
type=float,
default=900.0,
)
Expand All @@ -311,7 +314,7 @@ def test_action_table_excludes_orchestration(args_mod, monkeypatch):
assert "vllm_router_ip" not in table
assert "vllm_router_port" not in table
assert "vllm_server_concurrency" not in table
assert "vllm_weight_transfer_timeout_sec" not in table
assert "vllm_engine_request_timeout_secs" not in table


@pytest.mark.unit
Expand Down
20 changes: 10 additions & 10 deletions tests/unit/backends/vllm_utils/test_vllm_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ def fake_post(endpoint: str, payload: dict, timeout: float):
assert len(calls) == 1
assert calls[0][0] == "start_weight_update"
assert calls[0][1] == {"is_checkpoint_format": True}
assert calls[0][2] == vllm_engine._weight_transfer_http_timeout()
assert calls[0][2] == vllm_engine._engine_request_http_timeout()


@pytest.mark.unit
Expand All @@ -189,7 +189,7 @@ def fake_post(endpoint: str, payload: dict, timeout: float):
result = vllm_engine.finish_weight_update()

assert result == {"done": True}
assert calls == [("finish_weight_update", {}, vllm_engine._weight_transfer_http_timeout())]
assert calls == [("finish_weight_update", {}, vllm_engine._engine_request_http_timeout())]


@pytest.mark.unit
Expand Down Expand Up @@ -311,20 +311,20 @@ def fake_post(endpoint: str, payload: dict, timeout: float):


@pytest.mark.unit
def test_weight_transfer_http_timeout_reads_config(vllm_engine):
vllm_engine.args.vllm_weight_transfer_timeout_sec = 123.5
assert vllm_engine._weight_transfer_http_timeout() == 123.5
def test_engine_request_http_timeout_reads_config(vllm_engine):
vllm_engine.args.vllm_engine_request_timeout_secs = 123.5
assert vllm_engine._engine_request_http_timeout() == 123.5


@pytest.mark.unit
def test_weight_transfer_http_timeout_uses_argument_default(vllm_engine):
assert vllm_engine.args.vllm_weight_transfer_timeout_sec == 900.0
assert vllm_engine._weight_transfer_http_timeout() == 900.0
def test_engine_request_http_timeout_uses_argument_default(vllm_engine):
assert vllm_engine.args.vllm_engine_request_timeout_secs == 900.0
assert vllm_engine._engine_request_http_timeout() == 900.0


@pytest.mark.unit
def test_start_weight_update_uses_config_timeout(vllm_engine, monkeypatch):
vllm_engine.args.vllm_weight_transfer_timeout_sec = 123.5
vllm_engine.args.vllm_engine_request_timeout_secs = 123.5
calls: list[tuple] = []

def fake_post(endpoint: str, payload: dict, timeout: float):
Expand All @@ -339,7 +339,7 @@ def fake_post(endpoint: str, payload: dict, timeout: float):

@pytest.mark.unit
def test_init_weights_update_group_uses_config_timeout(vllm_engine, monkeypatch):
vllm_engine.args.vllm_weight_transfer_timeout_sec = 123.5
vllm_engine.args.vllm_engine_request_timeout_secs = 123.5
calls: list[tuple] = []

def fake_post(endpoint: str, payload: dict, timeout: float):
Expand Down
Loading