Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,12 @@ Things that look arbitrary in the code but are load-bearing (full reasoning in [
- `deployment.tf`'s `system` volume is an `empty_dir`, rebuilt from the image on every pod start — a fix to anything under `/usr`, `/etc`, `/var` must go in the image or the init script, not be treated as a one-time patch.
- The Dockerfile writes shared env vars to `/etc/environment` rather than using `ENV`, because `PATH` needs to be extended by a script running after the image is built, not fixed at build time.
- `parameters.tf`'s `local.validated_*` allowlist is the only thing stopping `system_packages`/`preferred_nodes` from injecting shell metacharacters into the init container — any new parameter whose value reaches a shell must go through the same validate-then-use step. `memory_watchdog_mode` follows it too: Coder constrains the value server-side, but it is the single switch deciding whether the watchdog may signal processes, so an unrecognised value falls back to the inert `observe` rather than being passed through.
- `script-memory-watchdog.sh` computes headroom as `memory.max − U`, where `U` sums only the *unreclaimable* fields of `memory.stat` (`anon`, `shmem`, `unevictable`, `slab_unreclaimable`, `kernel_stack`, `pagetables`, `sec_pagetables`, `percpu`, `sock`). Do not "simplify" it to `memory.current` or to `memory.stat`'s `kernel` roll-up: on the live pod those read 92% and 42% of the limit while true `U` is 23%, so either substitution makes the watchdog fire permanently on an idle container. Its thresholds are absolute bytes, not percentages, because the page cache a workload needs is a property of the workload rather than of the limit — which also means the 4 GiB memory parameter needs its own numbers, and the script logs a warning when it detects that mismatch.
- `script-memory-watchdog.sh` computes headroom as `memory.max − U`, where `U` sums only the *unreclaimable* fields of `memory.stat` (`anon`, `shmem`, `unevictable`, `slab_unreclaimable`, `kernel_stack`, `pagetables`, `sec_pagetables`, `percpu`, `sock`). Do not "simplify" it to `memory.current` or to `memory.stat`'s `kernel` roll-up: on the live pod those read 92% and 42% of the limit while true `U` is 23%, so either substitution makes the watchdog fire permanently on an idle container. Its tier thresholds and its `RLIMIT_DATA` ceilings are **derived from the pod's own `memory.max`**, not written into the script — the workspace is offered at 4 and 8 GiB, and a ladder fixed for one sits permanently on its first rung in the other. The rungs are multiples of a single critical reserve (`memory.max / 10`, clamped to 384 MiB…1 GiB); at 8 GiB this reproduces the hand-tuned numbers it replaced. A pod too small for the ladder to fit inside refuses to enforce and logs why. Set any `WATCHDOG_T_L*` or `WATCHDOG_CEILING_<role>` in the environment to override one value without replacing the derivation.
- **The watchdog decides what is a VS Code process by executable path — `argv[0]` under `~/.vscode-server/` — never by whether something "is node".** A provisioned workspace has two unrelated node installations: VS Code's bundled one under `~/.vscode-server/cli/servers/Stable-<commit>/server/`, and mise's on `PATH`, which is what repo tooling and the operator's agent sessions run on. (There is no `/usr/bin/node`, and nothing named `node` on `PATH` at all without dotfiles.) Matching on `comm`, on a basename, or on a loose cmdline substring would classify an agent session spawned by an extension — a child of the extension host, and *not* under ptyHost — as a sheddable editor helper. `comm` in particular is `MainThread` for every node process in a real tree, never `node`, because V8 renames its main thread; nothing may key off it. `script-memory-watchdog-test.sh` asserts this three ways, each paired with the mutation that flips it.
- The watchdog never signals anything in the `--type=ptyHost` subtree. Tree membership alone is *not* a safe kill criterion: tmux sessions and agent runs started from a VS Code integrated terminal are descendants of the server tree through ptyHost, so a tree-wide kill would take the operator's work with it. The exclusion is asserted, together with the mutation that must flip it, in `script-memory-watchdog-test.sh`.
- **The never-signal guards match `comm`, `argv[0]`'s basename, and whole path segments of argv elements — never a substring of the joined command line.** Loose substrings over-matched twice: `*/claude*` protected an unrelated process because a scratchpad path contained `/claude`, and `*memory-watchdog*` protected *every* process in a test harness because the harness's own directory path contained it, leaving two full runs green while asserting nothing. The watchdog's own identity is now structural — its pid, ancestors and descendants — rather than a name at all. Each guard records which rule claimed a process, and the tests assert every rule is individually reachable; a guard nothing can trigger is untested, not correct.
- **`RLIMIT_DATA` accounts `VmData`, not RSS, and on a V8 process the two differ by an order of magnitude** — measured on the live 4 GiB workspace at rest, the extension host was 497 MB resident against 1004 MB of data, the file watcher 66 MB resident against 622 MB. A ceiling reasoned about as though it bounded RSS is therefore far tighter than intended: the first derived file-watcher ceiling was *below* what an idle file watcher already held, which in enforce mode would have killed it on its next allocation and again on every restart. Every ceiling is now `max(derived, observed data + 2 × reserve)`, i.e. a growth allowance rather than an absolute size, and a role that could only be capped above `memory.max` is reported instead of capped.
- **The graded rungs do not engage against the failure this pod actually has** — measured, not assumed. See [DESIGN.md](DESIGN.md#design-tensions-and-decisions): the recorded OOM kills are 40–90 second spikes, and a live reproduction on the test workspace took the container from idle to `OOMKilled` in 43 seconds while the watchdog, running in enforce mode throughout, never left `L0` and logged no action at all. Do not tune the debounce to "fix" this without first asking whether a poll loop can see the event at all; the lever that did work was the `RLIMIT_DATA` cap, which is preventive and needs no sampling.
- Adding a package/tool has three possible homes, and picking the wrong one is a real mistake, not a style choice — route by the rule in [DESIGN.md](DESIGN.md#where-the-workspace-environment-comes-from): universal + stable → image (`Dockerfile`); occasionally-needed + apt-only + too heavy to bake in → the template's `system_packages` parameter; personal, fast-moving, or not an apt package → the operator's dotfiles (a *different* repo — see below), never this one.
- `deployment.tf`'s Deployment `metadata.name` (`local.workload_name` in `main.tf`) is not cosmetic: the cluster's Prometheus resolves pod → ReplicaSet → Deployment via an existing `kube_pod_owner` recording rule and exposes the result as a `workload` label with no other join needed, so whatever this Deployment is named *is* the identity CPU/memory/PSI/OOM metrics get attributed to. Don't revert it to an opaque identifier (e.g. the workspace UUID) without re-breaking that attribution — see [DESIGN.md](DESIGN.md#design-tensions-and-decisions).

Expand Down
6 changes: 5 additions & 1 deletion DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,11 @@ What is left is to never reach the limit in the first place, which is what [`scr

Which processes it may touch is settled by executable path, not by name or role heuristics: only a process whose own binary lives under `~/.vscode-server` counts as the editor's. That boundary is doing more work than it appears to. A provisioned workspace carries two unrelated node installations — VS Code's bundled one, which arrives with the server download, and the operator's from mise, which is what repo tooling and long-running agent sessions run on — and a rule that asked "is this node" instead of "whose binary is this" would classify an agent session spawned by an extension as an editor helper and shed it. The watchdog exists to stop the operator's work being collateral damage, so a detection rule that makes it the target would be a self-defeating one. Terminal descendants are excluded on top of that, by excising the editor's pty host and everything beneath it.

The trade is that this is a userspace daemon in a pod with no supervisor, doing something the kernel would do better if it were allowed to. It is therefore built to be deletable in one step if the constraint ever lifts, and it defaults to an observe-only mode — measuring and logging, changing nothing — so that the thresholds at which it acts get set from a week of this workload's own data rather than from a guess. That default is a workspace parameter rather than a constant, because the thresholds are absolute byte counts sized for the larger pod, and the same setting that suits it sits permanently near the first tier on a smaller one. Its measurement deliberately disagrees with every stock memory reading, including Coder's own: page cache and reclaimable slab make this pod look near death while it is idle, and a watchdog that believed them would fire constantly. That disagreement is the point of the thing, so the honest number is surfaced next to the misleading one in the workspace UI rather than replacing it.
The trade is that this is a userspace daemon in a pod with no supervisor, doing something the kernel would do better if it were allowed to. It is therefore built to be deletable in one step if the constraint ever lifts, and it defaults to an observe-only mode — measuring and logging, changing nothing. Its measurement deliberately disagrees with every stock memory reading, including Coder's own: page cache and reclaimable slab make this pod look near death while it is idle, and a watchdog that believed them would fire constantly. That disagreement is the point of the thing, so the honest number is surfaced next to the misleading one in the workspace UI rather than replacing it.

Everything it acts on is derived from the pod's own `memory.max` rather than fixed in the script, because the workspace is offered in more than one size and a ladder tuned for the larger one sits permanently on its first rung in the smaller. The rungs are multiples of a single critical reserve — a tenth of the limit, floored and capped — so the 8 GiB pod keeps the numbers that were reasoned about for it, the 4 GiB pod gets the same shape scaled down, and a pod too small for the ladder to fit inside at all refuses to enforce and says why instead of shedding the editor continuously. Frequency is part of that: each rung fires at most once per excursion and only recovery re-arms it, because an editor that dies every fifteen minutes gets the watchdog switched off, and a watchdog that is switched off protects nothing.

**What the watchdog does not address, established by measurement rather than assumed.** Every memcg OOM kill recorded for this workspace in the kernel's own log names a Claude Code session as the victim — never a VS Code process, and at the largest one VS Code was not running at all. The kills are also spikes rather than growth: a single session went from a few hundred megabytes to 7.3 GiB of anonymous RSS inside ninety seconds, and a reproduction on the test workspace took the container from idle to `OOMKilled` in fifty-two seconds. Two consequences follow, and they are the reason this section is worth reading before extending the thing. First, no sampling interval this daemon can afford will reliably see such an event in time — it is a preventive limit that helps, not a reactive one. Second, the editor tree it manages is roughly 0.7 GiB, so shedding all of it buys seconds against a runaway of that size; the ladder is a brake, not the answer. What *does* convert that failure into a survivable one is the same `RLIMIT_DATA` lever pointed at the runaway itself: capped, the identical allocation fails inside its own process with an ordinary `RangeError` and the container is untouched. Extending the ceilings beyond the editor tree is therefore the obvious next question, and it is deliberately left open here because it decides what may happen to the operator's own long-running sessions.

## Outcomes targeted

Expand Down
Loading
Loading