Skip to content

feat: opt-in RESTART_POLICY for boot persistence - #2

Closed
sethforprivacy wants to merge 1 commit into
MiaAI-Lab:mainfrom
sethforprivacy:feat/restart-policy
Closed

sethforprivacy wants to merge 1 commit into
MiaAI-Lab:mainfrom
sethforprivacy:feat/restart-policy

Conversation

@sethforprivacy

Copy link
Copy Markdown
Contributor

Companion QoL to the API_KEY PR, from the same production deploy:

The serving containers run with docker's default restart policy, so a host reboot silently takes the deployment down until someone reruns ./start.sh. For a recipe that positions itself as production ("a single-file, production recipe"), surviving reboots seems worth offering.

  • RESTART_POLICY= env (default no = exact current behavior)
  • unless-stopped brings both containers back after a clean reboot of the pair, and the TP rendezvous re-forms
  • docs are explicit about the single-side-crash caveat: a restarted rank re-enters rendezvous against a peer that never died, which may not recover — ./start.sh (idempotent) remains the coherent-relaunch path for that case, which is also why this stays opt-in rather than default

Happy to fold both PRs into one if you prefer fewer merges.

The serving containers currently run with docker's default restart policy
(no), so a host reboot silently takes the deployment down until someone
reruns ./start.sh. RESTART_POLICY=unless-stopped brings both containers back
after a clean reboot of the pair, and the TP rendezvous re-forms.

Default stays 'no' (exact current behavior). The docs are explicit about the
single-side-crash caveat: a restarted rank re-enters rendezvous against a
peer that never died, which may not recover -- ./start.sh (idempotent)
remains the coherent-relaunch path for that case.
@sethforprivacy

Copy link
Copy Markdown
Contributor Author

Walking this back somewhat — we've since run into evidence that cuts against my own proposal here, and it would be wrong to let it merge without saying so.

The short version: on a 2-node TP pair, a Docker restart policy on the ranks doesn't restore the service, it creates a wedged pair.

What happened to us: a head node needed a power cycle. Its container carried unless-stopped (applied by hand, which is what motivated this PR), so on boot dockerd restored rank 0 — unordered, and against a leftover rank 1 still running on the worker from before the outage. The pair sat at Init torch distributed begin indefinitely. Recovery meant killing the worker container by hand and then ./start.sh stop && ./start.sh to get the worker-first ordering the script does deliberately.

The reason it can't work is structural: dockerd cannot order two nodes. Worker-first is only expressible from the head, in start.sh. So a per-container restart policy will always restore rank 0 and rank 1 independently, in whatever order they happen to come up, with no barrier between them — and for a TP group that is not a restart, it's a coin flip on whether you get a wedge.

Two related things I'd flag:

  1. no is already this recipe's default, not a divergence. launch uses docker run -d with no --restart flag. So the status quo is arguably correct for a multi-node pair, and my framing of it as a missing feature was wrong.
  2. The half-assembled state is separately undetected, which is why our power-cycle recovery was confusing rather than obvious — cmd_serve short-circuits on the head container alone and exits 0. I've filed that as fix: cmd_serve reports success (exit 0) on a half-assembled pair — head up, worker gone #17; it's the piece I'd actually merge.

Where that leaves this PR, your call:

  • Close it — reasonable, and I won't be offended. On this topology the feature invites the failure it looks like it prevents.
  • Keep it but narrow it, with a documented warning that on --nnodes 2 the policy restores ranks unordered and can produce a wedged pair, so the supported path for boot persistence is host-level lifecycle management that can enforce worker-first.

We went with the second shape locally: a host unit that owns lifecycle and explicitly clears the policy off both ranks, so ordering is always enforced from the head. That's site-specific enough that I'm not proposing it upstream — but it's why I no longer think an opt-in RESTART_POLICY is the right answer for a 2-node pair.

Happy to convert this into a docs-only note if that's more useful than the flag.

@sethforprivacy

Copy link
Copy Markdown
Contributor Author

Closing this — I argued myself out of it above.

On a 2-node TP pair a per-container restart policy doesn't restore the service, it creates a wedged pair: dockerd cannot order two nodes, so rank 0 and rank 1 come back independently with no barrier, and rank 0 then sits in rendezvous against a stale peer. We hit exactly that after a power cycle. And no is already this recipe's default (launch uses docker run -d with no --restart), so framing it as a missing feature was my error.

The half-assembled-pair detection that made our recovery confusing is the part worth having, and that's #17.

We solved boot persistence locally with a host unit that owns lifecycle and explicitly clears the policy off both ranks, so worker-first ordering is always enforced from the head. That's site-specific enough that I'm not proposing it here.

pchar pushed a commit to pchar/Qwen3.8-Flash-Next-Dual-DGX-Sparks that referenced this pull request Sep 21, 2026
pchar pushed a commit to pchar/Qwen3.8-Flash-Next-Dual-DGX-Sparks that referenced this pull request Sep 22, 2026
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.

1 participant