trevato/forge

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

More
Forks1g fMapg aRuns14g rEventsFlows

A check's test shard dies of 'database or disk is full' with 384 GB free, and a good pull goes red #8680

Merged torquecrew opened this in answer to trevatoowner

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

ConversationFiles
Merged

torque/forge:torque/8679-a-check-s-test-shard-dies-of-database-or → main 4 commits fork

  • Merged. torque/8679-a-check-s-test-shard-dies-of-database-or was merged into main as 0ea41d8.

Review

Sign in to review this change.
9 files changed +279 −6
scripts/test-shards.ts +12 −3
@@ -85,6 +85,7 @@ import { gateFiles, gateRefusal, presentTests } from "./gate.ts";
8585 import { cgroupMemory, summaryLines } from "./gate-summary.ts";
8686 import { GATE_FILE, executedFiles, formatImpact, gateListAdds, handedImpact, mergeImpact, parseImpact } from "../src/checks/impact.ts";
8787 import { importsReader, readImpact } from "./tier-input.ts";
88 import { shardHome, spaceLine } from "../src/checks/disk_full.ts";
8889 import { fastSet, duration } from "../src/checks/tiers.ts";
8990 import { CHECK_CACHE } from "../src/checks/cache.ts";
9091 import { parseCheck } from "../src/apps/manifest.ts";
@@ -481,10 +482,11 @@ type Timed = ShardResult & { seconds: number };
481482 */
482483 async function runShard(slice: string[], index: number, live: boolean): Promise<Timed> {
483484 // Its own temp and data directories: shards side by side never share a
484 // path. They sit under the TMPDIR the cell set (tmpdir() reads it), and a
485 // FORGE_APP_DATA the cell set is nested under, never replaced. Removed
485 // path. They sit on the cell's disk (FORGE_CHECK_SCRATCH, else a
486 // FORGE_APP_DATA, which is nested under, never replaced), not in its
487 // 256 MiB /tmp; the system temp directory when neither is set. Removed
486488 // however the shard ends.
487 const home = mkdtempSync(join(tmpdir(), "shard-"));
489 const home = shardHome(process.env, tmpdir());
488490 try {
489491 return await runShardIn(slice, index, live, home);
490492 } finally {
@@ -625,6 +627,13 @@ async function runShardIn(
625627 if (verdict.red.length === 0) code = 0;
626628 }
627629 }
630 // Where the room went, named on the log of a red shard, before its
631 // directories are removed: the next red says its own cause (#8679).
632 if (code !== 0) {
633 const note = `${spaceLine({ scratch: home, tmp: tmpdir() })}\n`;
634 output.push(note);
635 through?.(note);
636 }
628637 if (process.env["FORGE_APP_DATA"])
629638 rmSync(childEnv["FORGE_APP_DATA"], { recursive: true, force: true });
630639 const text = output.join("");
src/checks/disk_full.ts added +93
@@ -0,0 +1,93 @@
1 /**
2 * A shard that ran out of room, told from a shard whose tests failed (#8679).
3 *
4 * The limit: a check's cell mounts its own `/tmp` as a 256 MiB tmpfs
5 * (`TMP_MIB`, `apps/contain.ts`), whatever the machine's disk has free. Every
6 * shard of the gate made its `TMPDIR` there, and the tests fill it with
7 * sqlite databases, their journals and WAL files, cloned repositories and
8 * sockets, so a few shards side by side reach 256 MiB while the disk shows
9 * hundreds of GB free. SQLite answers `database or disk is full`
10 * (SQLITE_FULL), the file's later `beforeAll`s fall over behind it as
11 * `(unnamed)`, and the pull goes red for the machine. Two answers:
12 * shards keep their temp on the cell's disk (`shardBase`, `shardHome`), and
13 * when one still runs out the run's detail says so (`fullShard`), which the
14 * crew reads as the environment's fault. `spaceLine` makes the next red name
15 * its cause either way.
16 */
17
18 import { mkdirSync, mkdtempSync, statfsSync } from "node:fs";
19 import { join } from "node:path";
20
21 /**
22 * What a full tmpfs or disk prints, as an error: SQLite's SQLITE_FULL or the
23 * kernel's ENOSPC, on a line that reports an error. A test that merely
24 * mentions the words (as this pull's own does) is not one.
25 */
26 const FULL_ERROR =
27 /^\s*(?:error: |\w*Error: ).*(?:database or disk is full|SQLITE_FULL|ENOSPC|No space left on device)/;
28 const FAIL = /^\(fail\) (.+?)(?: \[[0-9.]+m?s\])?(?: \[shard [0-9]+\])?$/;
29 const UNNAMED = /(^|> )\(unnamed\)(:|$)/;
30
31 /** The marker a run's detail carries when its temp space ran out. */
32 export const TEMP_FULL = "temp_full" as const;
33
34 /** The words a summary starts with when the environment, not the change, failed. */
35 export const OUT_OF_ROOM = "the check's temp space ran out";
36
37 /**
38 * Where a shard's temp and data directories go: under `FORGE_CHECK_SCRATCH`,
39 * the home the check service binds into the cell (a directory on the disk,
40 * not the tmpfs), else `FORGE_APP_DATA` when one is set, else under the
41 * system temp directory. The cell itself sets no `FORGE_APP_DATA`, so the
42 * scratch variable is the one that matters there.
43 */
44 export function shardBase(env: Readonly<Record<string, string | undefined>>, tmp: string): string {
45 for (const key of ["FORGE_CHECK_SCRATCH", "FORGE_APP_DATA"]) {
46 const dir = env[key];
47 if (dir !== undefined && dir !== "") return dir;
48 }
49 return tmp;
50 }
51
52 /** A shard's own fresh directory under {@link shardBase}, made if the base is not there yet. */
53 export function shardHome(env: Readonly<Record<string, string | undefined>>, tmp: string): string {
54 const base = shardBase(env, tmp);
55 mkdirSync(base, { recursive: true });
56 return mkdtempSync(join(base, "shard-"));
57 }
58
59 /** What the check service adds to a cell's environment: its bound home, when it runs in a cell. */
60 export const scratchEnv = (home: string | undefined): Record<string, string> =>
61 home === undefined ? {} : { FORGE_CHECK_SCRATCH: home };
62
63 /** One line of free space for each place a shard writes, for the log of a red shard. */
64 export function spaceLine(places: Readonly<Record<string, string>>): string {
65 const mib = (n: number): string => `${Math.round(n / 1048576)} MiB`;
66 const parts = Object.entries(places).map(([label, dir]) => {
67 try {
68 const fs = statfsSync(dir);
69 const size = Number(fs.bsize);
70 return `${label} ${dir}: ${mib(Number(fs.bavail) * size)} free of ${mib(Number(fs.blocks) * size)}`;
71 } catch {
72 return `${label} ${dir}: unreadable`;
73 }
74 });
75 return `test-shards: space after a red shard: ${parts.join("; ")}`;
76 }
77
78 /**
79 * Whether a log shows a shard that ran out of room: an error line (not any
80 * line) carrying SQLITE_FULL or ENOSPC, and no more than one distinct named
81 * test failing beside the `(unnamed)` hooks that fall over behind it. Two
82 * named failures are the change's tests failing; the real list stays.
83 */
84 export function fullShard(lines: readonly string[]): boolean {
85 const at = lines.findIndex((line) => FULL_ERROR.test(line));
86 if (at === -1) return false;
87 const named = new Set<string>();
88 for (const line of lines) {
89 const fail = FAIL.exec(line);
90 if (fail && !UNNAMED.test(fail[1]!)) named.add(fail[1]!.replace(/^test\/\S+ > /, ""));
91 }
92 return named.size <= 1;
93 }
src/checks/service.ts +4
@@ -59,6 +59,7 @@ import { MAX_FILED, clearedFiles, fileOrderDependent } from "./order_dependent.t
5959 import { parseBunTestFailure } from "./bun-test.ts";
6060 import { recordCrashFlakes, passedAlone, recordFlakes, rerunCommand, rerunnable, pullOwnsRed } from "./flake.ts";
6161 import { diskRecovered, withDisk } from "./disk_red.ts";
62 import { scratchEnv } from "./disk_full.ts";
6263 import { ASKED, redMain, unheld } from "./red_main.ts";
6364 import { proposeRevert } from "./revert_red.ts";
6465 import { rebasesFirst } from "./finishing_first.ts";
@@ -1160,6 +1161,9 @@ export function startChecks(app: App, options: ChecksOptions): ChecksHandle {
11601161 // The cell's CPU limit, which `availableParallelism()` may not
11611162 // report inside it; the shard count follows this (#2257).
11621163 FORGE_CELL_CPUS: String(cellCpus),
1164 // The one place besides the checkout the cell can write on disk; its
1165 // own /tmp is a 256 MiB tmpfs the shards would fill (#8679).
1166 ...scratchEnv(cell === undefined ? undefined : home),
11631167 ...(base === undefined
11641168 ? {}
11651169 : {
src/checks/summary.ts +12 −3
@@ -9,6 +9,7 @@
99
1010 import type { CheckDetail, Run } from "../core/runs.ts";
1111 import { parseBunTestFailure } from "./bun-test.ts";
12 import { fullShard, OUT_OF_ROOM, TEMP_FULL } from "./disk_full.ts";
1213 import { oneLine } from "./words.ts";
1314 import { clip } from "../text/clip.ts";
1415 import { webvitals, type Entries } from "../release/webvitals.ts";
@@ -50,8 +51,15 @@ export function summariseCheckFailure(
5051 lines: readonly string[],
5152 fallback: string,
5253 ): CheckFailure {
53 const detail = parseBunTestFailure(lines);
54 if (detail !== undefined) {
54 const parsed = parseBunTestFailure(lines);
55 // A shard that ran out of temp space is marked on the detail, not worded
56 // into the summary: the crew reads the mark, the summary keeps the failures.
57 const full = fullShard(lines);
58 const detail = full
59 ? { ...(parsed ?? { passed: 0, failed: 0, names: [] }), environment: TEMP_FULL }
60 : parsed;
61 const note = full ? `${OUT_OF_ROOM} (the environment's fault): ` : "";
62 if (parsed !== undefined && detail !== undefined) {
5563 const total = detail.passed + detail.failed;
5664 const names = [...new Set(detail.names)].slice(0, 3);
5765 const quoted = names.map((n) => `"${n}"`);
@@ -65,11 +73,12 @@ export function summariseCheckFailure(
6573 ? `${quoted.slice(0, -1).join(", ")} and ${quoted.at(-1)}`
6674 : quoted[0]!;
6775 return {
68 summary: truncate(`${detail.failed} of ${total} tests — ${list}`),
76 summary: truncate(`${note}${detail.failed} of ${total} tests — ${list}`),
6977 detail,
7078 };
7179 }
7280 const unnamed = lines.find((line) => UNNAMED_SHARD.test(line.trim()));
81 if (full) return { summary: truncate(`${note}${unnamed?.trim() ?? fallback}`), detail };
7382 if (unnamed !== undefined)
7483 return { summary: truncate(unnamed.trim()), detail: undefined };
7584 const block = lines.findLastIndex((line) => line.trim() === GATE_SUMMARY_HEADING);
src/core/runs.ts +2
@@ -49,6 +49,8 @@ export interface CheckDetail {
4949 readonly failures?: readonly { readonly file: string; readonly name: string }[];
5050 /** The check machine's free disk share (0..1) when a red finished (#5453). */
5151 readonly diskFree?: number;
52 /** `temp_full`: a shard ran out of temp space, the environment's fault, not the change's (#8679). */
53 readonly environment?: "temp_full";
5254 }
5355
5456 export interface Run {
src/crew/dispatch.ts +5
@@ -8,6 +8,7 @@
88 * time. A missed wake-up costs a tick, never a run.
99 */
1010
11 import { TEMP_FULL } from "../checks/disk_full.ts";
1112 import { freshPullFor } from "./fresh_pull.ts";
1213 import { twinNote, twinOf, type Twin } from "./twin.ts";
1314 import { NO_MERGE } from "../checks/merged.ts";
@@ -1149,6 +1150,10 @@ export async function judgeFailedCheck(
11491150 ): Promise<CheckJudgement> {
11501151 const run = getRun(app.db, runId);
11511152 const failingTest = run?.checkDetail?.names[0];
1153 // A shard that ran out of temp space is the machine's, whatever a decider
1154 // might guess: the check runs again without spending the agent's tries.
1155 if (run?.checkDetail?.environment === TEMP_FULL)
1156 return { retry: "again", cause: "environment", failingTest };
11521157 if (app.config.decider === undefined)
11531158 return { retry: "again", cause: "change", failingTest };
11541159 // A pull that does not merge is the change's, whatever a decider might
test/disk-full-shard.test.ts added +61
@@ -0,0 +1,61 @@
1 import { expect, test } from "bun:test";
2 import { existsSync, mkdtempSync, rmSync } from "node:fs";
3 import { tmpdir } from "node:os";
4 import { join } from "node:path";
5 import { fullShard, scratchEnv, shardBase, shardHome, spaceLine, TEMP_FULL } from "../src/checks/disk_full.ts";
6 import { summariseCheckFailure } from "../src/checks/summary.ts";
7
8 const log = [
9 "test/migrate-token-reach.test.ts:",
10 "SQLiteError: database or disk is full",
11 "(fail) migrate > pins [3ms]",
12 "error: undefined is not an object",
13 "(fail) test/migrate-token-reach.test.ts > (unnamed)",
14 " 10 pass",
15 " 2 fail",
16 ];
17
18 test("a full shard is marked on the detail, and the failures stay in the summary", () => {
19 const said = summariseCheckFailure(log, "exit 1");
20 expect(said.detail?.environment).toBe(TEMP_FULL);
21 expect(said.summary).toContain('"migrate > pins"');
22 expect(summariseCheckFailure(["(fail) a", " 1 pass", " 1 fail"], "exit 1").detail?.environment).toBeUndefined();
23 });
24
25 test("the words alone are not the verdict: a test that prints them, or two named failures, stay the change's", () => {
26 const prints = [" expect(text).toContain(\"No space left on device\")", "(fail) a [1ms]", " 1 fail"];
27 expect(fullShard(prints)).toBe(false);
28 expect(fullShard(["(fail) a: expected ENOSPC", " 1 fail"])).toBe(false);
29 const two = ["SQLiteError: database or disk is full", "(fail) one [1ms]", "(fail) two [1ms]", " 5 pass", " 2 fail"];
30 expect(fullShard(two)).toBe(false);
31 const said = summariseCheckFailure(two, "exit 1");
32 expect(said.detail?.environment).toBeUndefined();
33 expect(said.summary).toContain('"one" and "two"');
34 expect(fullShard(["error: database or disk is full", "(fail) one [1ms]", "(fail) test/x.test.ts > (unnamed) [1ms]"])).toBe(true);
35 });
36
37 test("a shard keeps its temp on the cell's disk when the cell set one", () => {
38 expect(shardBase({ FORGE_APP_DATA: "/cell/data" }, "/tmp")).toBe("/cell/data");
39 expect(shardBase({}, "/tmp")).toBe("/tmp");
40 expect(shardBase({ FORGE_CHECK_SCRATCH: "/home/run", FORGE_APP_DATA: "/cell/data" }, "/tmp")).toBe("/home/run");
41 });
42
43 test("a shard's home is made under the scratch, which need not exist yet", () => {
44 const root = mkdtempSync(join(tmpdir(), "scratch-"));
45 const scratch = join(root, "not", "yet");
46 const home = shardHome({ FORGE_CHECK_SCRATCH: scratch }, "/nowhere");
47 expect(home.startsWith(`${scratch}/shard-`)).toBe(true);
48 expect(existsSync(home)).toBe(true);
49 rmSync(root, { recursive: true, force: true });
50 });
51
52 test("the check service hands a cell's home to the shards, and nothing outside a cell", () => {
53 expect(scratchEnv("/runs/1.home")).toEqual({ FORGE_CHECK_SCRATCH: "/runs/1.home" });
54 expect(scratchEnv(undefined)).toEqual({});
55 });
56
57 test("the space line names each place's free room", () => {
58 const line = spaceLine({ scratch: tmpdir(), gone: "/no/such/dir" });
59 expect(line).toMatch(/scratch .*: \d+ MiB free of \d+ MiB/);
60 expect(line).toContain("gone /no/such/dir: unreadable");
61 });
test/ledger.test.ts +61
@@ -33,6 +33,7 @@ import { createUser, getUserByName, type User } from "../src/core/users.ts";
3333 import { setCrewRole } from "../src/core/agents.ts";
3434 import type { Answer, Decider, Questions } from "../src/crew/decide.ts";
3535 import { driveRepo, landIfGreen } from "../src/crew/autonomy.ts";
36 import { summariseCheckFailure } from "../src/checks/summary.ts";
3637 import { answerFailedCheck, judgeFailedCheck } from "../src/crew/dispatch.ts";
3738 import { serve, type Servers } from "../src/http/server.ts";
3839
@@ -241,7 +242,26 @@ describe("the decision ledger", () => {
241242 state: "failed",
242243 summary: "test failed: 1 of 9",
243244 });
245 // A shard that ran out of temp space is the environment's, even when the decider would stop.
246 const full = createRun(app.db, {
247 repoId: repo.id,
248 kind: "check",
249 issueId: pull.id,
250 env: `pull-${pull.number}`,
251 commitOid: pull.headOid,
252 reason: "pull",
253 });
254 claimRun(app.db, full.id);
255 // The real parse of a log, not a summary written by hand.
256 const said = summariseCheckFailure(
257 ["SQLiteError: database or disk is full", "(fail) migrate > pins [3ms]", "(fail) test/a.test.ts > (unnamed) [1ms]", " 9 pass", " 2 fail"],
258 "exit 1",
259 );
260 finishRun(app.db, full.id, { state: "failed", summary: said.summary, checkDetail: said.detail });
244261 script["retry"] = "stop";
262 const judged = await judgeFailedCheck(app, fresh(), pull, full.id, pull.createdAt);
263 expect(judged.cause).toBe("environment");
264 expect(judged.retry).toBe("again");
245265 expect(
246266 (await judgeFailedCheck(app, fresh(), pull, failed.id, pull.createdAt))
247267 .retry,
@@ -357,6 +377,47 @@ describe("the decision ledger", () => {
357377 }),
358378 ).toBe("reworked");
359379 expect(getPull(app.db, repo.id, pull.number)?.state).toBe("open");
380
381 // A full temp space on a commit is run again once; the same red on the
382 // same commit is not run again, however often it comes (#8679).
383 const second = await commitToBranch(app, alice, repo, {
384 branch: "feature/full",
385 base: "main",
386 message: "Full",
387 files: [
388 { path: "forge.json", content: `{"check": {"run": "test -f OK.md", "install": ""}}\n` },
389 { path: "OK.md", content: "ok\n" },
390 ],
391 });
392 if (!second.ok) throw new Error(JSON.stringify(second.error));
393 const reopened = await openPull(app, scout, repo, {
394 title: "Full",
395 baseBranch: "main",
396 headBranch: "feature/full",
397 });
398 if (!reopened.ok) throw new Error(String(reopened.error));
399 const filled = reopened.value;
400 script["retry"] = "again";
401 delete script["cause"];
402 const fullRed = (): string => {
403 const run = createRun(app.db, {
404 repoId: repo.id,
405 kind: "check",
406 issueId: filled.id,
407 env: `pull-${filled.number}`,
408 commitOid: filled.headOid,
409 reason: "pull",
410 });
411 claimRun(app.db, run.id);
412 const said = summariseCheckFailure(["Error: database or disk is full", "(fail) a [1ms]", " 1 fail"], "exit 1");
413 finishRun(app.db, run.id, { state: "failed", summary: said.summary, checkDetail: said.detail });
414 return run.id;
415 };
416 const again = { kind: "check", commitOid: filled.headOid, env: `pull-${filled.number}`, since: filled.createdAt };
417 const answers: string[] = [];
418 for (let i = 0; i < 3; i++)
419 answers.push(await answerFailedCheck(app, fresh(), filled, scoutAgent, { runId: fullRed(), ...again }));
420 expect(answers).toEqual(["reran", "reworked", "reworked"]);
360421 } finally {
361422 delete script["cause"];
362423 }
test/shards-run.test.ts +29
@@ -11,6 +11,35 @@ import { join } from "node:path";
1111 import { CRASH_RERUNS } from "../scripts/shards.ts";
1212 import { EXCLUDED, gateFiles, presentTests } from "../scripts/gate.ts";
1313
14 describe("where a shard writes", () => {
15 test("its TMPDIR sits under FORGE_CHECK_SCRATCH, and a red shard logs the free space (#8679)", async () => {
16 const dir = mkdtempSync(join(tmpdir(), "shards-scratch-"));
17 const scratch = join(dir, "scratch");
18 writeFileSync(
19 join(dir, "where.test.ts"),
20 `import { expect, test } from "bun:test";
21 test("temp is on the scratch", () => {
22 console.log("TMP=" + process.env.TMPDIR);
23 expect(false).toBe(true);
24 });
25 `,
26 );
27 const script = new URL("../scripts/test-shards.ts", import.meta.url).pathname;
28 const child = Bun.spawn(["bun", script, "where.test.ts"], {
29 cwd: dir,
30 env: { ...process.env, FORGE_CHECK_SHARDS: "1", FORGE_CHECK_SCRATCH: scratch },
31 stdin: "ignore",
32 stdout: "pipe",
33 stderr: "pipe",
34 });
35 const [out] = await Promise.all([new Response(child.stdout).text(), new Response(child.stderr).text()]);
36 await child.exited;
37 rmSync(dir, { recursive: true, force: true });
38 expect(out).toContain(`TMP=${scratch}/shard-`);
39 expect(out).toMatch(/test-shards: space after a red shard: scratch .*MiB free of/);
40 }, 30_000);
41 });
42
1443 describe("the shard's output", () => {
1544 // A check's log is this script's stdout, line by line, as the forge
1645 // reads it (`checks/service.ts`). Holding a shard's output until it

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs