[nabinchha]

📋 Summary

Detect required-backend health loss during generation and persist it in managed readiness, even when serving processes remain alive. Give logical endpoints their drain window before stopping backends, and complete routing, connection, and timeout telemetry.

🔗 Related Issue

Closes #1103, the remaining scope transferred from #966. Live overload acceptance is part of this PR; the broader release matrix remains separately tracked in #870.

🔄 Changes

• Add a CPU-only runtime health monitor that records aggregate backend counts and fails the attempt on service loss; retain its failure evidence through finalization and reject winner publication after a recorded health failure.
• Preserve terminal readiness when failure cleanup follows unsuccessful winner publication.
• Give health probes separate connection capacity so inference saturation cannot starve readiness checks or distort least-connections activity.
• Stop monitoring first, send TERM directly to each endpoint task with scancel using its validated allocation/step identity, and keep launchers and backends alive for the drain window. Force remaining endpoints to stop only after the 12-second allowance, then terminate other steps.
• Log each attempted upstream, connection/reuse timing, response-header and total attempt timing, and incomplete streams without payloads or credentials. Enable these events in production.
• Count expired, stale, and discarded upstream connections, plus terminal and streaming timeouts without requiring another retry.
• Add real-process cleanup tests, live loopback health/streaming/pool tests, and an eight-replica overload test through the actual HTTP adapter and request-admission controller. Update Slurm troubleshooting.

🔍 Attention Areas

• Eviction accounting overrides three narrow aiohttp 3.x connector hooks because aiohttp exposes no public eviction trace signal. Regression coverage exercises idle expiry, stale reuse, and discarded connections.
• Runtime service loss fails the attempt; it does not silently accept a deployment with fewer healthy replicas.

🧪 Testing

• Regression coverage for late health-failure success races, terminal-readiness failure cleanup, task-directed signals with launcher TERM → task KILL behavior, identity validation, and bounded escalation.
• Final full Slurm suite: 2,123 passed, 1 skipped. Focused proxy, health, and admission suite: 54 passed.
• Real-client partial overload succeeds through failover; total overload stops after three attempts, preserves Retry-After: 3, reduces admission from 8 to 4, and applies cooldown.
• Real Slurm CPU allocation: active HTTP responses finish during completion, failure cleanup, and controller cancellation using production launch/cleanup helpers and real srun/scancel. All three endpoint tasks exit 0; the allocation completes and releases its steps. Sanitized drain evidence.
• Bash syntax, git diff --check, and changed-file Ruff lint/format checks pass.
• Repository Ruff checks pass except an unchanged import/formatting issue in docs/colab_notebooks/8-generating-word-documents.ipynb.
• Fern internal links: 101 pages checked. Fern check: 0 errors; missing-redirects validation skipped because Fern authentication is unavailable.
• Full make test across all packages was not run.
• Live vLLM 0.22.0 on two GPUs, using four wheels built from f499303a: 32/32 persisted records, 22.325 seconds, 86.003 records/minute; 33 HTTP 200 responses, zero failures/retries/timeouts, 31 reused connections.
• Actual queue depths 7 (partial) and 7/7 (total): partial overload fails over successfully; total overload preserves Retry-After: 3, reduces admission 8 → 4, applies cooldown, then recovers. Proxy peak RSS was 38.34 MiB; measured CPU time and per-case metrics are recorded in the sanitized acceptance evidence.
• Source, wheel, runtime-source and dependency-lock digests recorded; Slurm wheel and public acceptance artifacts pass the public-artifact audit. Live tests use the supported queue-threshold override of 0; the default remains 128. The broader release matrix remains under Harden the Slurm runtime and validate sealed release artifacts #870.

GPU overload acceptance remains tied to f499303a; proxy, health-monitor, backpressure, and overload-harness source are unchanged. Later attempt-finalization fixes have regression coverage. The updated shell signal path has separate live Slurm validation with a synthetic streaming HTTP backend and recorded source hashes; it does not run the full managed slurm execute flow.

✅ Checklist

• Follows commit message conventions.
• Commit is signed off (DCO).
• Internal contributor; external triaged-issue gate is not applicable.
• Relevant Slurm user documentation updated; no Slurm architecture page exists under architecture/.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; the previous health-failure race is fixed, and no new blocking issues were found.

Summary

This PR completes Slurm health monitoring, shutdown ordering, and proxy telemetry.

• Slurm attempts track required backend health while generation runs.
• Slurm endpoints drain active requests before their backends stop.
• The Slurm proxy keeps health checks clear of inference traffic and reports each request attempt.

Diagram

sequenceDiagram
    participant Controller
    participant Monitor
    participant Endpoint
    participant Backend
    participant State
    Controller->>Monitor: Stop monitoring
    Controller->>Endpoint: Send TERM to recorded job.step
    Endpoint->>Backend: Finish active responses
    Note over Controller,Endpoint: Allow 12 seconds for the 10-second drain
    Controller->>Endpoint: Stop any remaining launcher
    Controller->>Backend: Stop remaining steps
    Controller->>State: Read final health evidence
    alt Health failure recorded
        Controller->>State: Mark attempt failed
    else No health failure
        Controller->>State: Publish winner
    end

Loading

_{Reviews (3) · Last reviewed commit: "fix(slurm): signal endpoint tasks before..." · Reviewed by Greptile}

[greptile-apps]

Health failure still permits success

After the client's last dd_require_running check, the monitor can save FAILED readiness before cleanup stops it. dd_stop_steps ignores its exit status, and the controller then runs succeed. _succeed replaces readiness with STOPPED and publishes the winner without checking the recorded failure. An attempt can therefore succeed despite a recorded loss of required backend health.

After stopping the monitor, check its saved readiness before allowing success.

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/data-designer-slurm/src/data_designer/slurm/runtime/cleanup.sh
Line: 49-50

Comment:
**Health failure still permits success**

After the client's last `dd_require_running` check, the monitor can save `FAILED` readiness before cleanup stops it. `dd_stop_steps` ignores its exit status, and the controller then runs `succeed`. `_succeed` replaces readiness with `STOPPED` and publishes the winner without checking the recorded failure. An attempt can therefore succeed despite a recorded loss of required backend health.

After stopping the monitor, check its saved readiness before allowing success.

---

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

[nabinchha]

Fixed in 033b5ae. After cleanup stops the monitor, _succeed loads its final persisted readiness and rejects winner publication if it contains a health failure, including failure evidence retained in STOPPED readiness. Failure cleanup also preserves terminal readiness so the attempt can be marked failed without an invalid STOPPED → FAILED readiness transition.

Regression tests cover a late health failure with a valid candidate output, retained failure evidence after readiness stops, and failure cleanup after winner publication fails. Validation: full Slurm suite 2,116 passed, 1 skipped; focused proxy/health/admission suite 54 passed.

[github-actions]

Fern preview: https://nvidia-preview-pr-1104.docs.buildwithfern.com/nemo/datadesigner

Fern previews include the docs-website version archive with PR changes synced into latest. Notebook tutorials are rendered without execution outputs in previews.

[andreatnvidia]

Requesting changes for the shutdown signal handling discussed inline.

[andreatnvidia]

It looks like these are the srun launcher PIDs, and Slurm forwards SIGTERM as SIGKILL to their tasks. I think that would kill the proxy before it can drain active responses, even with the longer wait. Could we signal the endpoint task directly and cover that path in the test? The current test sends SIGTERM straight to Python, so it seems to miss this behavior.

[nabinchha]

You're right—the launcher PID path bypassed the proxy's drain handler. Fixed in 868268d: each endpoint task records its Slurm job/step identity before executing the proxy, and cleanup validates that identity against the current allocation before sending scancel --signal=TERM to that exact step. Launchers and backends stay alive through the drain window; forced launcher shutdown happens only after the deadline.

The regression test now separates launcher and task processes and models launcher TERM → task KILL forwarding. I also validated the production launch/cleanup helpers with real srun/scancel in a CPU Slurm allocation: active HTTP responses completed during normal completion, failure cleanup, and controller cancellation; all three proxy tasks exited 0. Sanitized live-drain evidence. Full Slurm suite: 2,123 passed, 1 skipped.