Add the services: block for sidecars and standalone Mooncake stores - #399
Closed
ishandhanani wants to merge 3 commits into
Closed
ishandhanani wants to merge 3 commits into
ishandhanani wants to merge 3 commits into
Conversation
ishandhanani
marked this pull request as ready for review
September 7, 2026 19:16
ishandhanani
requested review from
alec-flowers,
csahithi and
nlevin-ui
as code owners
September 7, 2026 19:16
Closed
One top-level list for every long-running process launched next to the job. Each entry has a type that selects a registered ServiceKind (@register_service, like @register_benchmark): generic launches exactly the argv written; mooncake-store runs a standalone Mooncake Store wired to the managed master. The kind supplies defaults (command, start phase, criticality) and the env it injects; ServiceStageMixin launches every kind the same way: placement.node -> physical nodes (head, infra, prefill, decode, agg, workers), optional clone/build of an immutable git source, one srun per node, optional TCP readiness gate, ManagedProcess into the shared registry. start: before_workers runs after the Mooncake master; after_frontend (default) runs once the frontend is healthy. Folds the two open sidecar PRs onto the 2.0 shape: the generic sidecar with source/build_command/inherit_discovery_env/critical from #374 (its --bash direct-runner half is gone with that path), and the standalone Mooncake Store services from #265 as a typed service instead of a backend.mooncake_kv_store.standalone sub-block with a per-role placements map. Per-role segment sizes are now two entries placed on prefill and decode; two services on one node with the same port are rejected before launch. Adds examples/features/services.yaml (an HTTP log browser gated on its port), docs/services.md, a config-reference section, dry-run output, the regenerated schema reference, and tests for the schema, kinds, and stage.
The global-bool shortcut only matched prefill and decode, so an aggregated topology never got --kv-events-config and the sidecar's cache-overlap score stayed at zero. Carried over from #374 with its regression test.
… on death Close the windows where a service process could run untracked: - start_services takes the ProcessRegistry and registers each service process the moment its srun exists, not after the whole stage returns. A SIGTERM during a readiness wait now finds every launched process in the registry; the stage also terminates what it started on any BaseException before re-raising. - The one-shot clone and build sruns are ManagedProcesses in the registry too (non-critical), and wait under a wall-clock bound: the clone by the sum of its per-command git timeouts, the build by a new build_timeout_seconds (default 1800). A step that overruns is killed and the job fails pointing at its log instead of holding the allocation. - The readiness gate polls in short slices and checks the process between them, so a service that dies before opening its port fails at once with its exit code rather than after the full timeout. Documented under a Cleanup section in docs/services.md.
ishandhanani
force-pushed
the
idhanani/srt2-11-services
branch
from
September 8, 2026 03:54
bfc32bd to
2c868fa
Compare
Collaborator
Author
|
Consolidated into #407, which carries these same commits as one PR against main (this description is reproduced there as one of its parts). Closing to keep review in one place; the branch stays for per-step reference via stack #398. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #395 (Track 2 step 7 of #385). Draft until the stack below it merges.
What
One top-level
services:list for every long-running process launched next to the job: an experimental router built from a PR, a standalone Mooncake Store per worker node, a debugging HTTP server. Each entry has atypethat selects a registeredServiceKind(@register_service, same pattern as@register_benchmark):generic(default): launches exactly the argv written. Starts after the frontend, non-critical.mooncake-store: standalone Mooncake Store wired to the managed master. Default command, starts before workers, critical, container falls back tomooncake_kv_store.container,MOONCAKE_MASTER/MOONCAKE_TE_META_DATA_SERVER/MOONCAKE_LOCAL_HOSTNAMEinjected.The kind supplies defaults and the env it injects.
ServiceStageMixinlaunches every kind the same way:placement.noderesolves to physical nodes (head,infra,prefill,decode,agg,workers), optional clone and build of an immutable gitsource, onesrunper node, an optional TCPreadinessgate, and aManagedProcessin the shared registry for crash detection and teardown.Folds two open PRs onto the 2.0 shape
auxiliary_services): the generic sidecar withsource/build_command/inherit_discovery_env/critical, the head-node clone-on-bare-host then build-in-container flow, and the dry-run panel. Its--bashdirect-runner half is gone with that path ([2.0] Remove the --bash direct-host execution path #386). Renamed toservices:withcontainer(alias-resolved by the shared walker) instead ofcontainer_image. Its unrelatedkv_events_config: trueaggregated-mode fix is carried as its own commit with the regression test.mooncake_kv_store.standalone): the standalone Store services, as a typed service instead of a backend sub-block with aplacements: {prefill, decode, aggregated}map. Per-role segment sizes are two entries placed onprefillanddecode;preamble,cpus_per_task,cpu_bind,srun_options, the{node}/{node_ip}/{role}placeholders, and the TCP readiness check all carry over. Two services that listen on the same port on one node are rejected before launch (the co-location conflict feat(mooncake): manage standalone store services #265 checked). The worker-sidebackend.mooncake_kv_store(master, worker env, vLLMstore_config) is unchanged.Both external PRs can close in favour of this one once it lands; credit to their authors in the file history.
Also
examples/features/services.yaml: a runnable HTTP log browser on the head node gated on its port.docs/services.md, a## servicessection indocs/config-reference.md, a pointer section indocs/mooncake-kv-store.md, CLAUDE.md notes, regenerateddocs/schema-reference.md.tests/test_services.py(schema, kinds, stage: placement resolution, env layering, source build, readiness failure teardown, port collisions), dry-run cases.Cleanup guarantees
Nothing a service launches outlives the job. Every srun the stage starts, including the one-shot clone and build steps, is registered with the
ProcessRegistrythe moment it exists (not when the stage returns), so cleanup on normal exit, a failed stage, the SIGTERM handler, and the crash monitor all reach it. The stage also terminates what it started on any exception, includingSystemExitfrom the signal handler. The clone step is bounded by its per-command git timeouts and the build bybuild_timeout_seconds(default 1800); a step that overruns is killed. A service that dies before its readiness port answers fails at once with its exit code instead of waiting out the timeout. Slurm step cancellation kills the whole step cgroup, so forked children inside the container go with the service.Validation
examples/features/services.yamlran as job 11995: thelog-browserservice launched on the head node, itsreadinessgate on port 9911 passed after 21s, the benchmark completed with rollups written, and the service was torn down with the job on the cleanup path; themooncake-storeshape dry-run validated on a disagg recipe (no Mooncake image on that cluster to run it).sacctshows the service step cancelled with the workers and router at teardown; job 12005 wasscancelled while the service was up, every step ended (service stepCANCELLED), and a check on the compute node afterwards found nohttp.server, no engine processes, and no steps still allocated.