Skip to content

feat(deployment): optionally run orchestrator on an inference node - #2811

Merged
samsja merged 1 commit into
mainfrom
feat/orchestrator-on-inference
Jun 14, 2026
Merged

samsja merged 1 commit into
mainfrom
feat/orchestrator-on-inference

Conversation

@samsja

@samsja samsja commented Jun 14, 2026 •

Copy link
Copy Markdown
Member

Summary

Adds MultiNodeDeploymentConfig.orchestrator_on_inference (default False) to choose where the orchestrator launches:

  • False (default, unchanged): orchestrator runs on trainer rank 0.
  • True: orchestrator runs on the last inference node (a decode node — tail of the last replica, never a router/prefill head).

Why

On large multi-node runs the trainer nodes can be host-RAM-saturated (e.g. 8 ranks of CPU-offloaded optimizer state ≈ 800 GiB). Co-locating the orchestrator there means its rollout buffer + env workers compete for the same host RAM and can exhaust it (we hit a trainer SIGBUS from exactly this). Inference nodes keep weights/KV on GPU and typically have ample free host RAM, so moving the orchestrator there removes the contention. This makes it a per-deployment choice rather than hardcoding either placement.

Implementation

The orchestrator launch is now a single top-level block in multi_node_rl.sbatch.j2 (after the role if/else, so the existing wait -n teardown still covers it), gated on a flag-selected SLURM_PROCID:

{% if orchestrator_on_inference %}ORCH_PROCID=$((NUM_INFER_NODES - 1)){% else %}ORCH_PROCID=$NUM_INFER_NODES{% endif %}
if [ "$SLURM_PROCID" -eq "$ORCH_PROCID" ]; then ... fi

Both placements share one code path — no duplicated launch logic. NUM_INFER_NODES maps to trainer rank 0 (since TRAIN_NODE_RANK = SLURM_PROCID - NUM_INFER_NODES), NUM_INFER_NODES-1 is the last inference node. No effect when num_infer_nodes == 0.

Validation

Rendered the template both ways and bash -n'd the output — both valid; orchestrator launches exactly once in each, on the expected node.


Note

Medium Risk
Changes where a critical RL control process runs in multi-node SLURM jobs; default placement is unchanged, but enabling the flag shifts rollout/env RAM load onto an inference node that also runs vLLM.

Overview
Adds deployment.orchestrator_on_inference (default False) on multi-node RL configs so SLURM jobs can place the orchestrator on the last inference node instead of trainer rank 0, to avoid host-RAM contention with heavy trainer CPU offload.

The multi_node_rl.sbatch.j2 launch is refactored: orchestrator startup is no longer inside the trainer-only branch on rank 0, but a single shared block after the infer/train role split that picks ORCH_PROCID as either NUM_INFER_NODES (unchanged default) or NUM_INFER_NODES - 1 when the flag is set—still gated on num_infer_nodes > 0 and still covered by wait -n teardown.

write_slurm_script passes the flag into both standard and disaggregated template renders.

Reviewed by Cursor Bugbot for commit febd91c. Bugbot is set up for automated code reviews on this repo. Configure here.

Adds MultiNodeDeploymentConfig.orchestrator_on_inference (default False). When
False, the orchestrator launches on trainer rank 0 (unchanged behavior). When
True, it launches on the last inference node (a decode node) instead — useful
when trainer nodes are host-RAM-saturated (large CPU-offloaded optimizer state)
and the orchestrator's rollout buffer + env workers would otherwise exhaust
memory there; inference nodes keep weights/KV on GPU with ample free host RAM.

The orchestrator launch is now a single top-level block in multi_node_rl.sbatch.j2
gated on a flag-selected SLURM_PROCID (NUM_INFER_NODES for trainer rank 0, or
NUM_INFER_NODES-1 for the last inference node), so both placements share one code
path. No effect when num_infer_nodes == 0.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@samsja
samsja marked this pull request as ready for review June 14, 2026 18:47
@samsja
samsja merged commit 060f25b into main Jun 14, 2026
28 of 29 checks passed
faresobeid added a commit that referenced this pull request Oct 12, 2026
…3821)

Stacked on #3811, which is stacked on #3806. Move pruning, serialization
and shipped-payload release off the orchestrator event loop.

- Zero-advantage pruning runs in a worker after selection detaches
samples; queue/cohort mutations stay on the loop. `process_batch` and
its callers are async.
- `BatchSender.encode` encodes one microbatch at a time under the same
MessagePack array header. Output bytes and ZMQ frames are unchanged; ZMQ
encoding runs in a worker.
- Shipped sample, episode and microbatch lists are released
incrementally by a worker. At most one release remains pending, and
teardown awaits it. Explicit `gc.collect()` is removed from
`trim_process_memory`; this PR does not disable automatic GC.

The combined private smoke also uses Verifiers #2811, which disables
automatic GC. That integration retains explicit collection in a worker
to avoid having neither collection mechanism. Its live A/B is therefore
a comparison of the combined private runtime, not proof of the
standalone GC-removal effect.

Validation of the adapted layer: 266 CPU tests passed, two skipped,
across orchestrator, transport, utilities and config tests; Ruff
lint/format and diff checks passed. The integrated candidate replay
matched deterministic metrics, sample hashes and cohort membership.
Four-node, ten-update live comparison is scheduled; results are pending.

Earlier synthetic benchmark (before the #3806 API adaptation; CPU-only,
4,096 samples × 32k tokens, eight ZMQ ranks, one run per variant):

| Phase | Original wall / max pause | Earlier PR wall / max pause |
|---|---|---|
| Pruning | 10.9 s / 11.0 s | 11.9 s / 1.1 s |
| Packing, already threaded | 23.4 s / — | 23.6 s / 1.1 s |
| Sending | 6.6 s / — | 7.0 s / 0.09 s |
| Background release/trim | — | 6.1 s / 0.02 s |
| Ship-path total | 52.9 s | 43.2 s |
| Worst loop pause | 21.6 s | 1.1 s |

Those historical numbers are not a fresh benchmark of this adapted diff.
Automatic cyclic GC and GIL-held operations can still pause the loop.
#3822 adds queue draining while these ship phases run.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Touches the hot train-ship path and memory-release timing; wire format
is preserved but concurrency and teardown ordering change.
> 
> **Overview**
> Moves more of the **train ship path** off the asyncio event loop so
long GIL-held work no longer stalls dispatch and rollout handling.
> 
> **TrainSink** makes `take_batch` / `process_batch` async and runs
zero-advantage pruning in a worker thread after selection; queue/cohort
mutations stay on the loop. Call sites (orchestrator loop and drain
handling) now `await` `take_batch`.
> 
> **Batch transport** adds `BatchSender.encode` to MessagePack-encode
micro batches one at a time (same array header and bytes as a single
bulk encode). Filesystem sends use it directly; **ZMQ** encodes each
rank’s grid in `asyncio.to_thread`.
> 
> After each shipped batch, the orchestrator no longer calls
`trim_process_memory` synchronously on the loop. It **awaits a shielded
prior `release_future`**, then schedules `release_and_trim` in the
default executor to incrementally clear shipped
sample/episode/microbatch lists and call `malloc_trim`. Teardown awaits
any pending release. Shutdown still trims via `asyncio.to_thread`.
**`gc.collect()` is removed** from `trim_process_memory`.
> 
> Unit tests were updated to `await` `take_batch`.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
06dd796. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Co-authored-by: Hubert Marek <163992334+hubert-marek@users.noreply.github.com>
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