[chore] bump vllm to 0.26.0 - #1854
Conversation
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
| No-op when DP is enabled (vLLM assigns the master port itself on that path) | ||
| or when the caller already set the variable. | ||
| """ | ||
| os.environ.setdefault("VLLM_DP_MASTER_PORT", str(pick_free_port())) |
There was a problem hiding this comment.
This can have race conditions when running multiple engines on the same node right? Say TP=2, num_engines=4, we might grab the same port on all the VLLMServerActors with pick_free_port because we release the port immediately. The local torch.distributed group per engine might end up trying to use the bind to the same port.
Why can't we use the same reservation based setup as in find_and_reserve_port? Reserve the port first and then close it just before engine initialization.
There was a problem hiding this comment.
Hmm the actual port that gets used is VLLM_DP_MASTER_PORT + 100. So ideally we need to ensure that even this port is free.
I feel like the best fix for this is inside vLLM, but for now using find_and_reserve_port should atleast prevent the case of base port collision across engines.
There was a problem hiding this comment.
hmm seems like vllm-project/vllm#50969 might address this
There was a problem hiding this comment.
Oh nice. We will need to have a workaround meanwhile though
There was a problem hiding this comment.
yep added a workaround for now and inlined comments so an agent can delete it on bump to like 0.28.0 probably
There was a problem hiding this comment.
These changes seem reasonable to me! Have you run GPU CI already?
There was a problem hiding this comment.
yeah ran offline on h100 node overnight, all passing (report in the .md file)
There was a problem hiding this comment.
let me just run them here again
There was a problem hiding this comment.
Is this custom endpoint still needed in vllm 0.26.0 btw?
If we can remove it, we can also do so in a follow-up, but good to check
There was a problem hiding this comment.
yeah probably not needed anymore but i'm not familiar
created an issue to track: #2020
… infra log on failure (#2051) # Fix colocated tinker E2E nightly: vLLM startup OOM on L4 The `SkyRL-GPU-E2E-CI-Tinker` nightly has failed on every run since 2026-08-14 (the first nightly after the vLLM 0.26.0 bump in #1854) with: ``` RuntimeError: Engine core initialization failed. See root cause above. Failed core proc(s): {} ``` raised from `VLLMServerActor.start()` at the first `save_weights_for_sampler`. (Failures on 08-08..08-13 were the separate unpinned tinker SDK break, fixed by #2022.) ## Root cause Reproduced on a 1x L4 workspace with a scaled-down (1-GPU) version of the CI backend config. The real error never reaches the job log (see "Log visibility" below); it is a CUDA OOM during vLLM engine startup, at the flashinfer sampler warmup that runs after CUDA graph capture: ``` torch.OutOfMemoryError: CUDA out of memory. Tried to allocate 594.00 MiB. GPU 0 has a total capacity of 22.03 GiB of which 582.00 MiB is free. Process 29529 has 796.00 MiB memory in use. ... this process has 20.67 GiB in use. ``` Two factors combine: 1. **Engine startup ordering.** The main trainer entrypoint starts inference engines on empty GPUs before `build_models()`. The tinker `SkyRLTrainBackend` does the reverse: FSDP workers initialize first, and even after `offload_to_cpu()` each leaves ~800 MiB of unreclaimable CUDA context on the GPU that vLLM later profiles on. `gpu_memory_utilization` budgets a fraction of *total* (not free) memory, so vLLM still sizes its KV cache as if it had the whole card. 2. **vLLM 0.26.0's larger startup peak.** FULL_AND_PIECEWISE cudagraph capture plus the flashinfer sampler warmup transiently allocate ~3 GiB beyond the utilization budget. At 0.8 on a 22 GiB L4 with the FSDP context resident, this misses by ~12 MiB. This explains why only this nightly regressed: the fully-async tinker E2E is non-colocated (engines get empty GPUs), and the main colocated E2E starts engines before building models. ## Changes - `gsm8k_tinker.sh`: `gpu_memory_utilization` 0.8 → 0.7. Costs ~2 GiB of KV cache, irrelevant for 512-token GSM8K rollouts. - Both tinker E2E scripts: on any non-zero exit, the cleanup trap now dumps the tails of `server.log` **and** the newest `/tmp/skyrl-logs/infra-*.log`. ### Log visibility `VLLMServerActor` calls `redirect_actor_output_to_file()`, which sends actor output — including the vLLM engine's real traceback — to `/tmp/skyrl-logs/infra-*.log` on the cluster. The scripts previously only tailed `server.log`, and only when the server failed to boot, so a client-visible failure like this one showed nothing but the opaque re-raised `RayTaskError` (the empty `Failed core proc(s): {}` is a vLLM race where the engine-core proc dies before its exit code is captured). ## Verification On a 1x L4 with the 1-GPU equivalent of the CI backend config (`colocate_all=true`, 1 engine): - at `gpu_memory_utilization=0.8`, `save_weights_and_get_sampling_client()` fails with the exact CI error (OOM in flashinfer sampler warmup); - at `0.7`, the same call succeeds: engine starts, weights sync, KV cache wakes at 18.5/22 GiB. ## Note Any tinker + `colocate_all` user on ~24 GB GPUs will hit this same OOM at the default 0.8 utilization, since the lazy engine startup ordering is inherent to the backend. A follow-up could account for the training workers' CUDA context when sizing the KV budget, or start engines eagerly once the LoRA config is known. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > CI-only shell script changes; no production trainer or auth paths touched. > > **Overview** > Fixes the **colocated** tinker GSM8K nightly (`gsm8k_tinker.sh`) by lowering vLLM **`gpu_memory_utilization` from 0.8 to 0.7**, with comments explaining lazy engine startup after FSDP leaves CUDA context and vLLM 0.26’s larger startup peak OOMing 22 GiB L4s. The fully-async script keeps **0.8** (non-colocated engines). > > **Both** tinker E2E scripts replace the simple EXIT trap with a **`cleanup()`** that on any non-zero exit tails **`server.log`** and the newest **`/tmp/skyrl-logs/infra-*.log`** (Ray/vLLM errors) before tearing down the server process group—so CI logs show real OOM/tracebacks instead of opaque `RayTaskError`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 77ff96c. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Signed-off-by: Avi Basnet <avigyabb@stanford.edu> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
vLLM 0.26 (now on main via NovaSky-AI#1854) added an annotation-only lora_manager member to the SupportsLoRA protocol. The patched KimiK25ForConditionalGeneration is checked *structurally* (unlike native nominal subclasses), so the member must exist on the class; without it the supports_lora() gate rejects the model. Set it to None, the documented 'not yet set' state. Also future-proof KimiK25TextBridge.build_conversion_tasks for megatron-bridge 0.7.0 (NovaSky-AI#2042), which grows the base signature with a weight_dtype keyword: forward *args/**kwargs instead of pinning the two-argument shape. Co-authored-by: Cursor <cursoragent@cursor.com>
^
Note
High Risk
Touches critical inference startup and weight-sync paths (NCCL + delta), plus the core vLLM/flashinfer dependency stack used by all GPU training.
Overview
Bumps vLLM 0.23 → 0.26.0 (cu129 wheel) and applies the compatibility fixes needed to keep install, server startup, and weight sync working.
Dependency / install fixes: pins
flashinfer*to 0.6.13 (MNNVLlayout_codesupport; cubin has no 0.6.14), overridesxgrammar==0.2.3for a cp312 linux wheel, and poisonsflash_attn.cuteat import time so megatron-core's FA4 probe no longer crashes on cutlass-dsl 4.6.Server startup: passes the new required
reuse_port=Falsetocreate_server_socket, and seedsVLLM_DP_MASTER_PORTwith a free ephemeral port so RayExecutorV2 no longer tries privileged port 100 when DP is off.Weight sync API drift: NCCL sync now retargets the engine via
set_weight_update_target+ a_LoadWeightsProxy(0.26 dropped theload_weightscallback).DeltaWeightTransferEngineadopts the 4-arg constructor and the fullupdate_weights/ start / finish lifecycle.Tests / misc: multimodal
mm_serdeimport falls back across the moved module path; GLM MLA coverage swaps the unservable tiny fixture for realzai-org/GLM-4.7-Flashin the h100 job. AddsVLLM_0.26_UPGRADE_NOTES.mddocumenting each required fix.Reviewed by Cursor Bugbot for commit 8d61888. Bugbot is set up for automated code reviews on this repo. Configure here.