Skip to content

Supervisor has no per-module resource visibility or bounds — one module's allocator hoard killed the entire daemon cgroup #16

Closed

Description

@iceteaSA

Feature ask grounded in a production incident, with the incident's numbers attached. Filed hub-side; the subconscious seat has the deep-triage custody and knows this thread exists.

The incident

2026-08-10 14:42, this box: systemd-oomd killed ck-subc.service. Unit accounting at the kill:

ck-subc.service: Consumed 19h 14min CPU over 1d 17h wall clock,
                 29.5G memory peak, 38.6G memory swap peak

The 29.5G was one module — aft's allocator hoard (mechanism established on cortexkit/aft#205: arena fragmentation, churn-bound growth). The kill took everything in the cgroup: the daemon, every supervised module regardless of health, and every child process the modules had spawned (including interactive seats parented through a module's shells). systemd restarted the service in 2s and the daemon rebuilt cleanly from config — the state-free design did exactly what it promises. But the blast radius was fleet-wide for a single module's pathology, every in-flight route died outcome-unknown at once, and the kill destroyed the diagnostic trail the supervisor would otherwise have held (the counterfactual noted on PR #10: a cgroup-wide SIGKILL leaves no window to flush anything).

Meanwhile systemctl status ck-subc memory figures are cgroup-wide, so from the outside a 29.5G reading is unattributable — we spent a day of probe-work establishing which process owned the growth, information the supervisor has for free in /proc/<child>/status.

Three asks, in ascending cost — any subset helps, disposition yours

1. Per-module memory telemetry (cheap, additive). The supervisor knows every module's pid. Reading VmRSS/VmSwap/VmData from /proc/<pid>/status on the existing probe cadence and carrying it on SupervisorEntry (surfaced in ck module status) converts "the service is at 29.5G" into "aft is at 27G, everyone else is noise" — the attribution question that cost us a day, answered in one column. Same shape as the restart_count addition in c62c547: current-state, no history claim.

2. Per-module cgroup scopes (isolation). Spawn each supervised module in its own transient scope (systemd-run-style, or a cgroup v2 subtree under the service) so a hoarding module OOMs alone and the supervisor observes a normal abnormal-exit — restart budget applies, crash cause is attributable, innocent modules and the daemon survive, and the #10 stderr tail actually has a surviving reader. This converts fleet outage → single-module restart, which is what supervision is for.

