From d12e6342695ac0368bbd33b6c504bd3135e22dff Mon Sep 17 00:00:00 2001 From: Peter Pathirana Date: Mon, 17 Aug 2026 20:41:47 +0000 Subject: [PATCH 1/2] feat: derive watchdog thresholds from the pod and make its guards structural The memory watchdog shipped with tier thresholds and RLIMIT_DATA ceilings as absolute byte constants sized for an 8 GiB pod. On the 4 GiB workspace it detected its own miscalibration and warned rather than misbehaving, which was the right failure but left the mechanism usable at exactly one pod size. Both are now derived from memory.max: one critical reserve C = memory.max/10, clamped to [384 MiB, 1 GiB], with the ladder as fixed multiples of it, and ceilings as sixteenths of memory.max clamped to [512 MiB, 4 GiB]. At 8 GiB this reproduces the hand-tuned numbers it replaces, which is the only calibration point that existed. Below roughly 3 GiB the ladder cannot fit inside the pod at all, so enforce mode refuses itself and says why rather than shedding the editor continuously. The never-signal guards were substring matches over the joined command line and had over-matched twice, once protecting every process in a test harness because the harness path contained the string being matched. They are now keyed the same way positive selection already is: comm, argv[0]'s basename, whole path segments of argv elements, and - for the watchdog's own identity - pid ancestry rather than a name at all. Each guard records which rule claimed a process, and the tests assert that every rule is individually reachable. Frequency is now treated as part of correctness, since an editor that dies every fifteen minutes gets the watchdog switched off. Each rung fires at most once per excursion below L1 and recovery is what re-arms it; the fixed 180s cooldown is replaced by a settle window that no longer blocks escalation. Observe mode publishes the shed rate enforce mode would have produced. Co-Authored-By: Claude Opus 5 (1M context) --- .../script-memory-watchdog-test.sh | 352 +++++++++++- .../script-memory-watchdog.sh | 534 +++++++++++++++--- 2 files changed, 801 insertions(+), 85 deletions(-) diff --git a/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh b/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh index d72b0167..6a4f1b2c 100755 --- a/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh +++ b/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh @@ -69,6 +69,9 @@ reset_tier_state() { C_L2=0 C_L3=0 LAST_ACTION_AT=${1:-0} + RUNG_FIRED=() + IN_EXCURSION=0 + EXCURSIONS=0 } # --------------------------------------------------------------------------- # @@ -244,6 +247,12 @@ load_watchdog() { WATCHDOG_MODE=observe \ . "${SELF_DIR}/script-memory-watchdog.sh" mkdir -p "${WORK}/state" + # The thresholds and the RLIMIT_DATA ceilings are derived from memory.max on + # the first successful scan rather than being constants, so a harness that + # skipped this would be testing a watchdog with an empty ladder - which is not + # a state the real thing is ever in, and which silently passes any assertion + # about *not* acting. + read_cgroup_memory && derive_limits "$M_MAX" } scan_fixture() { @@ -256,12 +265,12 @@ scan_fixture() { candidate_pids() { select_candidates "$1" - local row pid out="" + local row pid acc="" for row in "${CANDIDATES[@]}"; do pid=${row#* } - out+="${pid%% *} " + acc+="${pid%% *} " done - printf '%s' "${out% }" + printf '%s' "${acc% }" } # --------------------------------------------------------------------------- # @@ -568,6 +577,152 @@ test_two_servers() { fi } +# --------------------------------------------------------------------------- # +# 3c. the guards are precise, and each of them is reachable +# +# Both historical over-matches were substring matches over the joined command +# line, both were found by accident, and both were invisible to a green suite - +# the second one protected every process in a fixture harness because the +# harness's own directory path contained the string the guard matched on, so two +# full runs asserted nothing at all. The decoys below are those exact paths. +# +# Each rule is also asserted to be *reachable*: a guard that no fixture can +# trigger is not being tested, it is only being carried. +# --------------------------------------------------------------------------- # + +# Three processes that all run VS Code's own node, inside the tree, outside the +# ptyHost subtree - so nothing structural separates them. Only the guards do. +add_decoys() { + local dir=$1 + local vsc="/home/coder/.vscode-server" + local sdir="${vsc}/cli/servers/Stable-abc123/server" + local ext="${vsc}/extensions" + # Historical over-match 1: a path that merely contains /claude. The segment is + # `claude-10001`, which is not the program `claude`. + add_proc "$dir" 80 41 MainThread 700000000 "${sdir}/node" \ + "${sdir}/out/bootstrap-fork" --type=fileWatcher \ + --logsPath /tmp/claude-10001/-home-coder-code-coder/scratchpad + # Historical over-match 2: the watchdog's own name appearing in an unrelated + # path. Self-identity is structural now, so nothing here can match it. + add_proc "$dir" 81 41 MainThread 600000000 "${sdir}/node" \ + "${ext}/redhat.vscode-yaml-1.24.0/dist/languageserver.js" \ + --config /home/coder/watchdog-live-evidence/memory-watchdog-run2/settings.json + # The case the payload rule genuinely exists for, and the reason it cannot + # simply be deleted: an agent session that an extension started with the + # editor's own interpreter. argv[0] is VS Code's node, so is_vscode_binary + # says "editor"; it is not under ptyHost, so the subtree rule never sees it. + add_proc "$dir" 82 41 MainThread 900000000 "${sdir}/node" \ + "${ext}/anthropic.claude-code-2.1.232/resources/claude-code/cli.js" --ide + # Outside the tree, and the reason argv[0] is consulted at all: for a script + # with a shebang the kernel sets comm from the *interpreter*, so a launcher on + # PATH called `claude` reports comm=bash. The name the operator knows it by + # survives only in argv[0]. + add_proc "$dir" 83 1 bash 500000000 /home/coder/.local/bin/claude --resume +} + +test_guards_are_precise() { + printf 'the guards are precise\n' + write_cgroup "${WORK}/cg" 8589934592 0.00 + local pdir="${WORK}/proc3c" + build_tree "$pdir" + add_decoys "$pdir" + load_watchdog "${WORK}/cg" "$pdir" + scan_fixture + + assert_protected 80 no "a helper whose log path contains /claude is not protected by it" + assert_protected 81 no "nor is one whose config path contains memory-watchdog" + assert_protected 82 yes "while a claude-code payload on VS Code's own node is" + assert_eq payload "${PROTECT_REASON[82]}" "and it is the payload rule that claims it" + assert_protected 83 yes "a shebang launcher named claude is protected" + assert_eq argv0 "${PROTECT_REASON[83]}" "by argv[0], since its comm is the interpreter" + + # Falsification: if the two decoys were unreachable for some other reason, the + # assertions above would pass against a watchdog that selects nothing. + local l4 + l4=" $(candidate_pids L4) " + for p in 80 81; do + if [[ $l4 == *" $p "* ]]; then + ok "decoy pid ${p} is genuinely reachable, so its non-protection means something" + else + bad "decoy pid ${p} is unreachable anyway - the assertion proves nothing" + fi + done + if [[ $l4 == *" 82 "* ]]; then + bad "the claude-code payload is reachable at L4" + else + ok "the claude-code payload is not reachable at any tier" + fi + + # Every guard the code can apply is applied to something here. A rule nothing + # exercises is a rule nobody has established the correctness of. + local want got reasons=" " + for p in "${!PROTECT_REASON[@]}"; do + reasons+="${PROTECT_REASON[$p]} " + done + for want in pid1 comm argv0 payload ptyhost foreign-binary; do + if [[ $reasons == *" $want "* ]]; then + ok "guard '${want}' is exercised by the fixture" + else + bad "guard '${want}' is never exercised - it is untested, not correct" + fi + done + + # And the census says something usable about all of it: a managed tree with + # nothing eligible is the shape of the harness bug, whatever the cause. + got="$(census_line)" + if [[ $got == *"eligible=0" ]]; then + bad "census reports no eligible processes on a healthy tree: ${got}" + else + ok "census reports the tree, the guards and what is left: ${got}" + fi +} + +# --------------------------------------------------------------------------- # +# 3d. an acting tier never acts silently +# +# The bug that hid the second over-match was not the over-match: it was that +# enforce mode logged tier=L3 with no signal line and no refusal line, a state +# that reads as "nothing needed doing". Whatever the guards do, every path out of +# an acting tier must now say what happened. +# --------------------------------------------------------------------------- # + +test_acting_tier_never_acts_silently() { + printf 'an acting tier never acts silently\n' + write_cgroup "${WORK}/cg" 8589934592 0.00 + local pdir="${WORK}/proc3d" + build_tree "$pdir" + rm -rf "${WORK}/state" + load_watchdog "${WORK}/cg" "$pdir" + scan_fixture + + # Reproduce the failure exactly: a guard that swallows the entire tree. + # shellcheck disable=SC2317,SC2329 # invoked indirectly, via the sourced watchdog + is_operator_payload() { return 0; } + compute_protected + : >"${WORK}/state/actions.log" + shed_load L3 1000 + if [[ "$(cat "${WORK}/state/actions.log")" == *"no-candidates tier=L3"* ]]; then + ok "a tier with nothing left to signal says so, with the census" + else + bad "an acting tier signalled nothing and logged nothing - the original bug" + fi + + # And the other silent path: a candidate too small to be worth the disruption. + # shellcheck disable=SC2317,SC2329 # invoked indirectly, via the sourced watchdog + is_operator_payload() { return 1; } + compute_protected + : >"${WORK}/state/actions.log" + WATCHDOG_MIN_SHED_RSS_SAVED=$MIN_SHED_RSS + MIN_SHED_RSS=999999999999 + shed_load L2 1000 + MIN_SHED_RSS=$WATCHDOG_MIN_SHED_RSS_SAVED + if [[ "$(cat "${WORK}/state/actions.log")" == *"no-worthwhile-candidate tier=L2"* ]]; then + ok "declining to shed something too small is logged, not skipped quietly" + else + bad "a tier declined to act and said nothing" + fi +} + # --------------------------------------------------------------------------- # # 4. the tier ladder # --------------------------------------------------------------------------- # @@ -648,6 +803,193 @@ test_tiers() { assert_tier L4 "$dead" 0 0 "$dead" 1010 "L4 ignores the cooldown" } +# --------------------------------------------------------------------------- # +# 4b. the ladder is derived from the pod, and stays sane at every pod size +# +# The thresholds used to be absolute bytes chosen for an 8 GiB pod, which made +# the 4 GiB workspace sit permanently on the first rung - the watchdog detected +# that itself and declined to be useful. What is asserted here is not that the +# arithmetic is what it is, but that the properties that make the ladder usable +# hold across every size the workspace is offered at, and past both ends of it. +# --------------------------------------------------------------------------- # + +# ladder_at -> "L4 L3 L2 L1" +ladder_at() { + T_L1="" T_L2="" T_L3="" T_L4="" + CEILING=() + derive_limits "$1" + printf '%s %s %s %s' "$T_L4" "$T_L3" "$T_L2" "$T_L1" +} + +test_derivation() { + printf 'the ladder is derived from the pod\n' + write_cgroup "${WORK}/cg" 8589934592 0.00 + load_watchdog "${WORK}/cg" "${WORK}/proc2" + + # 8 GiB is the only size the original absolute numbers were ever reasoned + # about, so the derivation has to land near them or it has thrown away the one + # piece of thinking that existed. 0.80/1.20/2.00/3.20 against 0.75/1.25/2.00/3.00. + assert_eq "858993459 1288490188 2147483647 3435973836" "$(ladder_at 8589934592)" \ + "8 GiB reproduces the hand-tuned ladder it replaces" + # 4 GiB: the case that forced this change. Resting headroom on the live 4 GiB + # workspace was 3.36 GiB, which has to be comfortably clear of L1. + assert_eq "429496729 644245093 1073741822 1717986916" "$(ladder_at 4294967296)" \ + "4 GiB scales the whole ladder down rather than sitting on it" + # 16 GiB: the objection that made the thresholds absolute in the first place - + # a plain percentage would reserve 1.6 GiB and shed with real headroom left. + assert_eq "1073741824 1610612736 2684354560 4294967296" "$(ladder_at 17179869184)" \ + "16 GiB is capped rather than scaled into absurdity" + # 1 GiB: far below anything offered. The floor holds, and the ladder stays + # ordered - the failure to design out is a rung that overtakes another. + local small + small="$(ladder_at 1073741824)" + assert_eq "402653184 603979776 1006632960 1610612736" "$small" \ + "a pod below the floor gets the floor, not a ladder of noise" + + local max l4 l3 l2 l1 + for max in 1073741824 2147483648 4294967296 8589934592 17179869184 34359738368; do + # Not via a command substitution: TOO_SMALL is state the derivation sets, and + # a subshell would discard exactly the answer being asserted on. + T_L1="" T_L2="" T_L3="" T_L4="" + CEILING=() + derive_limits "$max" + l4=$T_L4 l3=$T_L3 l2=$T_L2 l1=$T_L1 + if ((l4 < l3 && l3 < l2 && l2 < l1)); then + ok "memory.max=${max}: the rungs stay strictly ordered" + else + bad "memory.max=${max}: rungs out of order (${l4} ${l3} ${l2} ${l1})" + fi + # Either the pod has room above its own first rung, or the watchdog has + # declared the pod too small to act on. What must never happen is a pod that + # boots inside the shedding tiers while still believing it may shed - that is + # the "kills the editor every fifteen minutes" failure, reached by arithmetic + # rather than by bad luck. + if ((l1 * 2 <= max)); then + ok "memory.max=${max}: the pod has room above L1" + elif ((TOO_SMALL == 1)); then + ok "memory.max=${max}: too small for the ladder, and says so" + else + bad "memory.max=${max}: L1 (${l1}) crowds the limit and enforce is not refused" + fi + done + + T_L1="" T_L2="" T_L3="" T_L4="" + CEILING=() + derive_limits 1073741824 + assert_eq 1 "$TOO_SMALL" "a 1 GiB pod is marked too small for the ladder" + T_L1="" T_L2="" T_L3="" T_L4="" + CEILING=() + derive_limits 4294967296 + assert_eq 0 "$TOO_SMALL" "a 4 GiB pod is not" + + # Ceilings scale for the same reason and with the same failure modes: a 3 GiB + # extension-host ceiling on a 4 GiB pod is not cautious, it is unreachable. + T_L1="" T_L2="" T_L3="" T_L4="" + CEILING=() + derive_limits 8589934592 + assert_eq 1610612736 "${CEILING[serverMain]}" "8 GiB keeps the reasoned serverMain ceiling" + assert_eq 3221225472 "${CEILING[extensionHost]}" "and the extension-host one" + assert_eq 3758096384 "${CEILING[tsserver]}" "and tsserver's" + T_L1="" T_L2="" T_L3="" T_L4="" + CEILING=() + derive_limits 4294967296 + assert_eq 1610612736 "${CEILING[extensionHost]}" "4 GiB halves the extension-host ceiling" + assert_eq 536870912 "${CEILING[fileWatcher]}" "and floors the small ones rather than shrinking them to nothing" + local role + for role in serverMain extensionHost tsserver languageServer fileWatcher; do + if ((CEILING[role] < 4294967296)); then + ok "4 GiB pod: the ${role} ceiling is inside the pod" + else + bad "4 GiB pod: the ${role} ceiling (${CEILING[$role]}) is the whole pod - inert" + fi + done + + # An explicit environment value must win, or the live demonstration cannot + # force a ceiling to bite without editing the script under test. + T_L1="" T_L2="" T_L3="" T_L4="" + CEILING=() + WATCHDOG_CEILING_extensionHost=268435456 derive_limits 8589934592 + assert_eq 268435456 "${CEILING[extensionHost]}" "an explicit ceiling overrides the derivation" + T_L1="" T_L2="" T_L3="" + T_L4=123456789 # as WATCHDOG_T_L4 would have left it at startup + CEILING=() + derive_limits 8589934592 + assert_eq 123456789 "$T_L4" "and an explicit rung overrides its share of the ladder" + assert_eq 3435973836 "$T_L1" "while the rest of the ladder is still derived" +} + +# --------------------------------------------------------------------------- # +# 4c. how often it acts +# +# Shedding the editor is the right trade against an oom.group kill. Shedding it +# every fifteen minutes is not, because the operator switches the watchdog off +# and then it protects nothing. Correctness and frequency are both requirements. +# --------------------------------------------------------------------------- # + +# LAST_ACTION_AT and RUNG_FIRED are globals of the sourced watchdog, standing in +# here for the bookkeeping shed_load would have done after a real signal. +# shellcheck disable=SC2034 +test_shedding_is_rate_limited() { + printf 'shedding is rate limited\n' + write_cgroup "${WORK}/cg" 8589934592 0.00 + load_watchdog "${WORK}/cg" "${WORK}/proc2" + + local mid=2500000000 low=1800000000 crit=1000000000 big=6594088184 + local t=1000 + + reset_tier_state + # Reach L2 and let it fire. + step_tier "$low" 1500 0 "$low" $t + step_tier "$low" 1500 0 "$low" $t + assert_tier L2 "$low" 1500 0 "$low" $t "the first L2 of an excursion acts" + RUNG_FIRED[L2]=1 + LAST_ACTION_AT=$t + + # Still bad, well past the settle window: the old cooldown would have let L2 + # fire again every 180s for as long as the pressure lasted. + t=$((t + 600)) + assert_tier L1 "$low" 1500 0 "$low" $t \ + "and no later sample in the same excursion sheds again at L2" + + # Escalation is untouched - this is what the fixed cooldown got wrong, by + # demoting L3 to L1 for three minutes after an L2 shed. + step_tier "$crit" 0 0 "$crit" $t + assert_tier L3 "$crit" 0 0 "$crit" $t "but a worse rung still fires during the same excursion" + RUNG_FIRED[L3]=1 + LAST_ACTION_AT=$t + + # Recovery above L1 is what re-arms, not the clock. + t=$((t + 60)) + assert_tier L0 "$big" 0 0 "$big" $t "recovery above L1 ends the excursion" + step_tier "$low" 1500 0 "$low" $t + step_tier "$low" 1500 0 "$low" $t + assert_tier L2 "$low" 1500 0 "$low" $((t + 100)) "and the next excursion may shed again" + + # The settle window still separates two actions inside one excursion, so the + # ladder sees the effect of a kill before deciding it was not enough. + reset_tier_state + step_tier "$crit" 0 0 "$crit" 2000 + assert_tier L3 "$crit" 0 0 "$crit" 2000 "L3 acts" + LAST_ACTION_AT=2000 + RUNG_FIRED=() + assert_tier L1 "$crit" 0 0 "$crit" 2005 "another action 5s later is held back by the settle window" + + # L4 is exempt from all of it: by then the alternative is the whole pod. + reset_tier_state 2000 + RUNG_FIRED[L4]=1 + assert_tier L4 700000000 0 0 700000000 2001 "L4 ignores both the settle window and the rung flag" + + # An excursion is one continuous dip, not one sample below the line. + reset_tier_state + step_tier "$mid" 0 0 "$mid" 3000 + assert_eq 1 "$EXCURSIONS" "a dip below L1 opens exactly one excursion" + step_tier "$mid" 0 0 "$mid" 3010 + assert_eq 1 "$EXCURSIONS" "and staying down does not open another" + step_tier "$big" 0 0 "$big" 3020 + step_tier "$mid" 0 0 "$mid" 3030 + assert_eq 2 "$EXCURSIONS" "recovering and falling again does" +} + # --------------------------------------------------------------------------- # # 5. observe mode really is inert # --------------------------------------------------------------------------- # @@ -692,9 +1034,13 @@ main() { test_selection test_operator_runtime_is_never_a_helper test_selection_is_falsifiable + test_guards_are_precise + test_acting_tier_never_acts_silently test_comm_is_not_a_criterion test_two_servers test_tiers + test_derivation + test_shedding_is_rate_limited test_observe_mode_is_inert printf '\n%d passed, %d failed\n' "$PASS" "$FAIL" ((FAIL == 0)) diff --git a/templates/kubernetes/homelab-workspace/script-memory-watchdog.sh b/templates/kubernetes/homelab-workspace/script-memory-watchdog.sh index de910c69..69c30ebb 100644 --- a/templates/kubernetes/homelab-workspace/script-memory-watchdog.sh +++ b/templates/kubernetes/homelab-workspace/script-memory-watchdog.sh @@ -54,17 +54,47 @@ INTERVAL_IDLE="${WATCHDOG_INTERVAL_IDLE:-10}" INTERVAL_BUSY="${WATCHDOG_INTERVAL_BUSY:-2}" CALIBRATION_EVERY="${WATCHDOG_CALIBRATION_EVERY:-6}" # cycles between CSV rows -# Tier thresholds, in absolute bytes of headroom H. Absolute rather than a -# percentage of memory.max, because the page cache a workload needs to make -# forward progress is a property of the workload, not of the limit. +# Tier thresholds, in absolute bytes of headroom H, derived from the pod's own +# memory.max by derive_limits() below. Set any of these in the environment to +# override the derivation entirely; empty means "derive it". +T_L1="${WATCHDOG_T_L1:-}" +T_L2="${WATCHDOG_T_L2:-}" +T_L3="${WATCHDOG_T_L3:-}" +T_L4="${WATCHDOG_T_L4:-}" + +# The one number the ladder is derived from: the critical reserve C, the amount +# of headroom below which the next allocation burst can reach memory.max before +# the next sample can react. T_L4 is C, and the rungs above it are fixed +# multiples of it. # -# NOTE: unvalidated starting points, derived from role and an 8 GiB budget rather -# than from measurement. Replacing them with numbers taken from calibration.csv -# is the entire reason this ships in observe mode first. -T_L1="${WATCHDOG_T_L1:-3221225472}" # 3.00 GiB -T_L2="${WATCHDOG_T_L2:-2147483648}" # 2.00 GiB -T_L3="${WATCHDOG_T_L3:-1342177280}" # 1.25 GiB -T_L4="${WATCHDOG_T_L4:-805306368}" # 0.75 GiB +# Why this is a clamped fraction rather than an absolute constant, having been an +# absolute constant first. The original argument for absolute bytes was that the +# page cache a workload needs for forward progress is a property of the workload, +# not of the container's limit - and that is true, but it only settles what the +# thresholds *mean*, not what values are available. Headroom is bounded above by +# memory.max, so on a pod small enough that the fixed floor exceeds the range the +# pod ever has, an absolute ladder does not become conservative, it becomes +# permanently tripped and therefore inert: the 4 GiB workspace rests at H = 3.4 +# GiB against a 3.0 GiB L1, which is one editor window away from sitting at L1 +# for the rest of the pod's life. So the fraction scales the ladder to the pod, +# while: +# +# - the FLOOR expresses reaction time, which really is size-independent. It is +# what the sample interval times a plausible allocation rate costs, and no +# pod is too small to need it. +# - the CAP stops a percentage from scaling into absurdity on a large pod, +# which was the correct half of the original objection: a fraction that gave +# 16 GiB a 4 GiB "critical" reserve would shed with plenty of genuine +# headroom left. +# +# At 8 GiB this reproduces the hand-tuned ladder it replaces (0.80/1.20/2.00/3.20 +# against 0.75/1.25/2.00/3.00), which is the only calibration point that ever +# existed; at 4 GiB it gives 0.41/0.61/1.02/1.64, leaving the measured 3.4 GiB +# resting headroom two thirds of the pod clear of the first rung. +C_FRACTION_NUM="${WATCHDOG_C_NUM:-1}" +C_FRACTION_DEN="${WATCHDOG_C_DEN:-10}" +C_FLOOR="${WATCHDOG_C_FLOOR:-402653184}" # 384 MiB +C_CAP="${WATCHDOG_C_CAP:-1073741824}" # 1.00 GiB # Corroboration is required at L2 only. At L2 we are "at the limit"; PSI and the # refault rate are what separate "at the limit and fine" - the normal resting @@ -76,27 +106,71 @@ T_REFAULT_RATE="${WATCHDOG_T_REFAULT_RATE:-20000}" DEBOUNCE_L1="${WATCHDOG_DEBOUNCE_L1:-3}" DEBOUNCE_L2="${WATCHDOG_DEBOUNCE_L2:-3}" DEBOUNCE_L3="${WATCHDOG_DEBOUNCE_L3:-2}" -COOLDOWN="${WATCHDOG_COOLDOWN:-180}" PROJECTION_HORIZON="${WATCHDOG_PROJECTION_HORIZON:-60}" # seconds -# Soft RLIMIT_DATA ceilings by role, in bytes. Hard limits are never touched, so -# an inheriting shell restores itself with `ulimit -d unlimited`. +# How rarely it acts is part of what "working" means here, not a refinement of +# it. Shedding the editor is the right trade against an oom.group kill that takes +# every tmux session and every agent with it - but an editor that dies every +# fifteen minutes gets the watchdog switched off, and a watchdog that is switched +# off protects nothing. So the ladder is rate-limited by construction rather than +# by a timer alone: +# +# - SETTLE is the minimum gap between any two actions. It exists so the ladder +# can see the effect of a kill before deciding it was not enough. It does NOT +# hold back a higher rung indefinitely, which the fixed 180s cooldown it +# replaces did: that cooldown demoted L3 to L1 for three minutes after an L2 +# shed, so a fast-growing extension host could not be stopped during exactly +# the window when it most needed stopping. +# +# - Each rung then fires at most once per excursion, and an excursion only ends +# when headroom recovers above L1. Escalation is unaffected - L2, then L3, +# then L4 all remain available as things get worse - but a pod that is simply +# too small for its workload sheds one helper and one extension host and then +# stops, instead of shedding one every SETTLE seconds forever. If that is not +# enough, the answer is a bigger pod, and repeatedly killing the editor is a +# worse way of finding that out than the log line that says so. +SETTLE="${WATCHDOG_SETTLE:-30}" + +# A shed has to be worth its disruption. Killing a 40 MiB file watcher frees +# nothing, restarts a component the operator can notice, and burns the rung that +# would otherwise have been available later in the same excursion. +MIN_SHED_RSS="${WATCHDOG_MIN_SHED_RSS:-134217728}" # 128 MiB + +# Soft RLIMIT_DATA ceilings by role, as sixteenths of memory.max. Hard limits are +# never touched, so an inheriting shell restores itself with `ulimit -d +# unlimited`. +# +# These scale for the same reason the ladder does, and it matters more here: a +# 3 GiB extension-host ceiling on a 4 GiB pod is not conservative, it is inert - +# the pod dies first. The numerators are the hand-picked 8 GiB values expressed +# against that pod's limit (1.5, 3.0, 3.5, 1.0, 1.0 GiB), so an 8 GiB workspace +# gets exactly the ceilings that were reasoned about, and every other size gets +# the same shape. # -# NOT AGREED. Enforce mode must not be turned on until these are set from -# calibration data: too low kills a healthy extension host mid-edit, too high -# makes the mechanism inert. +# The floor is what keeps the scaling from turning into a different failure: a +# ceiling below what a role legitimately needs makes it die doing ordinary work, +# which is worse than no ceiling because it is constant rather than occasional. +# The cap keeps a large pod from being handed a ceiling so high that nothing +# could ever reach it. # # Every role here is a V8 process, and that is what makes a ceiling a reasonable # thing to set: hitting it makes mmap return ENOMEM, V8 raises its own fatal heap # OOM, and the editor offers "Restart Extension Host" or silently respawns the # language server. `extensionHelper` is deliberately absent - see role_of(). -declare -gA CEILING=( - [serverMain]=1610612736 # 1.50 GiB - [extensionHost]=3221225472 # 3.00 GiB - [tsserver]=3758096384 # 3.50 GiB - [languageServer]=1073741824 # 1.00 GiB - [fileWatcher]=1073741824 # 1.00 GiB +declare -gA CEILING_SIXTEENTHS=( + [serverMain]=3 + [extensionHost]=6 + [tsserver]=7 + [languageServer]=2 + [fileWatcher]=2 ) +CEILING_FLOOR="${WATCHDOG_CEILING_FLOOR:-536870912}" # 512 MiB +CEILING_CAP="${WATCHDOG_CEILING_CAP:-4294967296}" # 4.00 GiB + +# Filled by derive_limits(). A role may also be pinned outright from the +# environment - WATCHDOG_CEILING_extensionHost=... - which is how the live +# demonstration forces a ceiling to bite without editing the script. +declare -gA CEILING=() # Roles L2 is allowed to shed. Each is restarted transparently or on demand by # the editor, and none of them holds unsaved user state. @@ -108,18 +182,87 @@ MAX_CSV_LINES="${WATCHDOG_MAX_CSV_LINES:-50000}" # Scan state. Declared at file scope, not inside main(), so that sourcing the # script with WATCHDOG_SOURCE_ONLY=1 gives the test harness correctly-typed # globals without having to restate them. -declare -gA P_COMM=() P_CMD=() P_RSS=() CHILDREN=() -declare -gA SERVER_TREE=() PROTECTED=() +declare -gA P_COMM=() P_CMD=() P_ARGV0=() P_PPID=() P_RSS=() CHILDREN=() +declare -gA SERVER_TREE=() PROTECTED=() PROTECT_REASON=() WATCHDOG_KIN=() declare -ga PIDS=() CANDIDATES=() SERVER_ROOTS=() SERVER_PID="" TIER=L0 ROLE=other +GUARD="" PREV_TIER=L0 PREV_AT=0 PREV_U=0 PREV_REFAULT=0 PREV_PGSCAN=0 CYCLE=0 +H_MAX_SEEN=0 +H_MIN_SEEN=0 +CALIBRATION_WARNED=0 +C_RESERVE=0 +TOO_SMALL=0 +M_MAX=0 +STARTED_AT=${WATCHDOG_NOW:-$EPOCHSECONDS} + +# How often each rung has acted since the watchdog started, and when it last did. +# Published in the summary file every cycle - in observe mode too, where it is +# the count of sheds enforce mode *would* have performed. That number is what +# says whether enforce mode is tolerable on a live workspace, and it can be had +# without ever signalling anything. +declare -gA ACTIONS=([L2]=0 [L3]=0 [L4]=0) +declare -gA RUNG_FIRED=() +EXCURSIONS=0 +IN_EXCURSION=0 + +# --------------------------------------------------------------------------- # +# derivation - one pure function of memory.max; no state, no I/O +# --------------------------------------------------------------------------- # + +clamp() { + local v=$1 lo=$2 hi=$3 + ((v < lo)) && v=$lo + ((v > hi)) && v=$hi + printf '%s' "$v" +} + +# Sets T_L1..T_L4 and CEILING from memory.max. Anything already set from the +# environment is left alone, so a single role or a single rung can be pinned for +# an experiment without replacing the derivation. +derive_limits() { + local max=$1 role c + + c=$(clamp $((max * C_FRACTION_NUM / C_FRACTION_DEN)) "$C_FLOOR" "$C_CAP") + C_RESERVE=$c + : "${T_L4:=$c}" + : "${T_L3:=$((c * 3 / 2))}" + : "${T_L2:=$((c * 5 / 2))}" + : "${T_L1:=$((c * 4))}" + + # Inversion: what pod size makes this formula harmful rather than imprecise? + # A small enough one. The floor is a reaction-time budget and cannot shrink + # with the pod, so below roughly 3 GiB the top of the ladder approaches + # memory.max and the pod is inside the shedding tiers from the moment it boots + # - which would mean killing the editor continuously, the one outcome that + # reliably gets a watchdog switched off for good. There is no threshold that + # fixes this, because the pod genuinely has no runway; the honest response is + # to say so and refuse to act. The workspace offers 4 and 8 GiB, both of which + # clear this comfortably, so this is a guard against a future option rather + # than a live case. + TOO_SMALL=0 + ((T_L1 * 2 > max)) && TOO_SMALL=1 + + local var + for role in "${!CEILING_SIXTEENTHS[@]}"; do + # An explicit WATCHDOG_CEILING_ wins over the derivation. + var="WATCHDOG_CEILING_${role}" + if [[ -n ${!var:-} ]]; then + CEILING[$role]=${!var} + continue + fi + CEILING[$role]=$(clamp \ + $((max * CEILING_SIXTEENTHS[$role] / 16)) "$CEILING_FLOOR" "$CEILING_CAP") + done + return 0 +} # --------------------------------------------------------------------------- # # measurement - reads only, sets M_*/P_* globals, decides nothing @@ -209,6 +352,8 @@ read_process_table() { PIDS=() P_COMM=() P_CMD=() + P_ARGV0=() + P_PPID=() P_RSS=() CHILDREN=() @@ -234,6 +379,12 @@ read_process_table() { PIDS+=("$pid") P_COMM[$pid]=$comm P_CMD[$pid]="${argv[*]}" + # argv[0] is kept as its own field rather than recovered later by cutting + # P_CMD at the first space, which is wrong for any executable path + # containing one, and which is precisely the kind of "close enough" string + # handling this script has already been bitten by. + P_ARGV0[$pid]="${argv[0]:-}" + P_PPID[$pid]=$ppid CHILDREN[$ppid]+=" $pid" done return 0 @@ -369,50 +520,126 @@ subtree_of() { # the exact outcome this design exists to prevent, arriving one layer earlier # than the action rules that are meant to prevent it. Path, and only path. is_vscode_binary() { - local argv0=${P_CMD[$1]:-} - argv0=${argv0%% *} - [[ $argv0 == *"/.vscode-server/"* ]] + [[ ${P_ARGV0[$1]:-} == *"/.vscode-server/"* ]] +} + +# The watchdog's own process, everything that started it, and everything it +# started. Structural, by pid: this replaces a `*memory-watchdog*` cmdline match +# that once protected every process in a test harness because the harness lived +# in a directory whose path contained that string. Two full runs of that harness +# looked clean while exercising nothing. Identity is a thing you know about +# yourself, not a thing you pattern-match out of other processes' arguments. +compute_watchdog_kin() { + WATCHDOG_KIN=() + local cur=$$ hops=0 p + local -A mine=() + subtree_of "$$" mine + for p in "${!mine[@]}"; do + WATCHDOG_KIN[$p]=self + done + # Ancestors. Bounded, because a corrupt or racing ppid chain must not spin. + while ((cur > 1 && hops < 64)); do + cur=${P_PPID[$cur]:-0} + ((cur > 1)) || break + WATCHDOG_KIN[$cur]=ancestor + ((hops += 1)) + done + return 0 +} + +# True when an argv element names one of the operator's own programs. This is +# the one guard that has to look at arguments rather than at argv[0], because the +# case it exists for is a payload run by somebody else's interpreter: an agent +# session that an extension spawned as `/node .../claude-code/cli.js` +# has VS Code's binary at argv[0] and is not under ptyHost, so neither of the +# structural rules reaches it. +# +# Matching is on whole path segments, never on a raw substring. Substrings are +# what made `*/claude*` protect an unrelated process whose scratchpad path +# happened to contain /claude: the segment there was `claude-10001`, which is not +# the program and does not match. The cost of being wrong is asymmetric but not +# free in either direction - over-matching protects something that could have +# been shed, which quietly turns the mechanism off, and that is exactly how the +# harness bug hid. +is_operator_payload() { + local tok seg + for tok in ${P_CMD[$1]:-}; do + tok=${tok%/} + seg=${tok##*/} + case "$seg" in + claude | tmux | chezmoi) return 0 ;; + esac + case "/${tok}/" in + */claude-code/* | */claude-code-*/*) return 0 ;; + esac + done + return 1 } # The never-signal list. Every action consults this directly, so a defect in -# tree-walking still cannot route around it. +# tree-walking still cannot route around it. Sets GUARD to the rule that fired, +# so that a protection can be reported rather than merely happening. # # Tree membership is NOT a safe kill criterion: tmux sessions and long-running # agents started from a VS Code integrated terminal are descendants of the server -# tree via ptyHost. Errors here are one-directional by design - refusing to -# signal something that could safely have been signalled costs nothing. +# tree via ptyHost. The rules below are ordered cheapest and most certain first; +# each of them is asserted, and asserted to be individually reachable, by +# script-memory-watchdog-test.sh. is_never_signal() { local pid=$1 - ((pid <= 1)) && return 0 - ((pid == $$)) && return 0 - ((pid == BASHPID)) && return 0 - ((pid == PPID)) && return 0 + GUARD="" + ((pid <= 1)) && GUARD=pid1 && return 0 + ((pid == $$)) && GUARD=self && return 0 + ((pid == BASHPID)) && GUARD=self && return 0 + ((pid == PPID)) && GUARD=self && return 0 + if [[ -n ${WATCHDOG_KIN[$pid]:-} ]]; then + GUARD=watchdog-${WATCHDOG_KIN[$pid]} + return 0 + fi case "${P_COMM[$pid]:-}" in - coder | claude | chezmoi | sshd | init | systemd) return 0 ;; - tmux*) return 0 ;; + coder | claude | chezmoi | sshd | screen | init | systemd) + GUARD="comm" + return 0 + ;; + tmux*) + GUARD="comm" + return 0 + ;; esac - # Names are a second line of defence, not the first - is_vscode_binary is. They - # are also deliberately loose: `*/claude*` will match a path that merely - # contains /claude, which over-protects rather than under-protects, and that is - # the direction to err in. Nothing here should be load-bearing on its own. - case "${P_CMD[$pid]:-}" in - *"coder agent"*) return 0 ;; - *"/claude"* | "claude" | "claude "*) return 0 ;; - *chezmoi*) return 0 ;; - *memory-watchdog*) return 0 ;; + # argv[0]'s basename, i.e. the program actually being executed - the same + # question is_vscode_binary asks, asked of the other side. `/home/coder/...` + # appears in nearly every command line in this pod, so a rule that looked + # anywhere but argv[0] for the name `coder` would protect the entire pod. + case "${P_ARGV0[$pid]:-}" in + */coder | coder | */chezmoi | chezmoi | */tmux | tmux | */claude | claude) + GUARD=argv0 + return 0 + ;; esac + if is_operator_payload "$pid"; then + GUARD=payload + return 0 + fi return 1 } -# Everything the watchdog must never touch: the never-signal names anywhere in -# the pod, plus every ptyHost fork inside the server tree and all its descendants. +# Everything the watchdog must never touch: the never-signal rules anywhere in +# the pod, plus every ptyHost fork inside the server tree and all its +# descendants. PROTECT_REASON records which rule claimed each pid, which is what +# makes an over-broad guard visible instead of silently inert. compute_protected() { PROTECTED=() + PROTECT_REASON=() local pid p local -A pty=() + compute_watchdog_kin + for pid in "${PIDS[@]}"; do - is_never_signal "$pid" && PROTECTED[$pid]=1 + if is_never_signal "$pid"; then + PROTECTED[$pid]=1 + PROTECT_REASON[$pid]=$GUARD + fi done # Everything in the tree that is not running a VS Code binary, whatever its @@ -420,7 +647,10 @@ compute_protected() { # the extension host and not of ptyHost, so neither the subtree excision nor # tree membership would save it; its executable path does. for pid in "${!SERVER_TREE[@]}"; do - is_vscode_binary "$pid" || PROTECTED[$pid]=1 + if ! is_vscode_binary "$pid"; then + PROTECTED[$pid]=1 + PROTECT_REASON[$pid]=${PROTECT_REASON[$pid]:-foreign-binary} + fi done for pid in "${!SERVER_TREE[@]}"; do @@ -428,6 +658,7 @@ compute_protected() { subtree_of "$pid" pty for p in "${!pty[@]}"; do PROTECTED[$p]=1 + PROTECT_REASON[$p]=${PROTECT_REASON[$p]:-ptyhost} done fi done @@ -443,8 +674,7 @@ compute_protected() { # direction, whereas over-matching any other role means mis-classifying it. role_of() { local cmd=" ${P_CMD[$1]:-} " - local argv0=${P_CMD[$1]:-} - argv0=${argv0%% *} + local argv0=${P_ARGV0[$1]:-} case "$cmd" in *" --type=ptyHost "* | *"--type=ptyHost"*) ROLE=ptyHost ;; *" --type=extensionHost "*) ROLE=extensionHost ;; @@ -526,42 +756,73 @@ C_L1=0 C_L2=0 C_L3=0 LAST_ACTION_AT=0 +SUPPRESSED="" # Sets TIER. Called in the current shell, never in a command substitution - the # debounce counters are state and a subshell would silently discard them. +# +# SUPPRESSED records why an acting tier was demoted, so that "the ladder reached +# L3 and nothing happened" is always accompanied by the reason. It is not +# decoration: the one class of bug this watchdog has repeatedly produced is a +# state that looks correct because the evidence of its being wrong is absent. decide_tier() { local h=$1 psi=$2 refault=$3 proj=$4 now=$5 + local want + SUPPRESSED="" if ((h < T_L1)); then ((C_L1 += 1)); else C_L1=0; fi if ((h < T_L2)); then ((C_L2 += 1)); else C_L2=0; fi if ((h < T_L3)); then ((C_L3 += 1)); else C_L3=0; fi + # Excursion tracking. An excursion opens the first time headroom falls below + # L1 and closes when it comes back above it; the rung-fired flags live for + # exactly that long. Recovery is what re-arms the ladder, not the passage of + # time, because time passing does not mean the pressure went away. + if ((h < T_L1)); then + if ((IN_EXCURSION == 0)); then + IN_EXCURSION=1 + ((EXCURSIONS += 1)) + RUNG_FIRED=() + fi + elif ((IN_EXCURSION == 1)); then + IN_EXCURSION=0 + RUNG_FIRED=() + fi + if ((h < T_L4)); then TIER=L4 return 0 fi - TIER=L0 + want=L0 if ((C_L3 >= DEBOUNCE_L3)); then - TIER=L3 + want=L3 elif ((C_L2 >= DEBOUNCE_L2)) && { ((psi >= T_PSI_CENTI)) || ((refault >= T_REFAULT_RATE)); }; then - TIER=L2 + want=L2 elif ((C_L1 >= DEBOUNCE_L1)); then - TIER=L1 + want=L1 fi # Projection. Armed only once headroom is already below L1, so a momentary # allocation spike from an idle state cannot vault the ladder. - if [[ $TIER != "L0" ]] && ((proj < T_L4)); then - TIER=L3 + if [[ $want != "L0" ]] && ((proj < T_L4)); then + want=L3 fi - # Cooldown applies to the acting tiers only. L4 is exempt because by then the - # alternative to acting is an oom.group kill of everything in the pod. - if [[ $TIER == "L2" || $TIER == "L3" ]] && ((now - LAST_ACTION_AT < COOLDOWN)); then - TIER=L1 + # Rate limiting, in two independent parts - see SETTLE above for why the + # single 180s cooldown they replace was worse than either. + if [[ $want == "L2" || $want == "L3" ]]; then + if ((LAST_ACTION_AT > 0)) && ((now - LAST_ACTION_AT < SETTLE)); then + SUPPRESSED="settling(${want})" + want=L1 + elif [[ -n ${RUNG_FIRED[$want]:-} ]]; then + SUPPRESSED="already-fired-this-excursion(${want})" + want=L1 + fi fi + + TIER=$want return 0 } @@ -605,12 +866,71 @@ publish_headroom() { return 0 } +# How many processes the current scan found, how many of them the guards claimed, +# and - the number that matters - how many remain eligible to be signalled at +# all. A managed tree with zero eligible processes is the signature of a guard +# that has swallowed everything, and it is worth more than any assertion about a +# particular pid because it does not depend on knowing which pid to ask about. +census_line() { + local pid role protected=0 eligible=0 pty_n=0 + for pid in "${!SERVER_TREE[@]}"; do + if [[ -n ${PROTECTED[$pid]:-} ]]; then + ((protected += 1)) + [[ ${PROTECT_REASON[$pid]:-} == "ptyhost" ]] && ((pty_n += 1)) + continue + fi + role_of "$pid" + [[ $ROLE == "ptyHost" ]] && continue + ((eligible += 1)) + done + printf 'tree=%d protected=%d ptyhost=%d eligible=%d' \ + "${#SERVER_TREE[@]}" "$protected" "$pty_n" "$eligible" +} + +# The question "would enforce mode have been tolerable to live with?" answered +# from observe mode, where it costs nothing to ask. Sheds per hour of uptime is +# the number that decides whether the operator turns this on and leaves it on - +# a watchdog that is switched off protects nothing, however correct each of its +# individual decisions was. +publish_summary() { + local now=$1 h=$2 tier=$3 + local up=$((now - STARTED_AT)) + ((up > 0)) || up=1 + local total=$((${ACTIONS[L2]:-0} + ${ACTIONS[L3]:-0} + ${ACTIONS[L4]:-0})) + { + printf 'mode=%s uptime_s=%d tier=%s\n' "$MODE" "$up" "$tier" + printf 'h=%s h_min=%s h_max=%s\n' \ + "$(fmt_gib "$h")" "$(fmt_gib "$H_MIN_SEEN")" "$(fmt_gib "$H_MAX_SEEN")" + printf 'thresholds l1=%s l2=%s l3=%s l4=%s (reserve=%s of memory.max=%s)\n' \ + "$(fmt_gib "$T_L1")" "$(fmt_gib "$T_L2")" "$(fmt_gib "$T_L3")" \ + "$(fmt_gib "$T_L4")" "$(fmt_gib "$C_RESERVE")" "$(fmt_gib "$M_MAX")" + printf 'excursions=%d sheds l2=%d l3=%d l4=%d total=%d\n' \ + "$EXCURSIONS" "${ACTIONS[L2]:-0}" "${ACTIONS[L3]:-0}" "${ACTIONS[L4]:-0}" "$total" + # Per day rather than per hour: the operator's question is "not every 15 + # minutes", and an hourly rate computed over a few minutes of uptime reads + # as a huge number for one event. + printf 'shed_rate_per_day=%d.%02d\n' \ + "$((total * 86400 / up))" "$((total * 86400 * 100 / up % 100))" + printf '%s\n' "$(census_line)" + } >"${STATE_DIR}/summary.tmp" && + mv -f "${STATE_DIR}/summary.tmp" "${STATE_DIR}/summary" + return 0 +} + +CSV_HEADER="ts,mem_max,mem_current,u,h,anon,shmem,unevictable,slab_unreclaimable,slab_reclaimable,kernel_stack,pagetables,sec_pagetables,percpu,sock,file,psi_full_avg10_centi,refault_file_per_s,pgscan_direct_per_s,tier,server_pid,tree_procs,tree_rss,protected,eligible,t_l1,t_l4" + append_calibration() { - local csv="${STATE_DIR}/calibration.csv" + local csv="${STATE_DIR}/calibration.csv" first="" + if [[ -s $csv ]]; then + # A file written by an earlier version has fewer columns, and appending to it + # would produce one CSV that is silently two different schemas. Rotate it. + read -r first <"$csv" 2>/dev/null + if [[ $first != "$CSV_HEADER" ]]; then + mv -f "$csv" "${csv}.1" 2>/dev/null + fi + fi if [[ ! -s $csv ]]; then - printf '%s\n' \ - "ts,mem_max,mem_current,u,h,anon,shmem,unevictable,slab_unreclaimable,slab_reclaimable,kernel_stack,pagetables,sec_pagetables,percpu,sock,file,psi_full_avg10_centi,refault_file_per_s,pgscan_direct_per_s,tier,server_pid,tree_procs,tree_rss" \ - >"$csv" + printf '%s\n' "$CSV_HEADER" >"$csv" fi printf '%s\n' "$*" >>"$csv" ((CSV_LINES += 1)) @@ -689,13 +1009,29 @@ shed_load() { local -a targets=() select_candidates "$tier" - ((${#CANDIDATES[@]})) || return 0 + + # An acting tier that signals nothing is the exact shape of the bug that hid a + # broken guard through two clean-looking test runs: enforce mode logged + # tier=L3 with no signal and no REFUSED line, a state the code is otherwise + # supposed to make impossible. It is now impossible for a different reason - + # every path out of here says something. + if ((${#CANDIDATES[@]} == 0)); then + log_action "no-candidates tier=${tier} $(census_line)" + return 0 + fi for row in "${CANDIDATES[@]}"; do rss=${row%% *} pid=${row#* } role=${pid#* } pid=${pid%% *} + # Below L4, a shed has to pay for itself. Killing something small does not + # move headroom, and it spends the rung as surely as killing something big + # would - see MIN_SHED_RSS. At L4 the tree is going down regardless. + if [[ $tier != "L4" ]] && ((rss < MIN_SHED_RSS)); then + log_action "no-worthwhile-candidate tier=${tier} best=${pid} role=${role} rss=${rss} min=${MIN_SHED_RSS}" + break + fi if signal_pid TERM "$pid" "${tier} ${role} rss=${rss}"; then acted=1 targets+=("$pid") @@ -718,7 +1054,11 @@ shed_load() { done fi - ((acted)) && LAST_ACTION_AT=$now + if ((acted)); then + LAST_ACTION_AT=$now + RUNG_FIRED[$tier]=1 + ACTIONS[$tier]=$((${ACTIONS[$tier]:-0} + 1)) + fi return 0 } @@ -771,9 +1111,25 @@ scan_once() { return 2 fi ((rc == 0)) || return 1 + # Derived once, from the pod's own limit, on the first successful read. + if [[ -z $T_L4 ]]; then + derive_limits "$M_MAX" + log_action "derived memory.max=$(fmt_gib "$M_MAX") reserve=$(fmt_gib "$C_RESERVE") ladder L1=$(fmt_gib "$T_L1") L2=$(fmt_gib "$T_L2") L3=$(fmt_gib "$T_L3") L4=$(fmt_gib "$T_L4")" + local r + for r in serverMain extensionHost tsserver languageServer fileWatcher; do + log_action "derived ceiling role=${r} rlimit_data=$(fmt_gib "${CEILING[$r]}")" + done + if ((TOO_SMALL)) && [[ $MODE == "enforce" ]]; then + MODE=observe + log_action "WARNING memory.max=$(fmt_gib "$M_MAX") leaves no room above L1=$(fmt_gib "$T_L1"); a pod this size would sit in the shedding tiers permanently, so enforce mode is refused and this run is observe-only" + fi + fi read_cgroup_pressure read_process_table + ((M_H > H_MAX_SEEN)) && H_MAX_SEEN=$M_H + ((H_MIN_SEEN == 0 || M_H < H_MIN_SEEN)) && H_MIN_SEEN=$M_H + # Rates. Elapsed time is tracked explicitly because the sample interval is # adaptive, so a fixed denominator would be wrong exactly when it matters. local dt=$((now - PREV_AT)) @@ -797,14 +1153,22 @@ scan_once() { tree_rss=$((tree_rss + ${P_RSS[$pid]:-0})) done + local census + census="$(census_line)" + local n_protected=${census#*protected=} + n_protected=${n_protected%% *} + local n_eligible=${census##*eligible=} + if ((CYCLE % CALIBRATION_EVERY == 0)); then - append_calibration "${now},${M_MAX},${M_CURRENT},${M_U},${M_H},${M_ANON},${M_SHMEM},${M_UNEVICTABLE},${M_SLAB_UNRECLAIMABLE},${M_SLAB_RECLAIMABLE},${M_KERNEL_STACK},${M_PAGETABLES},${M_SEC_PAGETABLES},${M_PERCPU},${M_SOCK},${M_FILE},${M_PSI_CENTI},${refault_rate},${pgscan_rate},${TIER},${SERVER_PID:--},${#SERVER_TREE[@]},${tree_rss}" + append_calibration "${now},${M_MAX},${M_CURRENT},${M_U},${M_H},${M_ANON},${M_SHMEM},${M_UNEVICTABLE},${M_SLAB_UNRECLAIMABLE},${M_SLAB_RECLAIMABLE},${M_KERNEL_STACK},${M_PAGETABLES},${M_SEC_PAGETABLES},${M_PERCPU},${M_SOCK},${M_FILE},${M_PSI_CENTI},${refault_rate},${pgscan_rate},${TIER},${SERVER_PID:--},${#SERVER_TREE[@]},${tree_rss},${n_protected},${n_eligible},${T_L1},${T_L4}" fi [[ -n $SERVER_PID ]] && apply_ceilings + publish_summary "$now" "$M_H" "$TIER" + check_calibration if [[ $TIER != "L0" && $TIER != "$PREV_TIER" ]]; then - log_action "tier=${TIER} h=$(fmt_gib "$M_H") u=$(fmt_gib "$M_U") psi_full10=${M_PSI_CENTI} refault/s=${refault_rate} dU/s=${du_rate} projected=$(fmt_gib "$projected") tree=${#SERVER_TREE[@]}" + log_action "tier=${TIER} h=$(fmt_gib "$M_H") u=$(fmt_gib "$M_U") psi_full10=${M_PSI_CENTI} refault/s=${refault_rate} dU/s=${du_rate} projected=$(fmt_gib "$projected") ${census}${SUPPRESSED:+ suppressed=${SUPPRESSED}}" fi case "$TIER" in @@ -819,16 +1183,22 @@ scan_once() { return 0 } -warn_if_thresholds_oversized() { - local raw="" - read -r raw <"${CGROUP_DIR}/memory.max" 2>/dev/null - [[ $raw =~ ^[0-9]+$ ]] || return 0 - # An 8 GiB pod rests at U ~1.9 GiB and so has ~6 GiB of headroom. A 4 GiB pod - # has ~2 GiB and would sit permanently at L1/L2 against these absolute - # thresholds. That is a calibration problem, not something to paper over with a - # clamp, so say it once, loudly, in the log the operator will actually read. - ((T_L1 * 2 > raw)) && - log_action "WARNING memory.max=${raw} is small relative to T_L1=${T_L1}; the ladder will sit low permanently - recalibrate before enabling enforce mode" +# The previous version of this check compared T_L1 against memory.max and warned +# when the ratio looked wrong - a hardcoded rule about hardcoded numbers, which +# could only ever restate the assumption it was meant to test. Now that the +# ladder is derived, the honest check is the observation itself: has this pod, in +# its own life, ever had enough headroom to sit above the first rung? If not, the +# derivation is wrong for this workload whatever the arithmetic says, and the +# operator should see that before switching enforce on rather than after. +CALIBRATION_WARMUP="${WATCHDOG_CALIBRATION_WARMUP:-30}" # cycles + +check_calibration() { + ((CALIBRATION_WARNED)) && return 0 + ((CYCLE >= CALIBRATION_WARMUP)) || return 0 + CALIBRATION_WARNED=1 + if ((H_MAX_SEEN < T_L1)); then + log_action "WARNING best headroom seen since start is $(fmt_gib "$H_MAX_SEEN"), below L1=$(fmt_gib "$T_L1") on memory.max=$(fmt_gib "$M_MAX") - this pod never leaves the ladder, so recalibrate or resize before enabling enforce mode" + fi return 0 } @@ -842,10 +1212,10 @@ main() { trap 'release_singleton; exit 0' HUP INT TERM trap release_singleton EXIT - log_action "started mode=${MODE} pid=$$ cgroup=${CGROUP_DIR} thresholds L1=${T_L1} L2=${T_L2} L3=${T_L3} L4=${T_L4}" - warn_if_thresholds_oversized - local now interval + STARTED_AT=${WATCHDOG_NOW:-$EPOCHSECONDS} + log_action "started mode=${MODE} pid=$$ cgroup=${CGROUP_DIR}" + while :; do now=${WATCHDOG_NOW:-$EPOCHSECONDS} scan_once "$now" From 20fd689b4dfb9a08abc3e9cc700b443ce948b6c3 Mon Sep 17 00:00:00 2001 From: Peter Pathirana Date: Mon, 17 Aug 2026 21:15:35 +0000 Subject: [PATCH 2/2] fix: make the watchdog survive an OOM restart and see a real spike Three defects found by running the watchdog against a live 4 GiB workspace and against a reproduction of the failure the pod actually has, rather than against fixtures. Each has a regression test built from the measurement. A stale pidfile disarmed the watchdog permanently. When the pod is OOM-killed the watchdog dies by SIGKILL, its EXIT trap never runs, and its pidfile survives on the NFS-backed home. `acquire_singleton` treated a failed O_EXCL create as proof another instance held the lock, so every restart afterwards logged "another instance is already running" and exited - leaving the pod unwatched from the first kill onward. Liveness now decides, and identity is the script's own path as a whole argv element rather than a substring. Ceilings were computed against the wrong quantity. RLIMIT_DATA accounts `data_vm`, not RSS, and on a V8 process these differ by an order of magnitude: the live extension host held 1004 MB of data against 497 MB resident, the file watcher 622 MB against 66 MB. The derived file-watcher ceiling was 512 MB - below what an idle file watcher already held - so enforcing it would have killed a healthy process on its next allocation and on every restart after. A ceiling is now a growth allowance above observed usage, and a role that could only be capped above memory.max is reported instead of capped. The poll interval could not see the event it exists for. A runaway growing at the production rate took the test pod from idle to OOMKilled in 43 seconds; the watchdog ran in enforce mode throughout, never left L0, and logged nothing, because a 10-second idle interval gives four samples and a three-sample debounce cannot climb three rungs in four samples. The interval is now keyed on measured dU/dt rather than on tier, which is the lagging indicator of the thing being raced. Re-run against the identical spike, the ladder climbed L0 to L3 to L4 as designed. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 5 +- DESIGN.md | 6 +- .../script-memory-watchdog-test.sh | 206 +++++++++++++++++- .../script-memory-watchdog.sh | 139 ++++++++++-- 4 files changed, 329 insertions(+), 27 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 69ec8493..d0c6011d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -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_` 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-/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). diff --git a/DESIGN.md b/DESIGN.md index 1e1b3c64..c5265543 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -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 diff --git a/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh b/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh index 6a4f1b2c..7ec6277d 100755 --- a/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh +++ b/templates/kubernetes/homelab-workspace/script-memory-watchdog-test.sh @@ -116,7 +116,12 @@ add_proc() { mkdir -p "$d" printf '%s (%s) S %s 0 0 0 -1 4194560 0 0 0 0 0 0 0 0 20 0 1 0 0\n' \ "$pid" "$comm" "$ppid" >"${d}/stat" - printf '0 %s 0 0 0 0 0\n' "$((rss / 4096))" >"${d}/statm" + # statm: size resident shared text lib data dt. `data` is what RLIMIT_DATA + # accounts, and on a real V8 process it is several times resident - see + # set_proc_data, and read_rss() in the watchdog for the live measurements. + # Defaulting it to resident keeps the fixtures honest about which field is + # being read without asserting a relationship that does not hold. + printf '0 %s 0 0 0 %s 0\n' "$((rss / 4096))" "$((rss / 4096))" >"${d}/statm" cat >"${d}/limits" <<'EOF' Limit Soft Limit Hard Limit Units Max data size unlimited unlimited bytes @@ -128,6 +133,12 @@ EOF done } +# set_proc_data +set_proc_data() { + local dir=$1 pid=$2 rss=$3 data=$4 + printf '0 %s 0 0 0 %s 0\n' "$((rss / 4096))" "$((data / 4096))" >"${dir}/${pid}/statm" +} + # A representative tree, transcribed from `ps -eo pid,ppid,comm,args` on a live # workspace with a VS Code server attached. Argument shapes, argv[0] paths and - # the part that matters most - the `comm` values are what that capture showed, @@ -897,7 +908,7 @@ test_derivation() { assert_eq 536870912 "${CEILING[fileWatcher]}" "and floors the small ones rather than shrinking them to nothing" local role for role in serverMain extensionHost tsserver languageServer fileWatcher; do - if ((CEILING[role] < 4294967296)); then + if ((${CEILING[$role]} < 4294967296)); then ok "4 GiB pod: the ${role} ceiling is inside the pod" else bad "4 GiB pod: the ${role} ceiling (${CEILING[$role]}) is the whole pod - inert" @@ -990,6 +1001,194 @@ test_shedding_is_rate_limited() { assert_eq 2 "$EXCURSIONS" "recovering and falling again does" } +# --------------------------------------------------------------------------- # +# 4d. a ceiling is never a kill order for a healthy process +# +# This is a regression test for a defect found by running the derivation against +# a live 4 GiB workspace rather than against these fixtures. RLIMIT_DATA accounts +# data_vm, not RSS, and on a V8 process the two differ by roughly an order of +# magnitude: the live file watcher held 622 MB of data while resident in 66 MB. +# The derived file-watcher ceiling for that pod was 512 MB - below what the +# process already had - so enforcing it would have killed a perfectly healthy +# file watcher on its next allocation, and again on every restart. +# +# The numbers below are that measurement, not an invention. +# --------------------------------------------------------------------------- # + +# CEILING_LOGGED is a memo global of the sourced watchdog, cleared here so the +# second half of the test can observe a fresh proposal. +# shellcheck disable=SC2034 +test_ceiling_is_never_below_observed_usage() { + printf 'a ceiling is never below what the process already holds\n' + write_cgroup "${WORK}/cg" 4294967296 0.00 + local pdir="${WORK}/proc4d" + build_tree "$pdir" + # Live 4 GiB workspace, at rest, VS Code 1.132: pid 918 fileWatcher and pid + # 907 extensionHost. + set_proc_data "$pdir" 42 67000000 637184000 + set_proc_data "$pdir" 41 509220000 1028004000 + rm -rf "${WORK}/state" + load_watchdog "${WORK}/cg" "$pdir" + scan_fixture + + # The case is real only if the derived ceiling really is below observed usage. + # Without this the assertion below would pass on a pod where nothing was wrong. + if ((CEILING[fileWatcher] < P_DATA[42])); then + ok "the derived file-watcher ceiling really is below observed usage on a 4 GiB pod" + else + bad "the fixture does not reproduce the condition - the test proves nothing" + fi + + apply_ceilings + local log + log="$(cat "${WORK}/state/actions.log")" + local line want + for pid in 42 41; do + line=$(printf '%s\n' "$log" | grep "pid=${pid} " | head -1) + want=${line##*rlimit_data=} + want=${want%% *} + if [[ -n $want ]] && ((want > ${P_DATA[$pid]:-0})); then + ok "pid ${pid}: proposed ceiling ${want} is above its observed ${P_DATA[$pid]:-0} bytes of data" + else + bad "pid ${pid}: proposed ceiling '${want}' would kill it at once (data=${P_DATA[$pid]:-0})" + fi + done + + # And the growth allowance is the reserve, so the ceiling means "may grow by + # this much", not "may be this big". + line=$(printf '%s\n' "$log" | grep "pid=42 " | head -1) + want=${line##*rlimit_data=} + want=${want%% *} + assert_eq "$((P_DATA[42] + 2 * C_RESERVE))" "$want" \ + "the ceiling is observed usage plus two critical reserves" + + # A ceiling that could only be set above memory.max bounds nothing. Saying so + # is better than setting it and looking protected. + set_proc_data "$pdir" 42 67000000 4000000000 + read_rss 42 + : >"${WORK}/state/actions.log" + CEILING_LOGGED=() + apply_ceilings + if [[ "$(cat "${WORK}/state/actions.log")" == *"no-ceiling pid=42"* ]]; then + ok "a process already too large to cap is reported, not silently capped" + else + bad "a process too large to cap was handled silently" + fi +} + +# --------------------------------------------------------------------------- # +# 4e. the sample interval is chosen by rate, not by tier +# +# Measured, not supposed. A runaway at the rate seen in production took the test +# workspace from idle to OOMKilled in 43 seconds while the watchdog ran in +# enforce mode, never left L0 and logged nothing: at a 10-second idle interval it +# had four samples in which to satisfy a three-sample debounce and climb three +# rungs. Tier cannot be the input to the interval, because tier is the lagging +# indicator of the very thing being raced. +# --------------------------------------------------------------------------- # + +# shellcheck disable=SC2034 # TIME_TO_LIMIT and PREV_TIER are the sourced globals +test_interval_is_chosen_by_rate() { + printf 'the sample interval is chosen by rate\n' + write_cgroup "${WORK}/cg" 4294967296 0.00 + load_watchdog "${WORK}/cg" "${WORK}/proc2" + + TIME_TO_LIMIT=0 + PREV_TIER=L0 + assert_eq 10 "$(next_interval)" "an idle pod that is not growing polls slowly" + + PREV_TIER=L1 + assert_eq 2 "$(next_interval)" "a pod already on the ladder polls faster" + + # 3.96 GiB of headroom disappearing at 91 MB/s - the reproduced production + # rate - is 43 seconds from the limit while still reading L0. + TIME_TO_LIMIT=43 + PREV_TIER=L0 + assert_eq 1 "$(next_interval)" \ + "but a rate that reaches the limit inside the horizon overrides L0 entirely" + + # The case that must not become chatty: growth so slow it will never matter. + TIME_TO_LIMIT=3600 + PREV_TIER=L0 + assert_eq 10 "$(next_interval)" "slow growth does not spin the loop up" + + # With a 1-second interval the debounce that could not complete in the live run + # completes with time to spare: three samples is three seconds, against the 43 + # the event took. + if ((DEBOUNCE_L1 * 1 < 43 && DEBOUNCE_L3 * 1 < 43)); then + ok "at the fast interval the debounces fit inside the observed event" + else + bad "the debounces still cannot complete inside a 43-second event" + fi +} + +# --------------------------------------------------------------------------- # +# 4f. a stale pidfile does not disarm the watchdog forever +# +# Found on the live test workspace, not here. The pod was OOM-killed, the +# watchdog died by SIGKILL without running its EXIT trap, and its pidfile +# survived on the NFS-backed home directory. Every restart afterwards logged +# "another instance is already running" and exited - so the first kill left the +# pod unwatched from then on, which is the worst possible time for that. +# --------------------------------------------------------------------------- # + +test_singleton_survives_a_hard_kill() { + printf 'a stale pidfile does not disarm the watchdog\n' + write_cgroup "${WORK}/cg" 4294967296 0.00 + local pdir="${WORK}/proc4f" + build_tree "$pdir" + rm -rf "${WORK}/state" + load_watchdog "${WORK}/cg" "$pdir" + + # A pid from the previous container that no longer exists at all. + printf '99999\n' >"${WORK}/state/watchdog.pid" + if acquire_singleton; then + ok "a pidfile naming a dead process is taken over" + else + bad "a dead process's pidfile locks the watchdog out" + fi + + # A pid that does exist in this container but is something else entirely - + # the recycled-pid case, which is the normal case after a restart. + printf '44\n' >"${WORK}/state/watchdog.pid" + if acquire_singleton; then + ok "a pidfile naming an unrelated live process is taken over" + else + bad "an unrelated process holding a recycled pid locks the watchdog out" + fi + + # An empty pidfile - what a truncated or half-written file looks like. + : >"${WORK}/state/watchdog.pid" + if acquire_singleton; then + ok "an empty pidfile is taken over" + else + bad "an empty pidfile locks the watchdog out" + fi + + # And the mutation that must flip it: a live process genuinely running this + # script. Without this, the three assertions above would pass equally against a + # watchdog with no singleton guard at all. + local d="${pdir}/4242" + mkdir -p "$d" + printf '4242 (bash) S 1 0 0 0 -1 4194560 0 0 0 0 0 0 0 0 20 0 1 0 0\n' >"${d}/stat" + printf '/bin/bash\0%s\0' "${SELF_DIR}/script-memory-watchdog.sh" >"${d}/cmdline" + printf '4242\n' >"${WORK}/state/watchdog.pid" + if acquire_singleton; then + bad "a genuinely running watchdog did not stop a second one" + else + ok "a live process running this same script does hold the lock" + fi + + # Releasing must not steal a pidfile owned by someone else. + printf '4242\n' >"${WORK}/state/watchdog.pid" + release_singleton + if [[ -s "${WORK}/state/watchdog.pid" ]]; then + ok "releasing leaves another instance's pidfile alone" + else + bad "releasing removed a pidfile this instance did not own" + fi +} + # --------------------------------------------------------------------------- # # 5. observe mode really is inert # --------------------------------------------------------------------------- # @@ -1041,6 +1240,9 @@ main() { test_tiers test_derivation test_shedding_is_rate_limited + test_ceiling_is_never_below_observed_usage + test_interval_is_chosen_by_rate + test_singleton_survives_a_hard_kill test_observe_mode_is_inert printf '\n%d passed, %d failed\n' "$PASS" "$FAIL" ((FAIL == 0)) diff --git a/templates/kubernetes/homelab-workspace/script-memory-watchdog.sh b/templates/kubernetes/homelab-workspace/script-memory-watchdog.sh index 69c30ebb..b9369798 100644 --- a/templates/kubernetes/homelab-workspace/script-memory-watchdog.sh +++ b/templates/kubernetes/homelab-workspace/script-memory-watchdog.sh @@ -50,8 +50,25 @@ PAGE_SIZE=4096 # Sampling. The interval shortens under pressure so the ladder can outrun a # process that allocates a gigabyte in a few seconds. +# +# INTERVAL_FAST and FAST_HORIZON exist because the original two-speed scheme was +# measured against a real event and lost. Reproduced on the test workspace, a +# runaway growing at the rate seen in production took the pod from idle to +# OOMKilled in 43 seconds; the watchdog was running in enforce mode throughout, +# never left L0, and logged no action at all. At a 10-second idle interval it +# got four samples, and a three-sample debounce cannot complete inside four +# samples that also have to cross three rungs. +# +# The fix is to key the interval on the measured rate rather than on the tier: +# once dU/dt implies the limit is reachable inside FAST_HORIZON, sample every +# second, whatever tier the ladder currently believes it is in. Tier is a lagging +# indicator of exactly the thing being raced. This changes only how often the +# rules are evaluated, not what they decide - the corroboration and debounce that +# keep L2 honest are untouched. INTERVAL_IDLE="${WATCHDOG_INTERVAL_IDLE:-10}" INTERVAL_BUSY="${WATCHDOG_INTERVAL_BUSY:-2}" +INTERVAL_FAST="${WATCHDOG_INTERVAL_FAST:-1}" +FAST_HORIZON="${WATCHDOG_FAST_HORIZON:-120}" # seconds to memory.max CALIBRATION_EVERY="${WATCHDOG_CALIBRATION_EVERY:-6}" # cycles between CSV rows # Tier thresholds, in absolute bytes of headroom H, derived from the pod's own @@ -182,7 +199,7 @@ MAX_CSV_LINES="${WATCHDOG_MAX_CSV_LINES:-50000}" # Scan state. Declared at file scope, not inside main(), so that sourcing the # script with WATCHDOG_SOURCE_ONLY=1 gives the test harness correctly-typed # globals without having to restate them. -declare -gA P_COMM=() P_CMD=() P_ARGV0=() P_PPID=() P_RSS=() CHILDREN=() +declare -gA P_COMM=() P_CMD=() P_ARGV0=() P_PPID=() P_RSS=() P_DATA=() CHILDREN=() declare -gA SERVER_TREE=() PROTECTED=() PROTECT_REASON=() WATCHDOG_KIN=() declare -ga PIDS=() CANDIDATES=() SERVER_ROOTS=() SERVER_PID="" @@ -195,6 +212,7 @@ PREV_U=0 PREV_REFAULT=0 PREV_PGSCAN=0 CYCLE=0 +TIME_TO_LIMIT=0 H_MAX_SEEN=0 H_MIN_SEEN=0 CALIBRATION_WARNED=0 @@ -355,6 +373,7 @@ read_process_table() { P_ARGV0=() P_PPID=() P_RSS=() + P_DATA=() CHILDREN=() local entry pid line rest comm ppid @@ -390,11 +409,27 @@ read_process_table() { return 0 } +# RSS, and the quantity RLIMIT_DATA actually accounts. +# +# These are not interchangeable and the difference is not small. RLIMIT_DATA +# limits mm->data_vm - private writable anonymous mappings - which for a V8 +# process is dominated by *reserved* address space rather than resident pages. +# 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 of data. A ceiling reasoned about as though it bounded RSS is therefore +# roughly an order of magnitude tighter than intended, and the first derived +# ceilings this script produced were *below* what an idle file watcher already +# held - which would have killed it on its next allocation, every time, forever. +# +# statm's sixth field is data_vm + stack_vm, so it overstates by the stack (a few +# hundred kB here). That is accepted rather than read a second file per process +# per scan: the allowance added on top is measured in hundreds of megabytes. read_rss() { - local pid res + local pid res data for pid in "$@"; do - read -r _ res _ <"${PROC_DIR}/${pid}/statm" 2>/dev/null || continue + read -r _ res _ _ _ data _ <"${PROC_DIR}/${pid}/statm" 2>/dev/null || continue P_RSS[$pid]=$((res * PAGE_SIZE)) + P_DATA[$pid]=$((data * PAGE_SIZE)) done return 0 } @@ -953,7 +988,7 @@ declare -gA CEILING_LOGGED=() # proactive mechanism and it must also cover processes spawned while the pod is # already under pressure - not only while it is idle. apply_ceilings() { - local pid want cur + local pid want cur floor obs local -a f for pid in "${!SERVER_TREE[@]}"; do role_of "$pid" @@ -962,6 +997,26 @@ apply_ceilings() { want=${CEILING[$ROLE]:-} [[ -n $want ]] || continue + # A ceiling must be a growth allowance above what the process already holds, + # never an absolute size derived from the pod alone. The derived value says + # what the pod can afford; this says what the process demonstrably needs + # right now. Taking the larger of the two is what stops a correctly-reasoned + # budget from being an instant kill order for a healthy process - see + # read_rss() for the measurements that made this necessary. + obs=${P_DATA[$pid]:-0} + floor=$((obs + 2 * C_RESERVE)) + ((want < floor)) && want=$floor + + # A ceiling at or above the pod's own limit cannot bound anything, and + # setting one would only look like protection. Say so instead. + if ((want >= M_MAX)); then + if [[ -z ${CEILING_LOGGED[${pid}:inert]:-} ]]; then + CEILING_LOGGED[${pid}:inert]=1 + log_action "no-ceiling pid=${pid} role=${ROLE} data=${obs} would-need=${want} memory.max=${M_MAX} (a cap this size cannot bound anything)" + fi + continue + fi + cur="" while read -r -a f; do [[ ${f[0]:-} == "Max" && ${f[1]:-} == "data" && ${f[2]:-} == "size" ]] || continue @@ -1071,33 +1126,53 @@ shed_load() { # atomic O_EXCL create, which is all that is needed here: /usr/bin/flock does # exist in the image, but it would only add a fork and a held descriptor to get # the same guarantee. +# Liveness decides, not the existence of a file. +# +# The previous version treated a failed O_EXCL create as evidence that another +# watchdog held the lock, and only then looked at whether the recorded pid was +# alive. That inverts the reliable test and the unreliable one, and it failed in +# exactly the situation this daemon exists for: the pod was OOM-killed, the +# watchdog died by SIGKILL without running its EXIT trap, and the pidfile +# survived on the NFS-backed home directory. Every restart afterwards logged +# "another instance is already running" and exited, so the first kill left the +# pod permanently unwatched - observed on the test workspace, not theorised. +# +# The identity check is the script's own path as a whole argv element, not a +# substring of the command line. A recycled pid in a fresh container would have +# to be running this same script for the check to match, which is precisely the +# case where refusing to start is correct. acquire_singleton() { local pidfile="${STATE_DIR}/watchdog.pid" - local other="" + local self="${BASH_SOURCE[0]}" + local other="" a local -a argv=() - if ( - set -o noclobber - printf '%s\n' "$$" >"$pidfile" - ) 2>/dev/null; then - return 0 + if [[ -s $pidfile ]]; then + read -r other <"$pidfile" 2>/dev/null + if [[ $other =~ ^[0-9]+$ ]] && ((other != $$)) && + [[ -r ${PROC_DIR}/${other}/cmdline ]]; then + mapfile -d '' -t argv <"${PROC_DIR}/${other}/cmdline" 2>/dev/null + for a in "${argv[@]}"; do + [[ $a == "$self" ]] && return 1 + done + fi fi + printf '%s\n' "$$" >"$pidfile" 2>/dev/null || return 1 + # Two watchdogs started at the same instant would both get this far. Settle it + # by reading back who actually owns the file rather than by trusting the write. + /usr/bin/sleep 1 read -r other <"$pidfile" 2>/dev/null - if [[ $other =~ ^[0-9]+$ ]] && [[ -r ${PROC_DIR}/${other}/cmdline ]]; then - mapfile -d '' -t argv <"${PROC_DIR}/${other}/cmdline" 2>/dev/null - [[ "${argv[*]}" == *memory-watchdog* ]] && return 1 - fi - - rm -f "$pidfile" - ( - set -o noclobber - printf '%s\n' "$$" >"$pidfile" - ) 2>/dev/null + [[ $other == "$$" ]] } release_singleton() { - rm -f "${STATE_DIR}/watchdog.pid" + local pidfile="${STATE_DIR}/watchdog.pid" owner="" + read -r owner <"$pidfile" 2>/dev/null + # Never remove a pidfile another instance owns - that would hand the lock to a + # third one while the second is still running. + [[ $owner == "$$" ]] && rm -f "$pidfile" + return 0 } # Returns 2 when the cgroup has no memory limit and there is nothing to protect. @@ -1142,6 +1217,13 @@ scan_once() { fi local projected=$((M_H - du_rate * PROJECTION_HORIZON)) + # Seconds until U reaches memory.max at the currently observed rate. Zero means + # "not growing", and is the normal reading. This is what chooses the next + # interval, and it is deliberately a raw single-interval rate rather than a + # smoothed one: smoothing is what would hide the only event shape that matters. + TIME_TO_LIMIT=0 + ((du_rate > 0)) && TIME_TO_LIMIT=$((M_H / du_rate)) + build_server_tree compute_protected @@ -1192,6 +1274,18 @@ scan_once() { # operator should see that before switching enforce on rather than after. CALIBRATION_WARMUP="${WATCHDOG_CALIBRATION_WARMUP:-30}" # cycles +# How long to wait before the next scan. Rate first, tier second - see +# INTERVAL_FAST for the measurement that made that ordering necessary. +next_interval() { + if ((TIME_TO_LIMIT > 0 && TIME_TO_LIMIT < FAST_HORIZON)); then + printf '%s' "$INTERVAL_FAST" + elif [[ $PREV_TIER == "L0" ]]; then + printf '%s' "$INTERVAL_IDLE" + else + printf '%s' "$INTERVAL_BUSY" + fi +} + check_calibration() { ((CALIBRATION_WARNED)) && return 0 ((CYCLE >= CALIBRATION_WARMUP)) || return 0 @@ -1224,8 +1318,7 @@ main() { [[ $ONESHOT == "1" ]] && break - interval=$INTERVAL_IDLE - [[ $PREV_TIER == "L0" ]] || interval=$INTERVAL_BUSY + interval=$(next_interval) /usr/bin/sleep "$interval" done return 0