Skip to content

fix(container): cherry-pick SGLang UCX compatibility (#12558) - #12647

Merged
pvijayakrish merged 1 commit into
release/1.4.0from
jegu/cherrypick-DYN-3695-sglang-ucx
Aug 4, 2026
Merged

fix(container): cherry-pick SGLang UCX compatibility (#12558)#12647
pvijayakrish merged 1 commit into
release/1.4.0from
jegu/cherrypick-DYN-3695-sglang-ucx

Conversation

@Chokoyo

@Chokoyo Chokoyo commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Cherry-pick of #12558 to release/1.4.0.

Backports the SGLang arm64 runtime UCX compatibility fix for DYN-3695. The upstream SGLang runtime can expose generic UCX SONAMEs from the system UCX installation while NIXL loads its wheel-bundled UCX, producing a mixed UCX stack. On GB200 this broke multimodal E/P/D embedding transfer when the embedding cache was enabled.

This change installs stable generic UCX aliases that resolve to the NIXL wheel libraries and makes that directory take precedence in LD_LIBRARY_PATH.

Original PR

Validation

  • Cherry-pick applied cleanly with no conflicts.
  • The backport patch is byte-identical to the patch merged on main (SHA-256: 5b586fdb97577bf28b9c654382521ae3388fb06d007b96f524dee87475f16e26).
  • Targeted pre-commit checks passed for all three changed files.
  • Main PR validation passed on GB200/aarch64 with Qwen/Qwen3-VL-2B-Instruct, multimodal E/P/D, --single-gpu, and default nixl-read:
    • Two cache-enabled video requests returned HTTP 200; the second request logged an embedding-cache hit.
    • The cache-disabled control returned HTTP 200.
    • Only the NIXL wheel UCX 1.21 libraries were mapped; the system-UCX warnings and NIXL remote-key errors were absent.
  • CI passes on this release branch

Related Issues

Fixes DYN-3695

…2558)

Signed-off-by: Zhuangcheng(Jesse) Gu <zcgu@connect.hku.hk>
@Chokoyo
Chokoyo marked this pull request as ready for review August 4, 2026 18:45
@Chokoyo
Chokoyo requested a review from a team as a code owner August 4, 2026 18:45

@devin-ai-integration devin-ai-integration Bot 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.

Devin Review found 2 potential issues.

Open in Devin Review

Comment on lines +80 to +84
if [[ "${#PLUGINS[@]}" -eq 0 ]] && \
[[ "${NIXL_UCX_COMPAT_ALLOW_NO_CONSUMER:-0}" != "1" ]]; then
die "no NVSHMEM UCX transport found; aliases were not validated against a consumer;" \
"set NIXL_UCX_COMPAT_ALLOW_NO_CONSUMER=1 to accept this"
fi

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.

🔍 Build fails hard when no NVSHMEM UCX transport is present

If the upstream SGLang runtime image does not ship an nvidia-nvshmem-cu* wheel with nvshmem_transport_ucx.so, PLUGINS is empty and the script calls die unless NIXL_UCX_COMPAT_ALLOW_NO_CONSUMER=1 is set. The Dockerfile invocation (container/templates/sglang_runtime.Dockerfile:176-178) never sets that variable, so any base-image variant (or a future upstream change) that drops NVSHMEM will break the entire non-xpu sglang runtime build rather than just skipping the validation. Validation in the description was only performed on GB200/aarch64; worth confirming the x86 base image also installs the NVSHMEM UCX transport, or setting the escape hatch in the Dockerfile.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

duplicate with #12558 (comment)

Comment on lines +86 to +106
# Keep the generic names beside NIXL's private libraries. Loading libucp or
# libucs through a separate symlink directory changes $ORIGIN and prevents UCX
# from finding both its hashed core dependencies and its ucx/ module directory.
# These aliases are not tracked by the wheel's RECORD, so a later NIXL uninstall
# or upgrade can leave stale or dangling links. Run this after the final install
# step that can modify the nixl-cu* wheel.
for alias in "${!ALIASES[@]}"; do
target="${ALIASES[${alias}]}"
[[ -f "${target}" ]] || die "alias target is missing: ${target}"
[[ "${target%/*}" == "${NIXL_LIB_DIR}" ]] || \
die "alias target is outside the NIXL private library directory: ${target}"
alias_path="${NIXL_LIB_DIR}/${alias}"
if [[ -e "${alias_path}" || -L "${alias_path}" ]]; then
[[ -L "${alias_path}" && "$(readlink -f "${alias_path}")" == "${target}" ]] || \
die "refusing to replace existing path: ${alias_path}"
else
ln -s "${target##*/}" "${alias_path}"
fi
[[ "$(readlink -f "${alias_path}")" == "${target}" ]] || \
die "failed to install ${alias}"
done

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.

🔍 Aliases and the stable symlink are untracked by the wheel RECORD

The generic aliases are written into the NIXL wheel's auditwheel .libs directory and /opt/dynamo/nixl-ucx-compat symlinks into it. Neither is recorded in the wheel's RECORD, so any later pip install/uninstall of nixl-cu* (e.g. in a derived image or at runtime by a user) leaves dangling links while LD_LIBRARY_PATH still points at the path — resulting in silent loader failures. The script comment acknowledges this and requires it be run last; worth confirming no downstream Dockerfile stage or recipe reinstalls nixl after this step.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@datadog-official

datadog-official Bot commented Aug 4, 2026

Copy link
Copy Markdown

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 16.28% (-32.11%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 62e0bab | Docs | Datadog PR Page | Give us feedback!

@pvijayakrish
pvijayakrish merged commit 5f392f5 into release/1.4.0 Aug 4, 2026
165 of 168 checks passed
@pvijayakrish
pvijayakrish deleted the jegu/cherrypick-DYN-3695-sglang-ucx branch August 4, 2026 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants