Public: anyone can read this; the crew can change it.
A booting forge removes only its own dead checkouts, not the checks another forge is running #1887
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/1884-a-booting-forge-removes-only-its-own-dea → main 1 commit fork
2 files
changed +64
−4
src/checks/service.ts +23 −4
| @@ -12,7 +12,7 @@ | ||
| 12 | 12 | */ |
| 13 | 13 | |
| 14 | 14 | import { pullDir, runDir } from "../core/pulls.ts"; |
| 15 | import { existsSync, mkdirSync, rmSync } from "node:fs"; | |
| 15 | import { existsSync, mkdirSync, readdirSync, rmSync } from "node:fs"; | |
| 16 | 16 | import { tmpdir } from "node:os"; |
| 17 | 17 | import { join } from "node:path"; |
| 18 | 18 | import type { App } from "../app.ts"; |
| @@ -24,6 +24,7 @@ import { getRepoById, type Repo } from "../core/repos.ts"; | ||
| 24 | 24 | import { |
| 25 | 25 | cancelRun, |
| 26 | 26 | getRun, |
| 27 | leaseLapsed, | |
| 27 | 28 | cancelRunsForPull, |
| 28 | 29 | checksForHead, |
| 29 | 30 | claimRun, |
| @@ -51,7 +52,7 @@ import { repoDir } from "../git/repos.ts"; | ||
| 51 | 52 | import { BUDGETS, budgetSummary } from "../kernel/budgets.ts"; |
| 52 | 53 | import { transact } from "../kernel/db.ts"; |
| 53 | 54 | import { createLog } from "../kernel/log.ts"; |
| 54 | import { holdLease } from "../runs/lease.ts"; | |
| 55 | import { holdLease, STALE_AFTER_MS } from "../runs/lease.ts"; | |
| 55 | 56 | import { appendRunLog } from "../runs/log.ts"; |
| 56 | 57 | import { exportTree, readFileAt } from "../apps/checkout.ts"; |
| 57 | 58 | import { parseCheck } from "../apps/manifest.ts"; |
| @@ -185,6 +186,25 @@ export function requeueAbandonedChecks(app: App, staleMs: number): number { | ||
| 185 | 186 | /** Where a check's checkout lives while it runs. */ |
| 186 | 187 | const checksRoot = (dataDir: string): string => join(dataDir, "checks"); |
| 187 | 188 | |
| 189 | /** | |
| 190 | * Remove the checkouts nobody is using: one whose run is not running with a | |
| 191 | * live lease, or has no run at all. A forge booting during a roll shares this | |
| 192 | * directory and database with the old one, whose checks are still running in | |
| 193 | * theirs; those are left alone. | |
| 194 | */ | |
| 195 | export function sweepCheckouts(app: App, now: number = Date.now()): void { | |
| 196 | const root = checksRoot(app.config.dataDir); | |
| 197 | if (!existsSync(root)) return; | |
| 198 | for (const name of readdirSync(root)) { | |
| 199 | const run = getRun(app.db, name); | |
| 200 | const live = | |
| 201 | run !== undefined && | |
| 202 | run.state === "running" && | |
| 203 | !leaseLapsed(run, STALE_AFTER_MS, now); | |
| 204 | if (!live) rmSync(join(root, name), { recursive: true, force: true }); | |
| 205 | } | |
| 206 | } | |
| 207 | ||
| 188 | 208 | const handles = new WeakMap<App, ChecksHandle>(); |
| 189 | 209 | |
| 190 | 210 | /** The checks service of this forge, if one was started. */ |
| @@ -206,8 +226,7 @@ export function startChecks(app: App, options: ChecksOptions): ChecksHandle { | ||
| 206 | 226 | >(); |
| 207 | 227 | let stopped = false; |
| 208 | 228 | |
| 209 | // A checkout left by a forge that died mid-check is nobody's. | |
| 210 | rmSync(checksRoot(app.config.dataDir), { recursive: true, force: true }); | |
| 229 | sweepCheckouts(app); | |
| 211 | 230 | |
| 212 | 231 | async function request( |
| 213 | 232 | repo: Repo, |
test/checks.test.ts +41
| @@ -5,6 +5,7 @@ | ||
| 5 | 5 | */ |
| 6 | 6 | |
| 7 | 7 | import { afterAll, beforeAll, describe, expect, test } from "bun:test"; |
| 8 | import { existsSync, mkdirSync } from "node:fs"; | |
| 8 | 9 | import { mkdtemp, rm } from "node:fs/promises"; |
| 9 | 10 | import { tmpdir } from "node:os"; |
| 10 | 11 | import { join } from "node:path"; |
| @@ -1041,3 +1042,43 @@ describe("a check its forge died under", () => { | ||
| 1041 | 1042 | expect(queuedFor(stale)).toHaveLength(0); |
| 1042 | 1043 | }); |
| 1043 | 1044 | }); |
| 1045 | ||
| 1046 | describe("startChecks: a booting forge removes only checkouts nobody is running (#1884)", () => { | |
| 1047 | test("a live run's checkout survives, a lapsed run's and a stranger's go", async () => { | |
| 1048 | const dir = await mkdtemp(join(tmpdir(), "forge-sweep-")); | |
| 1049 | const db = createApp({ dataDir: dir, port: 0 }); | |
| 1050 | try { | |
| 1051 | const u = await createUser(db.db, { | |
| 1052 | trusted: true, | |
| 1053 | username: "alice", | |
| 1054 | email: "[email protected]", | |
| 1055 | password: "a-good-passphrase", | |
| 1056 | adminIfFirst: true, | |
| 1057 | }); | |
| 1058 | if (!u.ok) throw new Error(u.error); | |
| 1059 | const made = await createRepo(db, u.value, { name: "lib" }); | |
| 1060 | if (!made.ok) throw new Error(String(made.error)); | |
| 1061 | const mk = (): Run => { | |
| 1062 | const run = createRun(db.db, { repoId: made.value.id, kind: "check", env: "x" }); | |
| 1063 | claimRun(db.db, run.id); | |
| 1064 | return run; | |
| 1065 | }; | |
| 1066 | const live = mk(); | |
| 1067 | const lapsed = mk(); | |
| 1068 | db.db | |
| 1069 | .query("UPDATE run SET heartbeat_at = ?2 WHERE id = ?1") | |
| 1070 | .run(lapsed.id, new Date(Date.now() - STALE_AFTER_MS - 5_000).toISOString()); | |
| 1071 | const where = (id: string): string => join(dir, "checks", id); | |
| 1072 | for (const id of [live.id, lapsed.id, "no-such-run"]) | |
| 1073 | mkdirSync(where(id), { recursive: true }); | |
| 1074 | const handle = startChecks(db, { forgeUrl: "http://localhost" }); | |
| 1075 | await handle.stop(); | |
| 1076 | expect(existsSync(where(live.id))).toBe(true); | |
| 1077 | expect(existsSync(where(lapsed.id))).toBe(false); | |
| 1078 | expect(existsSync(where("no-such-run"))).toBe(false); | |
| 1079 | } finally { | |
| 1080 | db.close(); | |
| 1081 | await rm(dir, { recursive: true, force: true }); | |
| 1082 | } | |
| 1083 | }); | |
| 1084 | }); |
new above
offline