fix: declare what each watchdog budget is anchored on, and stop arming guesses - #883
Closed
ppat wants to merge 1 commit into
Closed
fix: declare what each watchdog budget is anchored on, and stop arming guesses#883ppat wants to merge 1 commit into
ppat wants to merge 1 commit into
Conversation
…g guesses No budget value changes. What changes is that each one now says where it came from, and the default mode's arming is derived from that. The formula is asymmetric. `resting x 1.5` is a floor, so a budget can never ship below what its role was measured holding - the failure this design shipped twice - but nothing bounds a budget from above. A number derived from a measurement and a round number reached for in the absence of one therefore sat in the same column, printed the same `event=budget` line, and were enforced identically. Three of the four roles with no RESTING_ROLE entry carried exactly 512 MiB. A derived ceiling is the wrong fix and is not attempted. The whole sample is one operator, one Terraform/YAML repository, one 38-hour window in which the only member extensionHelper has ever had is a 24 MB terraform-ls. A ceiling fitted to that would put a healthy gopls or Pyright server under a limit set by a language server that was never running. So: BUDGET_BASIS declares `measured` / `adversary` / `guess` per role, `event=budget` carries `basis=`, UNKNOWN_ROLES is derived from it rather than hand-listed, and `enforce` requires both axes - low blast radius AND a budget with a measurement behind it. That moves two roles out of the default mode, and tsserver is why the rule is not a formality: VS Code launches it with --max-old-space-size=3072 (its shipped typescript.tsserver.maxTsServerMemory default, observed on both instances in the live pod, configured nowhere here). The 768 MiB budget is a quarter of the heap its own launcher sanctions, so on a large TypeScript project a healthy tsserver would dwell over budget, be killed, cost a multi-minute re-index with dead IntelliSense, and repeat until the breaker disarmed the role. Nothing in this workspace's data could contradict that: its two tsservers sat between 112 and 208 MiB for 29 hours on a repository with no TypeScript in it. extensionHelper is the same shape. Both keep their budget, dwell clock and `event=would-kill armed=no` line, which is the evidence needed to arm them later. RESTING_ROLE[fileWatcher] 88 -> 107 MiB: flat at 107 MiB for 28 straight hours, so the reference was stale. It moves no budget - 107 x 1.5 is still far below the declared 256 MiB. RESTING_ROLE[extensionHost] is deliberately NOT moved despite a 738 MiB peak against its 713 MiB figure, because its hourly minimum climbed 528 -> 686 MiB over the same window: that is drift, not staleness, and raising the reference would widen the budget of the only role in the pod that is drifting. A flat floor above the reference is a measurement to correct; a rising floor above it is a finding to report. Also fixes a wrong assertion description found while doing this: a test read "while a role with a measured resting size is armed" about extensionHelper, which has never had one. Every new assertion is paired with the production mutation that flips it red, and all eight were run: reverting the enforce clause, hand-listing UNKNOWN_ROLES, mislabelling a basis in either direction, dropping `basis=` from the log line, adding a role with no basis, dropping one from BUDGET_ORDER, and lowering a declared budget below its floor.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Superseded by #884 — closed unmerged
This PR's central mechanism was overruled by the operator before it landed, and #884 replaces the
construction it was built on. Nothing here should be revived as-is.
What this PR did: declared a
BUDGET_BASISper role (measured/adversary/guess),published it as
basis=inevent=budget, derivedUNKNOWN_ROLESfrom it rather than ahand-maintained list, and disarmed guess-anchored roles (
tsserver,extensionHelper) underenforceon the reasoning that killing on an underived number is worse than not killing.Why it is superseded:
enforce— things that exceed their share get killed. A mechanism that declines to enforcewhere it is least sure is inert exactly where it matters.
BUDGET_BASISloses its meaning under feat: give the VS Code tree one memory envelope and shares of it #884. Budgets are no longer per-role numbers withindependent provenance; each role holds a named share of a fixed 2048 MiB envelope for the VS
Code tree. A share is allocated. It is neither measured nor guessed, so the taxonomy has
nothing to classify.
RESTING_ROLEis deleted by feat: give the VS Code tree one memory envelope and shares of it #884. Theresting x 1.5floor is gone, so this PR'scorrection of the
fileWatcherresting reference (88 -> 107 MiB) has no surface left to applyto. feat: give the VS Code tree one memory envelope and shares of it #884 carries the 107 MiB measurement forward as an input to its apportionment instead.
Salvaged into #884, with attribution: the
BUDGET_ORDERdeterminism work (orthogonal to theconstruction and worth keeping); the observation that
tsserverlaunches with--max-old-space-size=3072, re-verified live; and the 107 MiBfileWatchermeasurement.Findings recorded here that outlived the PR, because they are the reasoning behind #884's
shape and are not restated in full there:
max(min(role budget, memory.max / 8), resting x 1.5)was asymmetric.The resting term is a floor, so a budget could never ship below a role's measured resting size —
the failure this design shipped twice, most memorably an
RLIMIT_DATAceiling below what an idlefile watcher already held. Nothing bounded a budget from above, so a number derived from
measurement and a round number reached for in its absence sat in the same column and were
enforced identically. Three of the four roles with no resting entry carried exactly 512 MiB.
The sample is one operator, one Terraform/YAML repository, one 38-hour window whose first ~9
hours had no VS Code tree connected. Across the managed population that window contains eight
distinct processes — 1720 rows for a role is one process sampled 1720 times, not 1720
observations. A ceiling fitted to that would put a healthy
gopls, Pyright orrust-analyzerunder a limit set by a language server that was never running.
tsserveris launched with--max-old-space-size=3072(observed live on both instances;VS Code's shipped default, set nowhere in the workspace's settings). Its 768 MiB budget was a
quarter of the heap its own launcher sanctioned, while it was armed by default. This is now
addressed at source in
ppat/dotfiles#789, which caps it at 768 MiB viajs/ts.tsserver.maxMemory— and which found that TypeScript spawns two tsserver processes that read the setting
independently, so the previous 2048 MiB value sanctioned up to 4096 MiB for tsserver alone.
that lacks it (
extensionHelper"has a measured resting size" — it never had one). Fixed in feat: give the VS Code tree one memory envelope and shares of it #884.