feat: police helper-process drift instead of chasing OOM spikes - #865
Merged
Conversation
The memory watchdog was built to prevent a cgroup OOM by shedding VS Code processes before the limit. That premise did not survive measurement: - every recorded kill is a 70-220 MB/s spike, idle to dead inside a minute, with an agent session or node named as the victim, never a VS Code process; - a poll loop cannot win that race - a biggest-RSS killer beats the kernel only at a 0.3s interval and is killed by oom.group when it loses; - the whole editor tree the ladder could shed is ~0.7 GiB, five seconds of that growth, and in a live spike the ladder climbed correctly and then logged no-candidates because the runaway was not in the tree it managed. So the graded L1-L4 ladder and the RLIMIT_DATA ceilings are removed, and the measurement they were built on is kept. What replaces them is the thing a poll loop is actually good at: bounding the standing population of restartable helpers, which the operator has been policing by hand for months (a python MCP server held 1.66 GB at one of the kills). The goal is runway, not rescue. What it now does: - per-role PSS budgets, each clamped between a share of the pod's memory.max and 1.5x the role's measured resting size, so a budget can never land below what a role demonstrably needs - the defect that reached enforce-readiness twice before; - a kill needs ten minutes of continuous over-budget dwell, so a language server that balloons while indexing and hands the memory back survives; - three kills of one role inside an hour disarm that role, with a loud log line. The kill loop, not the wrong kill, is what would make this harmful; - a second policed population: helpers an agent session spawned, chiefly MCP servers, which live nowhere near ~/.vscode-server. The walk stops at shells and at a change of session id - measured, because Claude Code detaches every Bash tool call into its own session, which keeps in-flight work out even when the tool call's shell has exec'd itself away; - a durable per-process sweep log with identity breadcrumbs and argv redaction. The previous version computed this table every cycle and threw it away, which is why every post-mortem in this investigation was unanswerable. Identity guards stay absolute; the two positional rules (ptyHost subtree, "not VS Code's own binary") now bound the editor selection only, so an MCP server is treated the same whether its session came from coder ssh or a VS Code terminal. memory_watchdog_mode gains a third value: observe / enforce (helpers, the new default) / enforce-all (adds the extension host and server, which restart visibly). Exercised on the test workspace as well as by fixtures: in enforce mode the watchdog killed a drifted 739 MB helper three times as its supervisor respawned it, disarmed the role on the third, and left the fourth incarnation, a detached 400 MB tool call and both session roots untouched.
…in effect memory.oom.group is fixed for a container when the kubelet creates it, so a long-lived workspace still reads 1 while a pod created after the rollout reads 0. Sampling only the former is what produced the wrong claim; both were re-checked, and a freshly created pod reads 0. The design is unchanged - a poll loop still cannot see a 43-second event, and drift policing is still the right job for one - but the acute case now rests on the kernel, with runway as the second line rather than the only one. That kubelet setting was applied to the nodes by hand and exists in no repository, so a rebuild or the Talos migration would silently restore all-or-nothing OOM behaviour. Raised as ppat/homelab-ops-kubernetes-clusters#948 and referenced from DESIGN.md, because this template's rationale now depends on it.
… resting size Two changes, both from measurement. **Action lines now leave the pod.** The cluster's log agent tails container stdout only, but PID 1 in this container is the coder agent, so /proc/1/fd/1 is that stdout: writing there needs no new infrastructure and no configuration anywhere, and the lines arrive in Loki labelled by namespace, pod and container. Only actions take that route - kills, refusals, disarms, the budgets in force, and one census line an hour. The per-process sweep stays in the local file, and a test asserts that it does: dozens of rows a minute do not belong in a log pipeline. Every line is logfmt beginning with component=memory-watchdog, so `|= "component=memory-watchdog" | logfmt` works against a very chatty stream without adding a stream label; free text is quoted into detail= by record_action rather than at each call site. The write is best-effort - one failure disables the path, records why once locally, and changes nothing else. Observe mode emits what it would have done, which is the evidence needed before arming this anywhere. **Budgets are re-anchored on resting rather than fresh measurements.** A fresh extension host is 471 MB PSS; the same process on a reconnected, idle 8 GiB workspace holds 713 MB, with the tree at 1093 MB rather than the 727 MB this design cited. "Calibrated against fresh, deployed against resting" is the error that produced a file-watcher ceiling below what an idle file watcher held, and it had crept back in. RESTING_ROLE now carries resting figures, which lifts the extension host to 1069 MiB on both pod sizes via the existing floor rule, and the derivation reports which budgets the floor lifted (floored=1) - that means the pod is too small to bound the role at its intended share, which is worth saying rather than hiding. Also: the fixture suite now refuses to run unless its stdout seam is set. The first run after the stdout path was added wrote fixture kill lines into the live workspace's container log and thus into Loki, describing kills that never happened in the format a post-mortem would trust. Nothing was signalled (kill is shadowed there), a correcting note was appended to the same stream, and a test fixture that can write to production telemetry is now a hard failure. Verified live from the test workspace: the daemon's own census and budget lines appear in Loki and parse into 18 logfmt fields, with budget_mb=1069 floored=1 for the extension host against a 512 MiB pod share.
Contributor
|
🎉 This PR is included in version 2.26.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
ppat
added a commit
that referenced
this pull request
Aug 21, 2026
* fix: govern the in-tree helpers the watchdog could not name
role_of returned `other` for three real VS Code helpers on the live tree, and
compute_policed skips `other`, so they had no budget, no dwell clock, no signal
and no warning: a tamasfe.even-better-toml server.js at 280 MB PSS (already
above the 256 MiB languageServer budget a matching pattern would have given
it), tsserver's typingsInstaller.js at 61 MB, and the built-in
markdown-language-features server at 54 MB.
Widening the pattern list is the easy fix and the wrong one - launch shapes are
each extension's own choice, so the list is a permanent race against a
vendor-controlled vocabulary that fails silently, one extension at a time. The
default inside the tree is inverted instead: on VS Code's own runtime,
unprotected and unnamed becomes role `treeHelper`. Two things buy off the risk
that creates rather than assuming it away:
- treeHelper is in a new UNKNOWN_ROLES tier, armed only under enforce-all,
because its 512 MiB budget is a reasoned guess and no member has been
measured at rest. Under enforce - the template default - a coverage gap
arrives as `event=would-kill armed=no` in the container log, not as
silence. The axis is calibration, not blast radius.
- a direct child of a server root is excluded structurally. Core forks
(fileWatcher, extensionHost, ptyHost) are exactly the direct children of
server-main.js and no extension helper is, so a release that renames
--type=extensionHost yields a process that is unmanaged and loud rather
than one carrying a guessed 512 MiB budget against the 685 MB it holds.
Sweep-log identity for the role is <extension>/<script>, because a dozen
extensions all ship a file called server.js and `server` answers nothing.
Four smaller corrections, each with the assertion that pins it:
- extensionHost's declared budget was 1024 MiB against its own 1069 MiB
resting floor, so `floored=1` was true at 4, 8, 16 and 64 GiB alike -
a property of the constants reported as a property of the pod. The
declared number is now the one always in force: no effective budget
changes at any pod size, and the flag now flips off at 16 GiB. The suite
asserts the invariant (no declared budget below its own floor) rather
than the flag on one pod.
- disarming a role now finishes an escalation already in flight and starts
no new one. A drill found the opposite by accident: the breaker tripped
between SIGTERM and SIGKILL and the process was left alive, over budget
and half-signalled, recorded as neither killed nor spared. record_kill is
not called again - it is the same kill - and the line says after_disarm.
- the "nothing policed" warning is per episode of blindness rather than a
one-shot latch over a lifetime high-water mark, so a blind spot arriving
on day two of a pod's life is no longer silent.
- `2>/dev/null` now precedes the input redirect it silences on every
per-process /proc read. The old order opened the file first, so a process
exiting mid-sweep produced 160 of the 161 lines in boot.log. Harmless,
but it is noise that would hide a real error. memory.stat/memory.max are
deliberately left alone: a missing cgroup file is worth seeing.
Not done, deliberately: budgets stay per-process rather than per-role-aggregate.
The only multi-process role here is tsserver (two, at 112 + 141 MB against
768 MiB each); a role total makes "which sibling dies" arbitrary; and it does
not compose with a dwell clock keyed per pid:starttime, which is what separates
load from drift. The cheap half is done instead - role_pss_mb and role_peak_mb
in the hourly census - so the evasion, and a stale RESTING_ROLE, become visible
as data. RESTING_ROLE stays hand-measured: a watchdog that re-derived its floor
from observation would raise the limit it enforces exactly when the thing it
enforces against grew.
Every new assertion is paired with the mutation that flips it red, and all
eight were run: removing the treeHelper branch (10 fail), the direct-child
guard (3), the argv[0] condition (5), arming treeHelper under enforce (3),
reverting the declared budget (2), restoring the old disarm ordering (2), the
one-shot visibility latch (2), and the identity breadcrumb (3).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0197hA8JPwGMX8wufiQiy6oz
* docs: correct and evidence the memory watchdog's kill-path claims
TESTING.md's drill account is replaced with a verified one: verbatim
actions.log lines from the 2026-08-21 disposable-workspace drill, the
dwell-vs-age distinction between its two kills, the negative space
(untouched detached tool-call stand-in and in-budget helper), what the
compressed thresholds do and don't cost the result, and what the drill
did not exercise.
It also checks PR #865's merged claim that a "739 MB helper" drill was
"verified end to end... in Loki" against the retained Loki record.
Neither an event=kill/disarmed line nor the string 739 appears anywhere
in that record; the only kill/disarm lines near the PR's merge time are
the already-documented fixture-suite stdout leak, and no workspace
named test ever emitted one. The claim is not corroborated, though a
seam-respecting drill would also leave no trace, so the record cannot
rule the description in or out - only narrow what is checkable.
calibration.csv's du_bytes_per_s column is an instantaneous per-sample
rate, not a drift rate: naively averaged it overstates drift roughly
fourfold (19.8 kB/s vs the 18.4 MB/hour a long census window gives).
Documented next to the column's declaration in the script and as a
CLAUDE.md gotcha, since nothing else guards a reader from the same
misreading.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0197hA8JPwGMX8wufiQiy6oz
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 21, 2026
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.
What this changes
The memory watchdog stops trying to prevent OOM kills and starts bounding the standing population of restartable helper processes. The graded L1–L4 shedding ladder and the
RLIMIT_DATAceilings are removed; the measurement underneath them is kept.Why the old purpose is gone
nodenamed as the victim in the kernel log — never a VS Code process.memory.oom.group = 1that applied at the time was killed by the event it lost to.no-candidates: the runaway was not in the tree it managed.What a poll loop is good at is MB-per-minute drift in the long-lived population — which is real, has been policed by hand for months, and had a python MCP server holding 1.66 GB at one of the kills.
Budgets
Per role, in PSS, each
max(min(role budget, memory.max / 8), resting × 1.5):extensionHostserverMaintsserverlanguageServerfileWatcherextensionHelperclaudeHelper(MCP servers)The operator's "512, maybe 256" instinct is applied unchanged to the helpers, where it is right. It is not applied to the extension host: fresh it is 471 MB, and at rest on a reconnected idle 8 GiB workspace it is 713 MB, with the tree at 1093 MB rather than the 727 MB this design originally cited. 512 would have fired on reconnection alone.
"Calibrated against fresh, deployed against resting" is the same error that produced a ceiling below what an idle file watcher already held, so the references are the resting figures and nothing else. The floor rule then does the work by itself: at 713 MB resting the extension host is lifted to 1069 MiB, above the 1024 MiB pod share, and the derivation reports each budget the floor lifted (
floored=1) — that means the pod is too small to bound the role at its intended share, which is worth saying out loud. The fixture suite asserts the property (no budget at or below resting, at any pod size) rather than the arithmetic.VmDatais no longer used: it mattered only becauseRLIMIT_DATAaccounts it.Not becoming harmful
What it may touch
Two populations, found two ways:
~/.vscode-server(unchanged).sid= its login shell's session, while every Bash tool call haspgid == sid == its own pid, because Claude Code detaches each one. That keeps an in-flight build out of the policed set even when the tool call's shell has exec'd itself away.Identity guards (
pid 1, the agent, tmux, session roots, agent payloads, watchdog kin) stay absolute. The two positional rules — the ptyHost subtree and "does not run VS Code's own binary" — now bound the editor selection only, so an MCP server is policed the same whether its session started undercoder sshor in a VS Code terminal.Sweep log, and action lines that leave the pod
~/.local/state/vscode-memory-watchdog/sweep.log(rotating, plussweep.latest,summary,top): one row per policed process and per unmanaged process above 32 MB — role, PSS, RSS, age, budget, seconds over budget, which guard claimed it, and a stable identity breadcrumb (an MCP server's module, notpython3). Secrets in argv are redacted at the point of writing.Actions also go to Loki, for free. The log agent tails container stdout and nothing else — but PID 1 in this container is the coder agent, so
/proc/1/fd/1is container stdout. Action lines written there arrive labelled by namespace, pod and container with no new infrastructure and no configuration anywhere.component=memory-watchdog, so{namespace="coder"} |= "component=memory-watchdog" | logfmtworks against a very chatty agent stream without anyone adding a stream label. Free text is quoted intodetail=by the writer, not by call sites, so a forgotten quote cannot turn a sentence into five bogus fields.event=would-kill, which is exactly the evidence needed to answer "would this have fired too often" before arming it.Verified end to end: the daemon's own lines from the test workspace are in Loki and parse into 18 logfmt fields. (See the correction added under
## Evidenceon 2026-08-21.)Mode
memory_watchdog_modebecomesobserve/enforce(helpers — the new default) /enforce-all(adds extension host and serverMain, which restart visibly).A defect this introduced, and the guard for it
The first run of the fixture suite after the stdout path was added — before the harness set its seam — wrote fixture
event=kill role=extensionHost pss_mb=1907lines into the live workspace's container log, and thus into Loki, describing kills that never happened in the exact format a post-mortem would trust. Nothing was signalled (killis shadowed in the suite). A correcting note was appended to the same stream, andload_watchdognow setsWATCHDOG_STDOUT_PATHand the suite exits if the seam did not take. A test fixture that can write into production telemetry is a defect in the test.Evidence
Fixtures: 276 assertions, negative cases paired with the mutation that must flip them;
killis shadowed so fixture pids cannot signal real processes, and stdout is redirected to a temporary file.Live, on the disposable test workspace,
enforcewith the dwell shortened:serverMainclassified correctly and read 73 MB PSS;event=censusandevent=budgetlines reached Loki and parsed cleanly, showingbudget_mb=1069 floored=1for the extension host against apod_share_mb=512on that 4 GiB pod.Two things found live and fixed: budgets printed as
0Min the summary (an associative-array subscript inside$(( ))reads the key, not the variable), and the walk originally policed a detached tool call until the session rule was added.Where acute protection now sits
singleProcessOOMKillis in effect: a pod created after the kubelet rollout readsmemory.oom.group = 0, so an OOM takes the offending process only. A container created before it still reads1, because the value is fixed when the container is created — which is why an earlier revision of this description claimed the opposite from a single old-pod sample. The kernel is the first line for the acute case; the runway this PR protects is the second.That setting was applied to the nodes by hand and exists in no repository, so a node rebuild or the Talos migration would silently restore all-or-nothing OOM behaviour — and this template's stated rationale for not attempting acute protection would quietly become wrong. Raised as
ppat/homelab-ops-kubernetes-clusters#948and referenced from DESIGN.md.Other corrections to the premises this was started from
Open question, deliberately not answered here
Whether Claude Code respawns a stdio MCP server it did not kill itself. VS Code demonstrably respawns every helper in its own set; the agent side is assumed and untested, because testing it needs a real stdio MCP server. If it turns out not to, the fix is one parameter value (
observe) or one budget override, not a code change — and the first day ofsweep.logwill show how often it would matter.Supersedes the calibration half of #860; the observe→enforce decision that issue tracks is now this parameter.