feat(slurm): harden distributed allocation runtime

[nabinchha]

📋 Summary

This completes the distributed-topology and failure-hardening slice of the Slurm allocation runtime on top of the merged reconciliation/retry foundation.

The branch has been restacked directly on feat/slurm-execution at 26aea789, which includes merged PRs #913 and #915 as well as #929. The three PR-owned commits layer only host placement, multi-node launch, remote readiness, follower-failure behavior, absolute model-path mapping, and a base-integration test adaptation onto that composition.

🔗 Related Issue

• Part of Implement Slurm submission and allocation runtime #868 (868#2: distributed execution and failure hardening)
• Depends on the merged Slurm stack through feat: reconcile persisted Slurm state #913

🔄 Changes

• resolve scheduler allocation hosts in planner order and pin client, endpoint, server, and control steps to their assigned hosts
• preserve the one-node per-process launch path while adding coordinated one-task-per-node srun steps for multi-node deployments
• add bounded node-worker specifications with deterministic GPU, rank, rendezvous, stagger, and port ownership
• preflight ports on remote deployment nodes before launch, and route readiness checks and logical endpoints to remote backend hosts
• propagate follower or lane failure across a deployment with sibling cleanup and --kill-on-bad-exit=1
• separate validated manifest records, server composition, distributed command construction, and node supervision into focused public modules
• map absolute model paths through the resolved container mount for coordinated and remote-node commands
• preserve the merged retry-plan binding and effective-resume arguments while composing multi-node runtime setup

🧪 Testing

• 16 passed — conflict-focused bootstrap, entrypoint, and shell-runtime tests after the base restack
• 1,438 passed — full packages/data-designer-slurm/tests suite after the base restack
• make check-slurm
• git diff --check
• Earlier branch validation built the data-designer-slurm wheel, verified the distributed runtime modules and shell assets, installed it in isolation, and imported the public runtime types
• real-cluster multi-node E2E proof remains in the joint Slurm acceptance lane

✅ Checklist

• Follows commit message conventions
• Commits are signed off (DCO)
• New behavior has focused regression coverage
• Architecture docs updated (not applicable; this implementation follows the reviewed Implement Slurm submission and allocation runtime #868 ownership boundary)

---

Description updated with AI

[greptile-apps]

Confidence Score: 4/5

The PR is not yet safe to merge because ambiguous array submissions can fail recovery while scheduler task rows are still becoming visible.

Findings

1.  Sampler failure disables backpressure ▶

Fix with agent prompt

### Issue 1
packages/data-designer-slurm/src/data_designer/slurm/runtime/backpressure.py:123-126
If a vLLM or Prometheus metrics collector raises during sampling, the exception terminates this daemon thread while `_thread` remains non-`None`, so later requests cannot restart it. Once the cached snapshot becomes stale, admission permanently fails open and the configured queue limit stops producing 429 responses.

```suggestion
    def _sample_forever(self) -> None:
        while True:
            try:
                self.sample_once()
            except Exception:
                pass
            time.sleep(self.settings.poll_interval_seconds)
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

• Resolves allocation hosts and pins runtime roles to planned nodes.
• Builds bounded node-worker specifications for coordinated distributed vLLM launches.
• Adds remote port preflight, backend routing, and multi-host readiness checks.
• Adds persisted retry, scheduler reconciliation, and collection state machinery.
• The previous queue-sampler finding is fully fixed: ordinary sampling failures now produce a fresh unavailable snapshot without terminating the polling thread.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Slurm allocation starts] --> B[Resolve planner-ordered hosts]
    B --> C[Prepare validated runtime manifest]
    C --> D[Run server port preflight]
    D --> E[Launch one node worker per deployment node]
    E --> F[Start node-local vLLM lanes]
    F --> G[Probe remote backend readiness]
    G --> H[Start logical endpoints on client host]
    H --> I[Run generation client]
    I --> J[Persist candidate and finalize winner]
    E -->|lane or follower exits| K[Terminate sibling lanes and fail deployment step]

Loading

_{Reviews (11) · Last reviewed commit: "adapt retry bootstrap coverage"}

[greptile-apps]

Sampler failure disables backpressure

If a vLLM or Prometheus metrics collector raises during sampling, the exception terminates this daemon thread while _thread remains non-None, so later requests cannot restart it. Once the cached snapshot becomes stale, admission permanently fails open and the configured queue limit stops producing 429 responses.

Suggested change

	def _sample_forever(self) -> None:
	while True:
	self.sample_once()
	time.sleep(self.settings.poll_interval_seconds)
	def _sample_forever(self) -> None:
	while True:
	try:
	self.sample_once()
	except Exception:
	pass
	time.sleep(self.settings.poll_interval_seconds)

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/data-designer-slurm/src/data_designer/slurm/runtime/backpressure.py
Line: 120-123

Comment:
**Sampler failure disables backpressure**

If a vLLM or Prometheus metrics collector raises during sampling, the exception terminates this daemon thread while `_thread` remains non-`None`, so later requests cannot restart it. Once the cached snapshot becomes stale, admission permanently fails open and the configured queue limit stops producing 429 responses.

```suggestion
    def _sample_forever(self) -> None:
        while True:
            try:
                self.sample_once()
            except Exception:
                pass
            time.sleep(self.settings.poll_interval_seconds)
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[nabinchha]

Fixed in 77933cc. QueueBackpressureController.sample_once now converts ordinary reader failures into a fresh unavailable snapshot, so admission fails open for that sample and the daemon continues polling; the next successful sample restores queue-limit rejection. BaseException is intentionally not caught, preserving process-control and shutdown signals. Added regression coverage for failure, fail-open behavior, recovery, and KeyboardInterrupt propagation. Validation: 5 focused tests and 1,235 full Slurm tests passed; check-slurm and focused strict complexity checks pass.

[andreatnvidia]

Could we handle PYTHONPATH explicitly here? It is valid in server.environment, but this assignment silently replaces it. A deployment using an approved mount for a custom parser or plugin will validate and then fail at startup because its module path disappears. Either prepend the runtime bundle path or reject PYTHONPATH during config validation, with a regression test for the collision.

[nabinchha]

Addressed in b1de388. The staged runtime bundle path is now prepended while the deployment-configured PYTHONPATH value is preserved unchanged behind it. Added a regression covering multiple configured plugin/parser entries. Validation: 19 focused runtime tests and all 1,273 Slurm tests pass; make check-slurm and git diff --check also pass.

[andreatnvidia]

urlsplit(...).hostname lowercases hostnames, while allowed_hosts keeps the spelling returned by scontrol. That makes an accepted host such as Compute-001 fail this check and prevents the proxy from starting. Normalizing both sides before comparison, plus a mixed-case test, should cover it.

[nabinchha]

Addressed in b1de388. Backend parsing now compares the URL hostname and scheduler allow-list using casefolded forms, while retaining the parsed canonical hostname for the connection. Added a mixed-case scheduler-host regression. Validation: 19 focused runtime tests and all 1,273 Slurm tests pass; make check-slurm and git diff --check also pass.

[andreatnvidia]

This picks uni whenever TP is 1, but the planner also supports TP=1 with PP>1. In that setup, vLLM's uni executor only initializes rank 0, so the nodes won't form the pipeline-parallel group. I think we should use uni only when TP x PP is 1, or use mp throughout this distributed path. A TP=1/PP=2 test would be useful too.

[andreatnvidia]

One more preflight gap: absolute model paths are mapped into the container, but we never check that the mapped path actually exists and is readable on each assigned node. A stale or node-local mount would pass preflight and fail only after vllm serve starts, while #868 calls for path failures before server launch. Let's carry the required model path in the node spec and validate it here, including missing and unreadable cases.

[andreatnvidia]

Just a couple more things I found in a final review, both around distributed startup behavior. Once those are addressed, I think this is good to go.