Repository navigation
[TRTLLM-11316][feat] Add kvbm disagg tests and support kvbm disagg slurm task #13400
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
e60e664
e3b07a1
2f2afbf
fd6cf71
9e342ed
c908240
7caf887
f152089
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -17,6 +17,8 @@ while [[ $# -gt 0 ]]; do | |||||||||||||||||||||||||
| --build-wheel) build_wheel="$2"; shift 2 ;; | ||||||||||||||||||||||||||
| --cuda-architectures) cuda_architectures="$2"; shift 2 ;; | ||||||||||||||||||||||||||
| --trtllm-wheel-path) trtllm_wheel_path="$2"; shift 2 ;; | ||||||||||||||||||||||||||
| --kvbm-enabled) kvbm_enabled="$2"; shift 2 ;; | ||||||||||||||||||||||||||
| --kvbm-package) kvbm_package="$2"; shift 2 ;; | ||||||||||||||||||||||||||
| *) | ||||||||||||||||||||||||||
| echo "Unknown argument: $1" | ||||||||||||||||||||||||||
| exit 1 | ||||||||||||||||||||||||||
|
|
@@ -39,6 +41,8 @@ echo " container_image: ${container_image}" | |||||||||||||||||||||||||
| echo " build_wheel: ${build_wheel}" | ||||||||||||||||||||||||||
| echo " cuda_architectures: ${cuda_architectures}" | ||||||||||||||||||||||||||
| echo " trtllm_wheel_path: ${trtllm_wheel_path}" | ||||||||||||||||||||||||||
| echo " kvbm_enabled: ${kvbm_enabled}" | ||||||||||||||||||||||||||
| echo " kvbm_package: ${kvbm_package}" | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # Set TRTLLM_DISAGG_BENCHMARK_GEN_ONLY=1 for gen_only_no_context mode | ||||||||||||||||||||||||||
| if [ "${benchmark_mode}" = "gen_only_no_context" ]; then | ||||||||||||||||||||||||||
|
|
@@ -138,6 +142,19 @@ else | |||||||||||||||||||||||||
| echo "TensorRT-LLM environment variables saved to ${full_logdir}/env_vars.json" | ||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # Install kvbm if enabled | ||||||||||||||||||||||||||
| if [ "${kvbm_enabled}" = "true" ]; then | ||||||||||||||||||||||||||
| echo "Installing kvbm package: ${kvbm_package}..." | ||||||||||||||||||||||||||
| if ! srun --container-name=${container_name} \ | ||||||||||||||||||||||||||
| --container-mounts=${container_mount} --no-container-mount-home \ | ||||||||||||||||||||||||||
| --mpi=pmix --overlap -N $SLURM_NNODES --ntasks-per-node=1 \ | ||||||||||||||||||||||||||
| bash -c "pip install ${kvbm_package}" \ | ||||||||||||||||||||||||||
| &> ${full_logdir}/2.1_kvbm_install.log; then | ||||||||||||||||||||||||||
|
Comment on lines
+147
to
+152
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: cd examples/disaggregated/slurm/benchmark && wc -l disaggr_torch.slurmRepository: NVIDIA/TensorRT-LLM Length of output: 87 🏁 Script executed: cd examples/disaggregated/slurm/benchmark && sed -n '140,160p' disaggr_torch.slurmRepository: NVIDIA/TensorRT-LLM Length of output: 1045 🏁 Script executed: cd examples/disaggregated/slurm/benchmark && rg -A 50 "kvbm_package" disaggr_torch.slurm | head -80Repository: NVIDIA/TensorRT-LLM Length of output: 2825 🏁 Script executed: cd examples/disaggregated/slurm/benchmark && rg -B 20 "kvbm_package" disaggr_torch.slurm | head -100Repository: NVIDIA/TensorRT-LLM Length of output: 2850 🏁 Script executed: cd examples/disaggregated/slurm/benchmark && grep -n 'bash -c' disaggr_torch.slurmRepository: NVIDIA/TensorRT-LLM Length of output: 392 Pass The Additionally, similar shell injection vulnerabilities exist on lines 95, 115, 126, and 138 using the same pattern with other user-controlled variables ( Suggested fix if ! srun --container-name=${container_name} \
--container-mounts=${container_mount} --no-container-mount-home \
--mpi=pmix --overlap -N $SLURM_NNODES --ntasks-per-node=1 \
- bash -c "pip install ${kvbm_package}" \
+ bash -c 'python3 -m pip install -- "$1"' _ "${kvbm_package}" \
&> ${full_logdir}/2.1_kvbm_install.log; then📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| cleanup_on_failure "kvbm installation failed. Check ${full_logdir}/2.1_kvbm_install.log for details" | ||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||
| echo "kvbm installation completed successfully" | ||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # Get node lists and replace the placeholder with the actual node names | ||||||||||||||||||||||||||
| echo "SLURM_NODELIST: ${SLURM_NODELIST}" | ||||||||||||||||||||||||||
| all_nodes=($(scontrol show hostname $SLURM_NODELIST | sort)) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,8 @@ while [[ $# -gt 0 ]]; do | |
| --build-wheel) build_wheel="$2"; shift 2 ;; | ||
| --cuda-architectures) cuda_architectures="$2"; shift 2 ;; | ||
| --trtllm-wheel-path) trtllm_wheel_path="$2"; shift 2 ;; | ||
| --kvbm-enabled) kvbm_enabled="$2"; shift 2 ;; | ||
| --kvbm-package) kvbm_package="$2"; shift 2 ;; | ||
|
Comment on lines
+20
to
+21
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Right now these flags have no effect in this launcher, so KVBM enablement can be silently ignored. Suggested fix+# Install KVBM package when enabled
+if [ "${kvbm_enabled:-false}" = "true" ]; then
+ echo "Installing KVBM package: ${kvbm_package}..."
+ if ! srun --container-name="${container_name}" \
+ --container-mounts="${container_mount}" --no-container-mount-home \
+ --mpi=pmix --overlap -N "${SLURM_NNODES}" --ntasks-per-node=1 \
+ bash -c "pip install ${kvbm_package}" \
+ &> "${full_logdir}/2_install_kvbm.log"; then
+ cleanup_on_failure "KVBM installation failed. Check ${full_logdir}/2_install_kvbm.log for details"
+ fi
+fi🤖 Prompt for AI Agents |
||
| *) | ||
| echo "Unknown argument: $1" | ||
| exit 1 | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat -n examples/disaggregated/slurm/benchmark/disaggr_torch.slurm | head -50Repository: NVIDIA/TensorRT-LLM
Length of output: 2191
🏁 Script executed:
sed -n '140,160p' examples/disaggregated/slurm/benchmark/disaggr_torch.slurmRepository: NVIDIA/TensorRT-LLM
Length of output: 1045
Initialize the new KVBM flags before first use.
With
set -u, the script will abort at lines 44-45 if a caller omits the new--kvbm-enabledor--kvbm-packageflags. Initializekvbm_enabledandkvbm_packagebefore the parse loop to maintain backward compatibility.Suggested fix
📝 Committable suggestion
🤖 Prompt for AI Agents