Repository navigation
[docker] add vllm-openai-efa target - #58768
Conversation
|
Documentation preview: https://vllm--58768.org.readthedocs.build/en/58768/ |
|
nice! can we also ensure that nixl and deepepv2 work properly in these images? |
functionstackx
left a comment
There was a problem hiding this comment.
Can u also test if Mooncake p2p store works and not just mooncake transfers?
here is an example here SemiAnalysisAI/InferenceX#3405
|
Thanks for the prompt response, @robertgshaw2-redhat ! I've already tested Nixl with P/D disagg with this image. @functionstackx , yeah, will use mooncake store instead of p/d for mooncake TE coverage here. |
NIXL is included in the base vLLM image, the Python wheels from pypi include the libfabric plugin. |
|
Here are the steps to build and run image from this PR on a AWS GPU cluster: Build Testing P/D disagg with NIXL+ libfabric/EFA backend |
|
The current cuda base image doesn't have an |
|
Mooncake Store has been tested, it's working fine with EFA using mooncake-transfer-engine-efa. and |
|
hi @snadampal since the new proposed approach is to have an separate efa specific build for vllm/sglang, we might not need to wait for nccl-EP to get well integrated into sglang/vllm to get multi node WideEP (>=EP16) working out of the box for EFA since you can reinstall the efa fork'ed DeepEP in the dockerfile i.e. |
|
seems like here is the AWS file for building deepepv2 https://github.com/dmvevents/awsome-distributed-ai/blob/ca5327caa8b263c0d308d3b992e86161d9db8ef7/examples/inference/vllm/deepep-v2-efa/setup_deepep_v2_efa.s |
We have a better way to integrate this into vLLM and SGLang containers, i'll wait for @snadampal to come online tomorrow and we'll work on it. We are already rolling this into our DLCs, you can see an example: https://github.com/aws/deep-learning-containers/blob/main/docker/vllm/Dockerfile.amzn2023#L163 |
| ARG EFA_INSTALLER_SHA256=fa6dff8593d866866c13cb4640d9059835cd4efa427971f100ab40c97bef2841 | ||
| # Mooncake version the vllm-openai-efa target pins its EFA wheel to. Keep in | ||
| # sync with requirements/kv_connectors*.txt, which install the non-EFA build. | ||
| ARG MOONCAKE_VERSION=0.3.13.post1 |
There was a problem hiding this comment.
please make a comment that this is temporary --- also, can we not just figure this out below?
e.g. we start by uninstalling, then re-install. Instead of manually keeping this in sync
export MY_PKG_VERSION=$(uv pip show mooncake | grep -i '^Version:' | awk '{print $2}')
There was a problem hiding this comment.
yes, it's possible to check the installed wheel's version instead of passing the version here.
| ARG DEEPEP_REPO="https://github.com/amazon-contributing/DeepEP.git" | ||
| ARG DEEPEP_REF="874779c9ccd2294b56304bd6cc5f138f1f71d097" | ||
| # DeepEPv2 GDA over EFA requires NCCL 2.31.x or later. | ||
| ARG EFA_NCCL_VERSION=2.31.2 |
There was a problem hiding this comment.
We should just change the nccl version globally for vllm to 2.31.2, we already have a custom version
cc @tlrmchlsmth - can you confirm this is okay?
There was a problem hiding this comment.
agreed. Should be ok to bump
There was a problem hiding this comment.
great! it will make things more cleaner. @tlrmchlsmth , do you prefer to make this change outside, or ok to make it part of this PR?
There was a problem hiding this comment.
I'm OK to make it part of the pr
|
cc @tlrmchlsmth - what do you think about updating nccl to 2.31? |
|
I think we should add this |
|
thanks for aligning on the base image nccl version, I will update the PR |
|
Hi @snadampal could you update the PR? I can try to test it out |
|
Hi @xinli-sw, I have updated the PR, please give it a try and let me know if you see any issues. thanks. |
|
This pull request has merge conflicts that must be resolved before it can be |
|
✅ Triggered Buildkite CI #96518 for commit |
|
The CI is all green now, all checks have passed! |
functionstackx
left a comment
There was a problem hiding this comment.
thank you @snadampal , Erez, Kevin, robert, Tyler, Xin for this PR!
| # conditional, so this is "tcp if TARGETARCH is not amd64, else efa". | ||
| ARG ARCH_IF_NOT_AMD64=${TARGETARCH#amd64} | ||
| ARG PROTOCOL_IF_NOT_AMD64=${ARCH_IF_NOT_AMD64:+tcp} | ||
| ENV MOONCAKE_PROTOCOL=${PROTOCOL_IF_NOT_AMD64:-efa} |
There was a problem hiding this comment.
I'm not sure if vllm code is reading this env var
| - "cuda-13-0-ubuntu-24-04" | ||
| # AWS EFA variants; no-ops when the opt-in EFA image was not built. | ||
| - "cuda-13-0-efa" | ||
| - "cuda-13-0-ubuntu-24-04-efa" |
There was a problem hiding this comment.
default ubuntu is 24.04 now so we can remove 24-04 variant
There was a problem hiding this comment.
but i still see this: "cuda-13-0-ubuntu-24-04"
are you planning to delete that on mainline?
| # No cu134 entry: Rubin CUDA 13.4 is nightly-only and is not promoted to | ||
| # DockerHub for the base variant either. | ||
|
|
||
| target_enabled cuda-13-0-efa && publish_image_family "-efa" || true |
There was a problem hiding this comment.
I know it's best effort but can we fail loudly here, just in case 1 platform pass but not the other and the failure is silenced
|
Can we also make the publish release image step depends on the EFA build, but with |
|
thanks for the feedback, @khluu , I will update once you confirm the |
|
This pull request has merge conflicts that must be resolved before it can be |
Add docker/Dockerfile.efa, a layer on top of a vllm-openai image (ARG BASE_IMAGE, built with INSTALL_KV_CONNECTORS=true). It adds: - The AWS EFA userspace stack (libfabric, aws-ofi-nccl), with build-time checks that NIXL's LIBFABRIC plugin resolves to the AWS libfabric. Also sets NCCL_NET_PLUGIN=ofi and NCCL_TUNER_PLUGIN=ofi. - On x86_64, the EFA Mooncake wheel at the base image's Mooncake version, with MOONCAKE_PROTOCOL=efa (tcp on other architectures). - DeepEP rebuilt from the amazon-contributing fork for DeepEPv2 GIN over EFA. Bump NCCL_VERSION to 2.31.2 in docker/Dockerfile, which DeepEPv2 GDA over EFA requires. Release pipeline: build EFA variants of the CUDA 13.0, and nightly Rubin CUDA 13.4 images by layering Dockerfile.efa on each staging image, and create their multi-arch manifests. publish-release-images.sh adds a shared publish_image_family helper and publishes the -efa families when present. Co-authored-by: Erez Zarum <erezz@amazon.com> Co-authored-by: Sean Westfall <sean.westfall@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Sunita Nadampalli <nadampal@amazon.com>
Head branch was pushed to by a user without write access
287930d to
1205391
Compare
|
Hi @khluu , I've updated the PR, please check and run CI if it looks good. |
|
(Claude Code here, on @khluu's behalf.) Kicking off PR CI for the updated head, and a release-pipeline test build (shadow pipeline) to check that |
|
/ci run |
|
✅ Triggered Buildkite CI #97029 for commit ℹ️ This PR is 10 commits behind upstream |
|
/ci retry |
|
✅ Queued 2 failed job(s) for retry in Buildkite CI #97029. |
|
Seems the CI failures are irrelevant, and appear in other builds as well |
|
Thanks for merging the PR, @khluu ! |
Conflict in publish-release-images.sh: keep main's new cuda-13-0-efa target (vllm-project#58768) and this branch's single ROCm target (no rocm72). Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andreas Karatzas <AndreasKaratzas@users.noreply.github.com>
Signed-off-by: Sunita Nadampalli <nadampal@amazon.com> Co-authored-by: Erez Zarum <erezz@amazon.com> Co-authored-by: Sean Westfall <sean.westfall@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Christopher Mackenzie <christopher.mackenzie.software@gmail.com>
Signed-off-by: Sunita Nadampalli <nadampal@amazon.com> Co-authored-by: Erez Zarum <erezz@amazon.com> Co-authored-by: Sean Westfall <sean.westfall@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Christopher Mackenzie <christopher.mackenzie.software@gmail.com>
This PR is to improve vLLM out-of-box experience on AWS GPU clusters.
vllm-openai-efato build image with EFA user space libraries and mooncake-transfer-engine-efa.NOTE:
The PR adds a new target
vllm-openai-efainstead of updating the existingvllm-openaibecause therdma-coreversion from EFA installer is different from the version comes with distro. Though the latestrdma-coreversion from the EFA installer should work for non-EFA platforms as well, we didn't test that configuration. And also not to interfere with the existing build configurations, we took a new target approach.Purpose
To improve vLLM out-of-box experience on AWS GPU clusters.
Test Plan
P-D disaggregated inference on AWS p5en.48xlarge 2 node cluster with NIXL LIBFABRIC (efa provider) backend - to be specific, I ran vllm multi-turn conversation benchmark with Qwen3-32B model.
TBD - test Mooncake TransferEngine with EFA trasport
Test Result
Passed
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.