feat(dynamo): support LoRA lifecycle - #3179
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5ff9be2. Configure here.
| timeout=httpx.Timeout(None), | ||
| ) | ||
| for url in urls | ||
| ] |
There was a problem hiding this comment.
Control clients omit auth headers
Medium Severity
_setup_control_clients builds worker admin and LoRA system clients without the API key or custom headers that setup_admin_clients always attaches. With VLLM_API_KEY or configured headers set, Dynamo pause/resume//v1/loras calls can fail auth while frontend readiness checks still succeed.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 5ff9be2. Configure here.
| def __init__(self, client_config: ClientConfig, workers: tuple[DiscoveredDynamoWorker, ...], **kwargs): | ||
| admin_clients = _setup_control_clients([worker.admin_base_url for worker in workers]) | ||
| super().__init__(client_config, admin_clients=admin_clients, **kwargs) | ||
| self._admin_world_sizes = [worker.world_size for worker in workers] |
There was a problem hiding this comment.
Discovered world sizes unused
Medium Severity
DynamoInferencePool stores per-worker _admin_world_sizes from discovery but never reads them. NCCL/NIXL init still derives rank offsets as inference_world_size // len(admin_clients), so heterogeneous Dynamo topologies (for example different prefill vs decode TP) get incorrect broadcast ranks.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 5ff9be2. Configure here.


Summary
Dependency
This branch includes the prerequisite commit because it targets
main. Its effective diff will shrink when #3176 merges.Test plan
ruff checkon the discovery, LoRA, and test filesNote
Medium Risk
Changes the orchestrator weight-update and inference discovery paths for Dynamo and LoRA; misconfigured
inference_world_sizeor partial worker capability can block startup or leave engines paused if resume fails (mitigated byfinallyresume in tests).Overview
Adds Dynamo-based inference pool setup via
dynamo_discovery_urland/v1/rl/workers, replacing staticadmin_base_urlfor worker admin endpoints and per-engineworld_size. Discovery retries until the summed workerworld_sizematchesweight_broadcast.inference_world_size, which is now required on the orchestrator when Dynamo is enabled and can be set on shared NCCL/NIXL/filesystem broadcast for runs without a localinferenceconfig.DynamoInferencePoolextends the static pool with discovered admin clients, optional per-workerupdate/load_lorasystem URLs, and readiness that also checks the frontend/v1/models. LoRA weight updates on Dynamo pause engines on admin clients, POST adapters to/v1/lorason every worker’s system endpoint, wait until the adapter appears on the frontend, then resume; non-LoRA updates still use the existing admin path.StaticInferencePoolcan inject pre-built admin clients for this flow.Config validation blocks mixing Dynamo with
admin_base_urlor elastic discovery; RL resolution propagatesweight_broadcastto the orchestrator and relaxes the single-node NCCL “two local GPUs” rule wheninference_world_sizeis set for external inference.Reviewed by Cursor Bugbot for commit 5ff9be2. Bugbot is set up for automated code reviews on this repo. Configure here.