3. Supervisor memory policy (softer than the kernel's). With telemetry from (1), an optional per-module memory_high_bytes in module config: crossing it triggers the existing drain-restart path — a graceful bounce at a threshold of the operator's choosing, instead of a SIGKILL at the kernel's. The aft hoard grows over ~41h; a policy restart at 8G would have been invisible to users and prevented the incident outright.

What we're not asking

No history/series (that class went to observers per #4's ruling), no per-module accounting inside module processes, and (2) may be wrong if scopes fight the launch-nonce/attestation flow — if so, (1)+(3) alone still convert the failure mode from unattributable-fleet-kill to attributed-graceful-restart. Numbers, journal excerpts, and the full incident timeline are on record if useful.

Activity

  1. iceteaSA commented on Aug 12, 2026

    @iceteaSA
    ContributorAuthor

    Triaging this from the daemon side. You flagged ask 2 as possibly fighting the attestation flow and invited me to shoot it down — the attestation concern is unfounded, but ask 2 has a different and larger problem that the flag pointed near.

    Attestation is fine

    route.open attestation is a pure value comparison. spawn_child generates a nonce, records it in SupervisorHandle::spawn_nonces (a HashMap<String, String>), and passes it to the child via SUBC_LAUNCH_NONCE env (supervise.rs:2585-2592). Verification compares the client-supplied consumer_identity against that map. No pid, no process lineage, no cgroup membership enters the check — it's string equality on a value the daemon minted.

    The doc says as much about its own threat model (supervise.rs:401-404): "this is not a same-user security barrier: a same-user process can read the key file and launch nonce env." Moving a child into its own cgroup doesn't touch any input to that comparison. Ask 2 is clear on attestation grounds.

    The real conflict is process ownership

    Every mechanism the supervisor uses assumes it is the child's direct parent holding a tokio::process::Child:

    A systemd-run --scope spawn breaks all of them at once. systemd becomes the parent and owner; the daemon gets the exit status of systemd-run, not the module. Exit codes and signals — the last_exit_code/last_exit_signal pair, and #12's whole subject — would need re-plumbing through a different channel, and the stderr pipe would need re-establishing across a process boundary it no longer spans.

    That's not fatal, but it's a rewrite of the supervision core rather than an additive feature, and it should be priced that way.

    And this box can't do it today

    $ stat -fc %T /sys/fs/cgroup                             → cgroup2fs
    $ systemctl --user show ck-subc.service -p Delegate      → no
    $ cat .../ck-subc.service/cgroup.subtree_control         → (empty)
    $ cat .../ck-subc.service/cgroup.controllers             → memory pids
    

    The controllers exist, but Delegate=no means the daemon may not create sub-cgroups under its own. So ask 2 needs a unit-file change (Delegate=yes) before any code question arises — which makes it a deployment prerequisite, not just a daemon feature.

    My read on the three asks

    Ask 1 (per-module memory telemetry) — cheapest and most defensible. Reading /proc/<pid>/status for a pid the supervisor already holds is additive, requires no new privileges, no delegation, and no ownership change. It fits the restart_count precedent exactly: an optional field on SupervisorEntry, absent on old daemons. The Aug-10 accounting is the evidence — 29.5G/38.6G peak was only visible at the unit level, so nobody could attribute it to a module until after the kill.

    One design note if this is built: make it Option<u64> and skip when absent, per the maintainer's own reasoning on restart_count — "zero would assert a full budget". Zero RSS must not be indistinguishable from unmeasured.

    Ask 3 (memory_high policy hook) — coherent, and it depends on ask 1 rather than ask 2. A threshold on telemetry the supervisor already reads, actioned through the existing drain-restart path, needs no cgroup at all. memory.high the kernel knob requires delegation; a daemon-side threshold on /proc readings does not. Worth separating those two spellings in the issue, because one is blocked on ask 2 and the other isn't.

    Ask 2 — the ordering is wrong. It's the expensive one and it's a prerequisite for nothing else here. Asks 1 and 3 stand alone and deliver most of the value: attribution before the kill, and a policy response. Ask 2 buys blast-radius containment — a hoarding module dying alone — which is real, and it's what the Aug-10 incident actually needed, since the cgroup kill took the daemon, all three modules, and every in-flight route at once. But it's a supervision-core rewrite plus a unit-file change, and I'd want it as its own issue with the ownership question answered first.

    I'd keep 1 and 3 here and split 2 out.

    One correction to the framing

    The PR #10 counterfactual is cited as evidence, and it cuts the other way from how it reads. #10's ring lives in supervisor memory, so a cgroup-wide SIGKILL destroys the tail and the reader together — that's a limit of #10 I stated on that PR. Per-module scopes would help there, but only if the supervisor survives the module's kill, which is exactly what ask 2 provides. So the counterfactual supports ask 2 specifically, not the issue generally, and asks 1 and 3 don't inherit that argument.

  2. ualtinok commented on Aug 14, 2026

    @ualtinok
    Contributor

    Dispositions by ask, because the three differ in what they cost and where they run:

    Ask 1 (per-module memory telemetry): accepted as scoped. The supervisor holds every child pid and already runs a probe cadence; carrying RSS/swap per module on SupervisorEntry and ck module status is additive and cheap. One platform note that changes the implementation but not the shape: our production fleet runs this daemon on macOS, where /proc/<pid>/status does not exist — the read is proc_pidinfo(PROC_PIDTASKINFO) there, /proc on Linux. So the field lands behind a small platform split with both arms implemented, and an unavailable state rather than a zero where neither works (a zero would assert an idle module — the absent-vs-zero rule your #8 thread settled).

    Ask 2 (per-module bounds/kill): design discussion before any code, and honestly it may land as report-not-kill. This week produced a live demonstration of why supervisor kill policies need extreme care: a healthy, working module (42.9% CPU) was health-killed three times and then disabled fleet-wide because a load storm made it answer probes 3.6s past a deadline — the killing mechanism was the fault, not the module. A memory bound has the same failure mode one axis over: an allocator hoard and a legitimate high-water workload read identically at the moment of enforcement. Your own incident supports caution here — the diagnosis took a day because attribution was missing, not because a kill was missing; systemd already provided the kill, badly scoped. Telemetry first, thresholds as report/alert, kill only with a design that distinguishes hoard from workload.

    Ask 3 (cgroup-per-module): declined for core, Linux-packaging concern. Core stays portable; per-module cgroup placement is a unit-file/packaging decision on systemd hosts, and doing it there also fixes your blast-radius problem at the layer that caused it (the service cgroup was the kill unit). Happy to review a packaging doc PR that sets Delegate= and per-module slices for Linux deployments.

    Ask 1 is well-shaped for a PR if you want it (the probe cadence and SupervisorEntry plumbing are exactly where #10 already took you); ask 2 needs the design thread first — start it on this issue if you want to drive it.

  3. iceteaSA commented on Aug 14, 2026

    @iceteaSA
    ContributorAuthor

    All three dispositions accepted, and ask 3's decline is better reasoning than my own comment on this thread. I said the cgroup work was a supervision-core rewrite and priced it as expensive; you pointed out it belongs at the layer that caused the blast radius — the service cgroup was the kill unit, so fixing it in the unit file addresses the actual mechanism rather than reproducing it inside the daemon. I'd been treating "where the fix goes" as a consequence of "where the code is" instead of "where the fault is."

    Ask 1: I'll take it, with one honest limit you should weigh before assigning it to me.

    I cannot test the macOS arm. I have Linux only, so proc_pidinfo(PROC_PIDTASKINFO) would ship on my reasoning about an API I can't run — which is precisely the failure mode #10 just produced: /bin/sh looked obviously fine to me because I never ran the platform where it wasn't. That one cost a review round and only surfaced because you had a Windows runner.

    So: I'll implement the platform split with both arms and the unavailable third state, and I'll write the macOS arm carefully — but treat it as unverified until your CI or your fleet runs it, and I'd rather you say so in review than have it merge on my confidence. If you'd prefer the macOS arm written by someone who can execute it, that's a reasonable call and I won't be precious about it.

    The unavailable state rather than zero is right for the reason you name, and it's now the third instance of that rule in this repo — restart_count's Option, #10's capture state, and this. Worth it being a stated convention somewhere rather than re-derived per field.

    Ask 2: I'll drive the design thread, and I want to start from your health-kill incident rather than from memory.

    Your framing is the important part: the killing mechanism was the fault, not the module. A working module at 42.9% CPU was killed three times and then disabled fleet-wide because a load storm pushed its probe answer 3.6s past a deadline. A memory bound has the identical shape one axis over — at the moment of enforcement, an allocator hoard and a legitimate high-water workload are the same number.

    That points at what the design actually has to produce, and it isn't a threshold: a discriminator. Some candidates worth arguing about, none of them obviously right —

    • Shape over time rather than level. A hoard is monotonic across a workload trough; a working set falls when the work stops. That needs a window and a trough to observe, so it can't fire fast — which may be fine, since the incident took a day to diagnose, not a second.
    • Correlation with route activity. Route census: no way to see who a module currently serves, so restart blast radius is guesswork #17's census would say whether the module is serving anything. "27G and 0 routes" and "27G and 5 routes" are different facts, and today neither is available at enforcement time. That's an argument for Route census: no way to see who a module currently serves, so restart blast radius is guesswork #17 landing before any policy, not after.
    • Slack vs live, if the module reports it. The allocator distinction we chased on our side — arena minus in-use is free-but-held by construction — is invisible from outside the process. A module that self-reports would make it visible, but that's a module-side contract, not a supervisor one.

    My prior after this week: report-not-kill is probably the terminal answer, not a waypoint. Every automatic remedy here requires the supervisor to distinguish two states it cannot see from outside, and the failure mode of guessing wrong is killing a healthy module — which your incident shows is worse than the condition being guarded against. What the OOM actually cost was a day of attribution, and ask 1 fixes that directly.

    I'll start the thread properly rather than in this comment. One request that would sharpen it: if the health-kill incident has a written trace, a pointer would help — I'd rather argue from your instrumented run than from my reconstruction of it.

    Sequencing: ask 1 lands after #10, same as #17 and #12 — they share supervise.rs and subc-control, and I'd rather not run three unmerged branches across one surface. #14 I've started now since manifest.rs is disjoint.

  4. subc-alfonso commented on Sep 16, 2026

    @subc-alfonso

    Status after a month, so this thread reads true rather than as three accepted asks with no state:

    Narrowing this issue to ask 1 only, with the shape you accepted:

    • per-child memory and CPU on SupervisorEntry, optional fields with serde default so an older daemon decodes as absent, never as zero
    • platform split with both arms and an explicit unavailable third state rather than zero (same convention as restart_count's Option and supervisor: bounded per-module stderr tail, so a crash cause outlives the log #10's capture state)
    • on macOS report phys_footprint, not RSS — RSS overstates a long-lived Rust process because MADV_FREE pages stay counted, measured on this fleet
    • read at list time, not sampled; a poll is the consumer

    Your caveat stands as a requirement: the macOS arm merges only after it has been executed on a macOS host. CI has no macOS leg; I run the fleet and will verify that arm live. You offered this with exactly that condition — still yours if you want it, say so and I hold; otherwise I take it under the same constraint.

  5. iceteaSA commented on Sep 23, 2026

    @iceteaSA
    ContributorAuthor

    Sorry for the week of silence on your question. Please take it. My offer was conditional on the macOS arm being executed on a macOS host, and I don't have one, so the condition was always going to land on you. Holding it here would only have added a hand-off. If the Linux arm would benefit from a live read on this fleet host once it's built (RSS vs MADV_FREE accounting, read at list time), ping me and I'll measure it.

  6. subc-alfonso commented on Sep 23, 2026

    @subc-alfonso

    Taking it, since the macOS arm has to run on a macOS host anyway. Scope stays as narrowed: report-only RSS and CPU per module in supervisor.list and ck module status, no limits. It follows #91.

  7. subc-alfonso commented on Sep 25, 2026

    @subc-alfonso

    Ask 1 is on master (34fd9f7), report-only as narrowed: no limits, no policy.

    Each supervisor.list entry now carries an optional resources field for the child the supervisor spawned (not its descendants), read at list time:

    • Memory: on macOS the process's phys_footprint (from proc_pid_rusage), not RSS, since RSS overstates long-lived Rust processes whose MADV_FREE pages stay counted. On Linux VmRSS, plus VmSwap, from /proc/<pid>/status.
    • CPU: cumulative user and system time. On macOS the kernel reports Mach ticks, which are converted with the timebase. A test that does 400 ms of busy work fails by name if that conversion is removed, because the reading drops to about 10 ms.
    • Could not read is its own state with a reason (not_running, unsupported_platform, unreadable, process_identity_unconfirmed), never zero. On Linux the daemon re-checks the spawn start time after the read, so a recycled pid reads as unconfirmed rather than as someone else's numbers. Windows reports unsupported_platform for now.
    • An older daemon's entries decode with the field absent, and ck shows "not reported by this daemon" rather than a number.

    ck module status <id> has a resources: line, and ck module list --verbose adds memory and CPU columns. The default list is unchanged.

    The OS calls live in subc-os, the one crate allowed unsafe. The macOS arm was exercised on a macOS host against real processes, as agreed. The Linux arm ran in a container (arm64); if you want to check it on your fleet host once you run a daemon with this change, the resources: line of ck module status against /proc/<pid>/status is the comparison.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions