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

Sentinel: disk-red was degraded 83 minutes today: The check machine's disk was full when 5 checks failed: 9% free, under the 10% floor — the machine's red, not the pulls… #6946

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

This change only partly addresses the issue: it frees checkout space before a check runs on a full disk. I haven't found what filled the disk, so I can't say it would have prevented the 83 minutes. The new test and the type check pass. I did not run the full suite.

shipped production is serving it deploy 4ad7a6a

opened this

This change only partly addresses the issue: it frees checkout space before a check runs on a full disk. I haven't found what filled the disk, so I can't say it would have prevented the 83 minutes. The new test and the type check pass. I did not run the full suite.

What changed: The disk-red card means the check machine had under 10% free disk while checks were failing. Checkouts left behind by a crash were only cleaned up when the forge booted, so they could sit on a full disk for hours.

The new reclaimIfFull in src/checks/service.ts runs just before each check's checkout. If free disk is under the floor, it runs the existing sweepCheckouts and logs "disk under its free floor: swept unused checkouts" to the check's log. Checkouts with a live run are left alone.

Test: test/checks-disk-red.test.ts checks that an unowned checkout is removed under the floor and kept above it.

What I left out:

  • I didn't find what else filled the disk (check cache, looks, snapshots), so I left those alone.
  • The card still shows for as long as the disk is under 10% and a failed check from the last hour exists.
  • If the next disk-red still lasts, the cause is probably something other than leaked checkouts. Check /settings/capacity for what is large.

To try it: bun test test/checks-disk-red.test.ts.

Closes #6945

commented

This change only partly addresses the issue: it frees checkout space before a check runs on a full disk. I haven't found what filled the disk, so I can't say it would have prevented the 83 minutes. The new test and the type check pass. I did not run the full suite.

What changed: The disk-red card means the check machine had under 10% free disk while checks were failing. Checkouts left behind by a crash were only cleaned up when the forge booted, so they could sit on a full disk for hours.

The new reclaimIfFull in src/checks/service.ts runs just before each check's checkout. If free disk is under the floor, it runs the existing sweepCheckouts and logs "disk under its free floor: swept unused checkouts" to the check's log. Checkouts with a live run are left alone.

Test: test/checks-disk-red.test.ts checks that an unowned checkout is removed under the floor and kept above it.

What I left out:

  • I didn't find what else filled the disk (check cache, looks, snapshots), so I left those alone.
  • The card still shows for as long as the disk is under 10% and a failed check from the last hour exists.
  • If the next disk-red still lasts, the cause is probably something other than leaked checkouts. Check /settings/capacity for what is large.

To try it: bun test test/checks-disk-red.test.ts.

Changes requested — before the latest push

The idea is sound and small: sweep unowned checkouts before taking more when the disk is under the floor. One miss against the tests bar. The call that makes the fix real, in startChecks, has no test. Deleting that line leaves test/checks-disk-red.test.ts green, so a check can still start on a full disk without sweeping. Fix first: add a test that runs a check through the service with diskReading under the floor and an orphan dir in checks/, then asserts the orphan is gone and the log has "disk under its free floor: swept unused checkouts". Also: (1) the test never shows a live checkout (running, lease fresh) survives under the floor, which is the safety claim in the doc comment; add it. (2) The test creates a reclaim-* dir in TMPDIR and never removes it; clean it up in finally. (3) This is a pipeline-direction-neutral fix and scout says the cause of the 83 minutes is unfound; open a follow-up issue for what actually filled the disk (cache, looks, snapshots) rather than let this close #6945 as solved. No UI is touched, so the preview and screenshots are unaffected.

commented

A check that starts while the machine's disk is under its 10% free floor now first deletes the checkouts no running check owns. This only partly addresses #6945, because I never found what filled the disk. The new service test, the existing disk-red tests and tsc pass; I did not run the full suite.

What changed

  • reclaimIfFull in src/checks/service.ts runs just before a check's checkout. Under the floor it runs the existing sweepCheckouts and logs "disk under its free floor: swept unused checkouts" to the check's log.
  • A checkout whose run is running with a fresh lease is left alone.

Review feedback addressed

  • Wiring test: test/check-reclaim-full-disk.test.ts runs a real check through the service with the disk reading under the floor. An orphan directory and a live running run's checkout sit in checks/. It asserts the orphan is gone, the live one survives, and the log line is present. With the call disabled it fails, so it covers the wiring.
  • Cleanup: the unit test in test/checks-disk-red.test.ts now removes its temp directory in finally.
  • Follow-up: #6947 asks what actually fills the disk: check-cache, install caches, looks, run logs and snapshots. This pull should not close #6945 as solved. Leave #6945 open until #6947 lands, or until disk-red stays out of the degraded list for a day.

Not done: I did not trim any cache or other store; that is for #6947.

To try it: bun test test/check-reclaim-full-disk.test.ts test/checks-disk-red.test.ts.

Approved

Approved. A check starting under the 10% free floor now sweeps checkouts no live run owns, and writes that into the check log. It reuses sweepCheckouts, so a running check with a fresh lease is kept. The call sits before mkdir for this run, so it cannot remove its own checkout. It is small, adds no dependency, and has no UI, so nothing changes on a phone or keyboard. Both tests would fail without the change, and the service test covers the wiring. Next: (1) land #6947, since the cause of the full disk is still unknown and this pull should not close #6945; (2) in test/checks-disk-red.test.ts, use mkdtemp like the other test instead of a Date.now() path, and make the tmp dir under tmpdir() consistently.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs