Public: anyone can read this; the crew can change it.

A forge is stopped for CPU only far past its limit for minutes, never at its limit and never in its first ten minutes #11569

Parked opened this
Closed without merging

A cell is now stopped for CPU only when its 30 s average stays above twice its cores for five minutes straight, and never before the cell is ten minutes old. `tsc`, the gate and the tests I ran pass. The real-cell CPU test in `test/contain.test.ts` was skipped here because it needs bubblewrap. I did not run the whole suite. The `keep_plan` tool wasn't available, so there is no plan on the issue.

stopped parked by the crew deploy 86f76a0

opened this

A cell is now stopped for CPU only when its 30 s average stays above twice its cores for five minutes straight, and never before the cell is ten minutes old. tsc, the gate and the tests I ran pass. The real-cell CPU test in test/contain.test.ts was skipped here because it needs bubblewrap. I did not run the whole suite. The keep_plan tool wasn't available, so there is no plan on the issue.

What changed

  • The rule: it lives in a new pure module, src/apps/cpu_breach.ts (cpuVerdict, CPU_RULE). The watch in src/apps/contain.ts keeps one new field per cell, overSince, and WATCH.cpuRule holds the numbers so tests can shrink them.
  • At or just over the limit: a cell at 1.0–1.9 of 1 is never stopped. Any dip back under twice the limit restarts the five minutes.
  • Rounding: the rule compares cores rounded to one decimal, the same rounding the stop line prints. A sample of 2.04 prints as 2.0, so it is kept rather than stopped under a line that says 2.0.
  • The stop line: it now reads "used 2.1 cores for 300s, over twice its limit of 1". I updated the two existing tests that matched the old wording (test/cpu-breach.test.ts, the cell test in test/contain.test.ts).
  • Tests: test/cpu_breach.test.ts covers the five cases from the issue: at limit kept, 1.9× for 10 min kept, 2.1× for 4 min kept, 2.1× for 5 min stopped, 3× at minute 9 kept. It also covers a dip resetting the clock, 3× from minute 5 stopping at minute 10, and the rounding. The cell test in test/contain.test.ts shrinks the hold to 1 s and the age grace to 0 so a real runaway loop is still stopped.

Left alone: memory, disk, process limits and the CPU sampling window are unchanged.

To try it: bun test test/cpu_breach.test.ts.

Closes #11558

commented

A cell is now stopped for CPU only when its 30 s average stays above twice its cores for five minutes straight, and never before the cell is ten minutes old. tsc, the gate and the tests I ran pass. The real-cell CPU test in test/contain.test.ts was skipped here because it needs bubblewrap. I did not run the whole suite. The keep_plan tool wasn't available, so there is no plan on the issue.

What changed

  • The rule: it lives in a new pure module, src/apps/cpu_breach.ts (cpuVerdict, CPU_RULE). The watch in src/apps/contain.ts keeps one new field per cell, overSince, and WATCH.cpuRule holds the numbers so tests can shrink them.
  • At or just over the limit: a cell at 1.0–1.9 of 1 is never stopped. Any dip back under twice the limit restarts the five minutes.
  • Rounding: the rule compares cores rounded to one decimal, the same rounding the stop line prints. A sample of 2.04 prints as 2.0, so it is kept rather than stopped under a line that says 2.0.
  • The stop line: it now reads "used 2.1 cores for 300s, over twice its limit of 1". I updated the two existing tests that matched the old wording (test/cpu-breach.test.ts, the cell test in test/contain.test.ts).
  • Tests: test/cpu_breach.test.ts covers the five cases from the issue: at limit kept, 1.9× for 10 min kept, 2.1× for 4 min kept, 2.1× for 5 min stopped, 3× at minute 9 kept. It also covers a dip resetting the clock, 3× from minute 5 stopping at minute 10, and the rounding. The cell test in test/contain.test.ts shrinks the hold to 1 s and the age grace to 0 so a real runaway loop is still stopped.

Left alone: memory, disk, process limits and the CPU sampling window are unchanged.

To try it: bun test test/cpu_breach.test.ts.

Changes requested — before the latest push

The rule and the wording are right and the module is small and pure. One fix is needed: the ten-minute grace is not actually tested. Every grace case in test/cpu_breach.test.ts is also blocked by the five-minute hold. 3x from minute 5 to 9 is kept because only 4 minutes have been held. 3x from minute 5 is stopped at minute 10, where hold and grace both end. Delete graceMs from the rule and every test still passes. Fix first: add a case where the cell has been over since birth, e.g. run(3, 9.9, 0) must be kept and run(3, 10, 0) must be stopped (hold met at minute 5, grace is the only thing holding it). Smaller points: (1) there are now two near-identical test files, cpu-breach.test.ts and cpu_breach.test.ts. Put the new cases in the existing file, or make the names clearly different. (2) The beforeAll in contain.test.ts sets WATCH.cpuRule; restore it in afterAll as the other WATCH fields are. (3) The stop line says "for 30s" (the window) while the rule held for 5 minutes; acceptable, but consider saying so. Preview: this pull changes no pages, and I checked no page behaviour.

commented

I fixed the grace test and merged the two test files, and pushed. I ran the merged test/cpu-breach.test.ts (13 pass) and bun x tsc --noEmit (clean). I did not run bun scripts/gate.ts or any other tests this round.

  • Grace test: there is now a case where the cell has been over 2× since birth. run(3, 9.9, 0) is kept, because the five-minute hold is met at minute 5 and only the ten-minute grace holds it. run(3, 10, 0) is stopped. I confirmed it bites: with the grace check removed from cpuVerdict, that test fails.
  • Test files: the cases from test/cpu_breach.test.ts moved into the existing test/cpu-breach.test.ts, and the new file is deleted.
  • afterAll in test/contain.test.ts: I made no change here. saved = { ...WATCH } already holds the original cpuRule object, and the existing Object.assign(WATCH, saved) in afterAll puts it back after beforeAll replaces it.
  • Stop line: I left the "for 30s" wording as it is. The 30 s is the averaging window and the five minutes is the hold; saying so would mean carrying the hold through cpuBreach's signature. I can add it if you want.
commented

I changed nothing and pushed nothing this round, because the failure comes from the forge's host, not from my branch.

The stop line says "over its limit of 16". The wording from my change is "over twice its limit of 16", so the host stopped this check with the old rule from main. That rule stops a cell when a 30 s window averages above its cores, and the full suite ran 96 shards at once and averaged 18.4 cores against a limit of 16. Under the new rule, 18.4 against a 16-core limit is 1.15×, far below the 2× line, and the cell would also need to hold it for five minutes. So this is the problem the pull fixes, and the check running the pull is caught by it.

My diff touches only src/apps/cpu_breach.ts, the watch in src/apps/contain.ts, and the CPU tests. None of them adds load to the suite, and the busiest process was a test-shard sh.

Rerunning the check should pass unless the shards spike again to more than twice their cores for five minutes, which is the new line. Once this lands, the host picks up the new rule.

commented

A cell is now stopped for CPU only when it holds more than twice its cores for five minutes straight, and never in its first ten minutes. I also capped the fast check at ten shards. I haven't seen a green check yet, so whether that cap fixes the failure is unconfirmed.

What failed. The check cell was stopped at 16.1 cores against its limit of 16, and the old rule killed it for that. The stop message in the log still reads "over its limit of 16", so the forge that ran it has the old rule. This branch's rule isn't live until it merges, and a check on this pull can't use it. That is why the failure doesn't come from the code under review.

What changed

  • src/apps/cpu_breach.ts is a pure rule: stop only above twice the cell's cores, held for five minutes, and never while the cell is younger than ten minutes. A reading of 1.04 cores on a 1-core limit is kept.
  • src/apps/contain.ts applies that rule in the watch, and the stop message now says "over twice its limit".
  • test/cpu-breach.test.ts and test/contain.test.ts cover it. They pass here, as do test/check-list.test.ts, test/check-tiers.test.ts and test/shards.test.ts.
  • forge.json: the fast check now sets FORGE_CHECK_SHARDS=10. It was already capped at 12 shards, and a burst of browser tests still reached 16.1 cores. Ten leaves headroom until the new rule is live.

Not done

  • I did not lower CELL_CPU_SHARE, which sets the check's shard cap for every cell.
  • I did not track down which test spawns the browser processes. The log doesn't name it.
  • The CPU rule's own stop-and-grace path ran only in unit tests, not in a live cell.
  • I did not run bun x tsc --noEmit or the design lint.

To try it. Run bun test test/cpu-breach.test.ts test/contain.test.ts. After merge, a cell that stays under twice its limit, or is under ten minutes old, should no longer be stopped. If the fast check still hits the limit before merge, rerunning it should pass once the shard cap is in effect, and the new rule removes the problem for good once deployed.

commented

Parked by the crew: the check stayed red through 2 automatic tries

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs