Skip to content

ai_dynamo.sh: launch workers before the ingress readiness wait#972

Merged
podkidyshev merged 1 commit into
NVIDIA:mainfrom
yanok:aidynamo-worker-launch-order
Jul 21, 2026
Merged

ai_dynamo.sh: launch workers before the ingress readiness wait#972
podkidyshev merged 1 commit into
NVIDIA:mainfrom
yanok:aidynamo-worker-launch-order

Conversation

@yanok

@yanok yanok commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Summary

On a combined frontend+worker node (e.g. a single-node aggregate
deployment), main() calls launch_ingress before the decode/prefill
blocks are reached. launch_ingress blocks in wait_for_router — but the
router cannot become ready until a worker registers, and no worker has been
launched yet. The wait therefore always burns its entire
ROUTER_START_TIMEOUT budget (120 s of failing readiness curls), every
combined-node run pays a guaranteed 2-minute startup tax, and the log ends
up with a misleading Router did not become ready ERROR for a router that
is actually fine.

This PR reorders main(): workers launch first, then the ingress. Workers
only need etcd/nats (already waited on above) and the lmcache config from
setup_lmcache; they register via etcd and never talk to the router — so
nothing about worker startup depends on the ingress. With the new order,
the router readiness wait overlaps worker/engine initialization instead of
serializing in front of it, and it completes as soon as a worker registers
rather than timing out.

Behavior is unchanged where the old order was already correct:

  • Multi-node: worker-only nodes skip the frontend blocks entirely;
    ordering across nodes was already concurrent.
  • sglang DSR1: launch_sgl_http_server moves together with
    launch_ingress, keeping their relative order.
  • Phase restarts via routerctl.sh are untouched.

Test Plan

  • Environment: internal Slurm cluster (DGX nodes), ai_dynamo workload
    in single-node aggregate mode (combined frontend+worker), vLLM workers
    with LMCache.
  • The reordered script has been running in our production benchmarking
    harness since 2026-07-14, across dozens of single-node combined runs
    (multi-hour benchmark sweeps): startup no longer serializes the 120 s
    router wait in front of worker launch, the Router did not become ready
    ERROR is gone, and the router readiness completes once the first worker
    registers.
  • Multi-node paths were verified by inspection: the frontend blocks are
    gated on _is_frontend_node, which worker-only nodes never enter, so the
    diff is a no-op for them.

Additional Notes

The root cause is a circular wait, not a slow router: wait_for_router
cannot succeed before a worker exists, so any ordering that puts it ahead
of worker launch on the same node converts the timeout into a fixed startup
cost. If a future change makes the router ready without workers, the new
order still holds (the wait just returns early).

On a combined frontend+worker node (single-node aggregate), launch_ingress
blocks in wait_for_router for the full ROUTER_START_TIMEOUT (120 s of
failing readiness curls) before the decode/prefill blocks are reached —
but the router cannot become ready until a worker registers, so the wait
always burns its entire budget and every run pays a guaranteed 2-minute
startup tax plus a misleading 'Router did not become ready' ERROR.

Workers only need etcd/nats (already waited on) and the lmcache config
from setup_lmcache; they register via etcd and never talk to the router.
Launch them first, then bring up the ingress: its readiness wait now
overlaps worker/engine initialization instead of serializing in front of
it. Multi-node behavior is unchanged (worker-only nodes skip the frontend
blocks; ordering across nodes was already concurrent). Phase restarts via
routerctl.sh are untouched.

Signed-off-by: Ilya Yanok <iyanok@nvidia.com>
@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 0187fd02-689a-4e32-a87c-519ac267609a

📥 Commits

Reviewing files that changed from the base of the PR and between 09f728c and fe7739e.

📒 Files selected for processing (1)
  • src/cloudai/workloads/ai_dynamo/ai_dynamo.sh

📝 Walkthrough

Walkthrough

Changes

Frontend startup ordering

Layer / File(s) Summary
Worker-before-ingress startup
src/cloudai/workloads/ai_dynamo/ai_dynamo.sh
The frontend startup flow waits for etcd, launches workers, and then starts ingress services, including the conditional SGLang HTTP server.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: srivatsankrishnan

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: launching workers before the ingress readiness wait.
Description check ✅ Passed The description accurately and thoroughly explains the reorder and its impact, matching the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@podkidyshev
podkidyshev merged commit 7f36e65 into NVIDIA:main Jul 21, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants