Repository navigation
ci(recipes): benchmark DeepSeek V4 Pro disaggregated nightly - #15393
Conversation
3f42ade to
ee2a278
Compare
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: ai-dynamo/dynamo/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (5)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review. WalkthroughAdds Kustomize resources and a reusable workflow for DeepSeek V4 Pro disaggregated deployment and performance testing. Nightly CI invokes the workflow, which can validate manifests in preflight mode or deploy, benchmark, check results, collect artifacts, and clean up. ChangesDisaggregated nightly performance
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to The new nightly benchmark is mergeable after normal checks. Configuration and cleanup contracts are consistent; live deployment and benchmark performance remain unvalidated. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
dmitry-tokarev-nv
left a comment
There was a problem hiding this comment.
I approve at 39dde44c2e. No P0 or P1 is open. Two P2 findings are on inline threads, and two P3 findings are below. main requires resolved threads, so the two threads hold the merge until you fix or resolve them.
- [P2]
deepseek-v4-pro-disagg-nightly.yml:49: in one nightly, this job and the aggregated job get the sameRUN_KEY. The thread has the measurement and a suggestion. - [P2]
deepseek-v4-pro-disagg-nightly.yml:183: the Verify step can read the warmup export and fail a good run. The thread has the measurement and a suggestion. - [P3]
deepseek-v4-pro-disagg/kustomization.yaml:10pins the Job image todocker.io/library/python@sha256:44ff437b…, a linux/amd64-only image. The aggregated Job usespython:3.12-slim, because #15325 removed the same pin ina06a5e7c4a. Please remove it here too, or add a comment that the amd64nodeSelectorinpatch-perf.yamlmust stay. - [P3] The Validation section of the description is out of date. It says "A live deployment and benchmark have not run." But dispatch run 36646643297 deployed this overlay at
a405ac850cand passed the benchmark. Run 36641543850 failed at ISL 8192. Please update that section, because the passing run is the best evidence for this PR.
What I measured, offline, with no contact to a cluster.
- I ran the Render step of
a405ac850coffline with kubectl 1.36.3 (kustomize v5.8.1). I replaced only the image host and digest. The result is byte-identical to therendered.yaml,deploy.yaml, andperf.yamlartifacts of run 36646643297. With the same run key, the head renders the samedeploy.yaml. Its Job differs only inbackoffLimit: 0,activeDeadlineSeconds: 7200, and the cleanuptrap. A merge with the currentmainrenders the same as the head. - In run 36646643297, 1,536 requests at ISL 8064 passed with 0 errors in 191.66 s. The Job took about 6 minutes, and the head gives it a 7,200 s deadline. At ISL 8192, run 36641543850 got 1,536 errors.
- The fixes from #15325 are all here. I ran the Job as root and the Collect step as the frontend user (uid 1000) on a Docker volume. Each control fails without its fix. The Job leaves no
inputs.jsonon the volume, and the artifact has none. If that delete fails, the file stays on the volume, but the artifact still has none. A failed benchmark keeps its exit code (3 and 5) through thetrap. After a lost run, the next preflight deletes the two Secrets and the three fixed names of that run. - The job uses the runner label,
needs, condition, and concurrency group of the aggregated job, and both actions are pinned by SHA. Apart fromRUN_KEY, the object names, the Secret label, and the artifact name differ from the aggregated job. - At earlier commits, the server-side dry runs of preflight-only runs 36639830031 and 36641433494 accepted the DGD, the ComputeDomain, the Job, and both Secrets. A dry run does not test GPU capacity, scheduling, image pulls, readiness, or the deletes, which run only in a full run.
dmitry-tokarev-nv
left a comment
There was a problem hiding this comment.
I approve at dcf127f089. No P0, P1, or P2 is open, and one P3 from my last review is still open. You applied both of my suggestions, so part of this review covers my own code.
- [P2] Fixed: the shared
RUN_KEY. I replied on its thread with the measurement and resolved it. - [P2] Fixed: the Verify step that read the warmup export. I replied on its thread with the measurement and resolved it.
- [P3] Fixed: the
python@sha256pin is gone. The Job now usespython:3.12-slimfrom the recipe, the same image as the aggregated Job. - [P3] Open: the Validation section of the description still says "A live deployment and benchmark have not run." Run 36646643297 deployed this overlay at
a405ac850cand passed.
What I measured at dcf127f, offline, with no contact to a cluster.
- I rendered the overlay at
39dde44c2eand atdcf127f089with the same run key. The DGD and the ComputeDomain are byte-identical, so the YAML anchors inpatch-deploy.yamlchange nothing in the output. The only change is the Job image, fromdocker.io/library/python@sha256:44ff437b…topython:3.12-slim. The rendered files hold no anchors or aliases. - The new
name:lines change only the names that GitHub shows. In nightly run 36688149628, the aggregated job showed asDeepSeek V4 Pro aggregated perf / perf. The step innotify-slack.ymlturns that name into:failed: perf, so with the old names both jobs print:failed: perf. With the new names, the same step prints:failed: DeepSeek V4 Pro aggregated perfand:failed: DeepSeek V4 Pro disaggregated perf. I found no other reader of these names in.github/. Apart from the job name, the aggregated workflow did not change. - sara4dev asked for the upstream image, YAML anchors, and one
recipesgroup. The pin is gone, the anchors render the same objects, and both caller jobs are now namedrecipes. I did not look at the result in the GitHub UI. Their three threads are still open. - A merge with
mainat1a26bf9cb7conflicts only in theneedslist ofnotify-slack, wheremainaddedsidecar-trtllm-test. With both entries kept, the render does not change, and actionlint 1.7.12 reports no new findings.
Signed-off-by: Lavanya <lvijayakrish@nvidia.com>
Signed-off-by: Lavanya <lvijayakrish@nvidia.com>
Signed-off-by: Lavanya <lvijayakrish@nvidia.com>
Signed-off-by: Lavanya <lvijayakrish@nvidia.com>
Signed-off-by: Lavanya <lvijayakrish@nvidia.com>
Signed-off-by: Lavanya <lvijayakrish@nvidia.com>
Signed-off-by: Lavanya <lvijayakrish@nvidia.com>
Signed-off-by: Lavanya <lvijayakrish@nvidia.com>
Signed-off-by: Lavanya <lvijayakrish@nvidia.com>
dcf127f to
58c36db
Compare
⏹️ Dynamo PR CI cancelled — run 36882120463 (attempt 1) on
|
| Framework | Build | 1-GPU amd64 | 1-GPU arm64 | Multi-GPU amd64 | Deploy | Snapshot |
|---|---|---|---|---|---|---|
| vLLM | ✅ 3 | ✅ 1 | ✅ 1 | ⏹️ 1 | ✅ 4 | ⏭️ |
| SGLang | ✅ 3 | ✅ 1 | ✅ 1 | ✅ 1 | ✅ 2 | ⏭️ |
| TRT-LLM | ✅ 3 | ✅ 1 | ✅ 1 | ✅ 1 | ✅ 2 | ⏭️ |
| Other | Jobs |
|---|---|
| changed-files | ✅ 1 |
| deploy-operator | ✅ 1 |
| DGDR Deploy Test | ✅ 4 |
| dynamo-runtime | ✅ 8 |
| frontend | ✅ 2 |
| frontend (amd64) | ✅ 1 |
| frontend (arm64) | ✅ 1 |
| Helm Chart Tests | ✅ 1 |
| Operator | ✅ 1 |
| Operator Integration | ✅ 1 |
| planner | ✅ 6 |
| Power Agent | ✅ 1 |
| triton-runtime | ✅ 2 |
⏹️ 1 cancelled jobs
⏭️ 3 other components not run (skipped by change detection or an upstream result)
allure-report, dynamo-sidecar, sidecar-runtime
Failure details
No jobs failed. The 1 cancelled job (vllm-runtime / 2-GPU Test cuda13.0, amd64) was not a fail-fast cancel: its self-hosted runner received a shutdown signal mid-test (infrastructure), which also turned backend-status-check red; rerunning failed jobs is likely enough.
| Job | Summary |
|---|---|
| ⏹️ vllm-runtime / 2-GPU Test cuda13.0, amd64 | Runner shutdown signal during Run GPU tests (sequential); infrastructure, rerun failed jobs. |
⏹️ vllm-runtime / 2-GPU Test cuda13.0, amd64: runner received a shutdown signal
Failed step: Run GPU tests (sequential) · Logs: gh api repos/ai-dynamo/dynamo/actions/jobs/110448255052/logs
tests/test_predownload_models.py::test_predownload_models[predownload_models_vllm_gpu2] PASSED [ 10%]
tests/serve/test_vllm.py::test_serve_deployment[agg-router-3]
##[error]Process completed with exit code 130.
##[error]The runner has received a shutdown signal. This can happen when the runner service is stopped, or a manually started runner is canceled.
##[error]Executing the custom container implementation failed. Please contact your self hosted runner administrator.
The self-hosted runner was shut down about a minute into the sequential GPU tests, while test_serve_deployment[agg-router-3] was running; no test failure was reported. This is an infrastructure signature (the rest of the run kept going for ~30 more minutes, so it was not a fail-fast cancel), and rerunning failed jobs is likely enough.
For agents
{"pr": 15393, "run_id": 36882120463, "run_attempt": 1, "head_sha": "fde2a8a7ce008c4ba749b6e83a4d2d53543e1c63", "failures": [], "cancelled": [{"job": "vllm-runtime / 2-GPU Test cuda13.0, amd64", "job_id": 110448255052, "failed_step": "Run GPU tests (sequential)", "signature": "The runner has received a shutdown signal (exit code 130)", "tests": [], "log_cmd": "gh api repos/ai-dynamo/dynamo/actions/jobs/110448255052/logs"}]}Previous runs
- ✅ run 36804087340 (attempt 2) on
58c36dbf1a: Passed (56 passed, 0 failed, 0 cancelled). - ❌ run 36804087340 (attempt 1) on
58c36dbf1a: 1 job failed: 1 SGLang tool-calling test (test_chained_tool_use_search_then_calculate[rust_parsers]) fails because the first step ends withfinish_reason='length'instead oftool_calls, after 3 automatic retries. No jobs were cancelled.
Posted automatically by Devin for run 36882120463. Updated on every full-CI run of this PR.
dmitry-tokarev-nv
left a comment
There was a problem hiding this comment.
I approve at 58c36dbf1a. No P0 or P1 is open. One P2 is on an inline thread, and two P3 findings are below. main requires resolved threads, so the P2 thread holds the merge until you fix or resolve it.
- [P2]
deepseek-v4-pro-disagg-nightly.yml:34: a dispatch of either DeepSeek workflow cancels the nightly DeepSeek job that waits for the other one. The thread has the evidence. - [P3]
nightly-ci.yml:541andnightly-ci.yml:551: when either DeepSeek job fails, Slack prints:failed: recipes > perf, so the alert does not name the recipe. Please make the first or the last part of the two job names differ. This corrects my last review. - [P3] Still open from my last review: the Validation section of the description says "A live deployment and benchmark have not run." Run 36646643297 deployed this overlay and passed.
The Slack names, measured with the real notifier step.
- GitHub joins the names of nested jobs with " / ". In nightly 36688149628, one such job showed as
dynamo-runtime / image / Build multi-arch cuda13.0. No nightly ran at this head yet. By the same pattern, the two DeepSeek jobs getrecipes / DeepSeek V4 Pro aggregated perf / perfandrecipes / DeepSeek V4 Pro disaggregated perf / perf. - For a name with more than two parts, the "Get Failed jobs" step of
notify-slack.ymlkeeps only the first and the last part. I ran that step offline on the real job list of nightly 36688149628, with a stub forcurl, and changed only the DeepSeek names:
| Names | Slack lines |
|---|---|
| This head, both jobs fail | :failed: recipes > perf, two times |
main, the aggregated job fails |
:failed: DeepSeek V4 Pro aggregated perf > perf |
dcf127f089, two parts, both jobs fail |
:failed: DeepSeek V4 Pro aggregated perf and :failed: DeepSeek V4 Pro disaggregated perf |
name: ${{ inputs.recipe }} on the perf job of shared-recipe-perf.yml, both jobs fail |
:failed: recipes > deepseek-v4-pro-agg and :failed: recipes > deepseek-v4-pro-disagg |
- My last review said that the new names print two different lines. That was true at
dcf127f089, where each name had two parts. It is not true at this head. - The run page shows the full name of each job, so only the Slack line loses the recipe.
- The GitHub docs allow the
inputscontext injobs.<job_id>.name. That example also changes the Kimi K2.5 line to:failed: Kimi K2.5 aggregated functional smoke > kimi-k25-agg. I did not run it on GitHub.
What I measured at 58c36db, offline, with no contact to a cluster.
- The rebase onto
7b1da11015kept both sides of each conflict. Theneedslist ofnotify-slackhas the entries frommainanddeepseek-v4-pro-disagg-perf.deepseek-v4-pro-nightly.ymlis the file frommainplus yourname:line. - I ran the real Configure and Render steps for
kimi-k25-agganddeepseek-v4-pro-aggat7b1da11015and at this head.rendered.yaml,deploy.yaml,perf.yaml, andprerequisites.yamlare byte-identical. InGITHUB_ENV, onlyWORKER=becameWORKERS=, and no other step readsWORKER. Each of 4 one-line mutants of the workflow changed the render. - With a stub
kubectl, both callers gave the same results at both commits. I compared the step outputs, thekubectlcalls, the deletes, the leftovers, and the artifacts. The cases were a full run, a lost run and its recovery, a preflight-only run, and a Job with no output. - The disaggregated DGD has exactly
Frontend,prefill, anddecode. Both workers gethf-ci-deepseek-v4-pro-disagg, and all three services getacr-ci-deepseek-v4-pro-disaggand the image by digest. Nohf-token-secretoracr-token-secretremains. A mutant withworkers=prefill,decodadded a fourth service, so this check can see an extra service. - In the disaggregated Job,
commandis[/bin/sh, -c, <script>], and the cleanuptrapgoes in front of the script, as in the aggregated Job. Apart from the labels, the Secret names, andARTIFACT_ROOT, the objects match the render of the old workflow atb841fc246d. - Configure has its own
deepseek-v4-pro-disaggarm, and Render names the recipe in itselif.Verify AIPerf resultstakes the branch for the DeepSeek recipes and expects 3 × 512 requests, the same count that the Job sends. Deploy, Run AIPerf, Collect, and Clean up do not branch onRECIPE. Verify AIPerf resultspassed on the real output of run 36646643297. When I copied the warmup export over the top-level file, the step failed. When I removed only the top-level file, it also failed. It failed on 1,535 requests, one error, no output,CONCURRENCIESset to "256 512", and the wrong container name.- Our two earlier P2 findings stay fixed. In one nightly, the keys are
ci-deepseek-v4-pro-agg-111-1andci-deepseek-v4-pro-disagg-111-1. With both jobs live, each Clean up deleted only its own DGD, Job, ComputeDomain, and two Secrets, in both orders. Each artifact held only its own concurrencies. - The control mutant with one shared key deleted the objects of the other job and mixed the artifacts. After a lost run of either recipe, the other recipe passed. The next nightly of the lost recipe then removed only its own leftovers.
- The two DeepSeek recipes and Kimi K2.5 share no fixed name. I compared the DGD, Job, ComputeDomain, claim template, Secrets, ConfigMap, frontend endpoint, and artifact root.
- Kimi K2.5 and the aggregated recipe each ask for 8 GPUs on 2 GB200 nodes. The disaggregated recipe asks for 16 GPUs on 4 nodes. All three use the same two node pools. I did not query the cluster, so I cannot say whether all three fit at once.
Signed-off-by: Lavanya <lvijayakrish@nvidia.com>
Signed-off-by: Lavanya <lvijayakrish@nvidia.com>
Signed-off-by: Lavanya <lvijayakrish@nvidia.com>
dmitry-tokarev-nv
left a comment
There was a problem hiding this comment.
I approve at 075a9f0084. No P0 or P1 is open. One new P2 is inline, and two earlier P3 findings are below. main requires resolved threads, so the P2 thread holds the merge until you fix or resolve it.
- [P2]
shared-recipe-perf.yml:233: the new--previousloop saves no log for either DeepSeek recipe. The thread has the evidence and a suggestion. - [P3] Still open from my last review:
nightly-ci.yml:541andnightly-ci.yml:551name both caller jobsrecipes. When a DeepSeek job fails, Slack prints:failed: recipes > perfand does not name the recipe. - [P3] Still open: the Validation section of the description says "A live deployment and benchmark have not run." Run 36646643297 deployed this overlay and passed.
- Fixed: my P2 on the shared concurrency group.
17ce31b9f1addsqueue: maxto both DeepSeek workflows. You resolved that thread, and I replied there with the evidence.
What I measured at 075a9f0, offline, with no contact to a cluster.
- Startup probe. Kustomize merges
failureThreshold: 900into the recipe probe, so each worker keepshttpGet /health:9090, period 10 s, and timeout 10 s. The operator replaces its default probe with this whole probe on the leader pod (graph.go:1844). It removes all probes from the second pod of each worker (backend_vllm.go:77). I ranGenerateBasePodSpecon the rendered DGD at this head and atmain5d6d44beef. The leader probes allow 900 × 10 s = 150 min, which is less than the 180-minute Deploy step. The deadline of the bench Job (7,200 s) starts after Deploy, and the job limit is 960 min. A control with onlyfailureThresholdreached the pod spec with no handler, so the full recipe probe keeps the result valid. - vLLM.
VLLM_ENGINE_READY_TIMEOUT_Sstays5400. In vLLM v0.30.0, that wait starts only after every engine finishes loading (core_client.py:709,utils.py:1240). It does not stop a long first load. - Deploy timeout. The GitHub contexts table allows
inputsinjobs.<job_id>.steps.timeout-minutes, and the step maximum is 360.@actions/expressions0.3.61 returns 105 forkimi-k25-agganddeepseek-v4-pro-agg, and 180 fordeepseek-v4-pro-disagg. actionlint 1.7.12 accepts the line. - Collect. I ran the step at
58c36dbf1a, at this head, and with the suggestion, for all three recipes.testand GNUtarran in the vLLM runtime image as uid 1000. The #15361 failed-Deploy case exits 2 at58c36dbf1aand 0 now. A finished benchmark copies the same files as before (2, 21, and 14 files, withoutinputs.json), and Verify passes. If the Job writes nothing, Verify still fails the run. - NIXL telemetry. From
17ce31b9f1to this head, each worker in the rendered DGD loses onlyNIXL_TELEMETRY_ENABLE=1. After the operator merge, both workers get the operator defaultNIXL_TELEMETRY_ENABLE=n, as in run 36646643297. - Kimi and aggregated callers.
rendered.yaml,deploy.yaml,perf.yaml, andprerequisites.yamlare byte-identical to58c36dbf1a. A full run, a lost run and its recovery, and a preflight-only run give the same steps, deletes, leftovers, and artifacts. The only newkubectlcalls are in Collect. mainat3af83c2056merges without a conflict and changes no file that these renders read. Its only change to a file of this PR is one comment innightly-ci.yml.
Signed-off-by: Lavanya <lvijayakrish@nvidia.com>
Signed-off-by: Lavanya <lvijayakrish@nvidia.com>
8ce574c to
a625c72
Compare
Summary
Add a DeepSeek-V4-Pro vLLM disaggregated GB200 nightly benchmark, following the aggregated setup in PR #15325. The overlay uses the existing 1P/1D deployment and its 8K input / 1K output perf recipe at concurrency 512.
Where should the reviewer start?
.github/ci/deepseek-v4-pro-disagg/: Review the cache, scheduling, and perf patches..github/workflows/deepseek-v4-pro-disagg-nightly.yml: Review image pinning, deployment, benchmark verification, artifacts, and cleanup..github/workflows/nightly-ci.yml: Review the nightly job wiring.Validation
shared-model-cache.nvidia.com/v1alpha1DGD API is deprecated.bash -n, andgit diff --checkpassed.workflow_dispatchcan run it from a branch.Related Issues
🚫 This PR is NOT linked to an issue:
Summary by CodeRabbit