diff --git a/src/lib/local/indexer.ts b/src/lib/local/indexer.ts index 64d38e3..03ec5f4 100644 --- a/src/lib/local/indexer.ts +++ b/src/lib/local/indexer.ts @@ -279,9 +279,12 @@ function rowToRoot(row: RootRow): IndexRoot { */ export function rootHealth(root: IndexRoot, staleMinutes?: number): RootHealth { if (root.status === "error") return "error"; - if (root.status === "pending") return "pending"; - if (root.status === "indexing") { + // A committed start marker can outlive its process even though the mutable + // status now changes only inside the file-update transaction. Inspect the + // marker for pending/ready rows too: after a crash, SQLite rolls status back + // to its prior value while this marker remains as recovery evidence. + if (root.status === "indexing" || root.status === "pending" || root.indexingStartedAt) { // Direct evidence about the process outranks every inference drawn from a // column, and it is consulted FIRST. Three cases depend on this ordering: // @@ -294,19 +297,18 @@ export function rootHealth(root: IndexRoot, staleMinutes?: number): RootHealth { // - Recovery must not steal a root from a live indexer. Since the removal // of the unconditional `status === 'indexing'` skip in // refreshStaleRoots, this is the only thing preventing that: any pass - // outlasting INDEX_LOCK_STALE_MS has a "stale" lock, because the lock - // mtime is never refreshed during a run, so acquireRootLock would - // happily unlink it and run a second indexer concurrently. + // outlasting INDEX_LOCK_STALE_MS would otherwise have its lock stolen; + // heartbeatRootLock keeps that evidence fresh throughout the pass. const lock = probeLockHolder(root.id); if (lock === "alive") return "indexing"; // A dead holder is proof the run ended, so a run killed seconds ago is // caught immediately rather than masquerading as live until the threshold. if (lock === "dead") return "wedged"; - // No lock to consult. Fall back to the start marker: absent means the row - // predates the marker column, i.e. written by a build that could not record - // liveness at all. - if (!root.indexingStartedAt) return "wedged"; + // No lock to consult. Fall back to the start marker. A raw `indexing` row + // without one predates the marker column and is abandoned; another status + // without a marker has no active-run evidence at all. + if (!root.indexingStartedAt) return root.status === "indexing" ? "wedged" : root.status; const startedAt = Date.parse(root.indexingStartedAt); if (!Number.isFinite(startedAt)) return "wedged"; return Date.now() - startedAt > INDEXING_STALL_MS ? "wedged" : "indexing"; @@ -467,13 +469,14 @@ export function indexRoot( const releaseRootLock = db ? () => undefined : acquireRootLock(root.id); const start = Date.now(); - // Committed immediately and outside the transaction below, so a concurrent - // reader can see that work started. That is also why it must carry a start - // marker: if this process is killed, nothing here runs again, and the marker - // is the only evidence that lets a later run tell 'running' from 'died'. - d.prepare( - "UPDATE index_roots SET status = 'indexing', error = NULL, indexing_started_at = ? WHERE id = ?", - ).run(new Date(start).toISOString(), root.id); + // The marker is deliberately committed before the expensive scan so other + // processes can distinguish a live run from an interrupted one. The status + // itself is not: it changes inside the file-update transaction below, where + // process death rolls it back together with every partial index mutation. + d.prepare("UPDATE index_roots SET indexing_started_at = ? WHERE id = ?").run( + new Date(start).toISOString(), + root.id, + ); try { const { files: scanned, skippedDirs } = scanRoot(root.path, root.exclude, () => @@ -513,7 +516,7 @@ export function indexRoot( const seen = new Set(); const changes: PreparedFileChange[] = []; for (const file of scanned) { - // Reading and tokenising file bodies is the longest phase of a run. + // Keep the process-owned lock fresh while reading and tokenising bodies. heartbeatRootLock(root.id); seen.add(file.relPath); const prev = existing.get(file.relPath); @@ -542,9 +545,13 @@ export function indexRoot( }); } + heartbeatRootLock(root.id, true); d.exec("BEGIN"); try { + d.prepare("UPDATE index_roots SET status = 'indexing', error = NULL WHERE id = ?").run(root.id); + for (const { file, prev, isBinary, body, grams, contentIndexed } of changes) { + heartbeatRootLock(root.id); if (prev) { if (prev.content_indexed) { deleteContent.run(prev.id); @@ -580,6 +587,7 @@ export function indexRoot( } for (const [relPath, row] of existing) { + heartbeatRootLock(root.id); if (seen.has(relPath)) continue; if (row.content_indexed) { deleteContent.run(row.id); @@ -630,7 +638,6 @@ export function refreshStaleRoots(staleMinutes: number, db?: Database): IndexSta const stats: IndexStats[] = []; for (const root of listRoots(db)) { - if (root.status === "pending") continue; if (refreshing.has(root.id)) continue; const health = rootHealth(root); @@ -640,7 +647,7 @@ export function refreshStaleRoots(staleMinutes: number, db?: Database): IndexSta // 48-day outage: the sentinel disabled its own recovery path. Recover it // regardless of staleness, because being wedged already makes every query // against it fail. The on-disk lock still prevents a real double-index. - if (health === "indexing") continue; + if (health === "indexing" || health === "pending") continue; if (health !== "wedged" && root.lastIndexedAt && Date.parse(root.lastIndexedAt) > cutoff) { continue; } diff --git a/src/lib/local/test-fixtures/crash-after-indexing-status.ts b/src/lib/local/test-fixtures/crash-after-indexing-status.ts new file mode 100644 index 0000000..ce6f81d --- /dev/null +++ b/src/lib/local/test-fixtures/crash-after-indexing-status.ts @@ -0,0 +1,43 @@ +import { Database } from "bun:sqlite"; +import { indexRoot } from "../indexer.js"; + +const [dbPath, rootId] = process.argv.slice(2); +if (!dbPath || !rootId) { + throw new Error("usage: crash-after-indexing-status "); +} + +const db = new Database(dbPath); +db.exec("PRAGMA busy_timeout = 5000"); +db.exec("PRAGMA journal_mode = WAL"); +db.exec("PRAGMA foreign_keys = ON"); + +const crashAfterStatusWrite = new Proxy(db, { + get(target, property) { + if (property === "prepare") { + return (sql: string) => { + const statement = target.prepare(sql); + if (!sql.includes("UPDATE index_roots SET status = 'indexing'")) return statement; + + return new Proxy(statement, { + get(statementTarget, statementProperty) { + if (statementProperty === "run") { + return (...bindings: unknown[]) => { + const result = Reflect.apply(statementTarget.run, statementTarget, bindings); + process.kill(process.pid, "SIGKILL"); + return result; + }; + } + + const value = Reflect.get(statementTarget, statementProperty, statementTarget); + return typeof value === "function" ? value.bind(statementTarget) : value; + }, + }); + }; + } + + const value = Reflect.get(target, property, target); + return typeof value === "function" ? value.bind(target) : value; + }, +}) as Database; + +indexRoot(rootId, {}, crashAfterStatusWrite); diff --git a/src/lib/local/wedged-index.test.ts b/src/lib/local/wedged-index.test.ts index fcba85c..f248404 100644 --- a/src/lib/local/wedged-index.test.ts +++ b/src/lib/local/wedged-index.test.ts @@ -18,8 +18,9 @@ import { describe, expect, test, beforeEach, afterEach } from "bun:test"; import { mkdtempSync, mkdirSync, writeFileSync, rmSync, utimesSync, statSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; -import type { Database } from "bun:sqlite"; +import { Database } from "bun:sqlite"; import { getIndexDbForTesting } from "../../db/index-db.js"; +import { runIndexMigrations } from "../../db/index-migrations.js"; import { addRoot, getRoot, @@ -251,6 +252,52 @@ describe("wedged index: health reporting", () => { }); describe("wedged index: recovery", () => { + test("SIGKILL after the indexing status write cannot commit a permanent wedge", async () => { + const workspace = join(root, "workspace"); + const dbPath = join(root, "index.db"); + mkdirSync(workspace, { recursive: true }); + writeFileSync(join(workspace, "a.ts"), "const crashAtomicity = 1;"); + + const fileDb = new Database(dbPath); + fileDb.exec("PRAGMA journal_mode = WAL"); + fileDb.exec("PRAGMA foreign_keys = ON"); + runIndexMigrations(fileDb); + const indexed = addRoot(workspace, {}, fileDb); + indexRoot(indexed.id, {}, fileDb); + expect(getRoot(indexed.id, fileDb)!.status).toBe("ready"); + fileDb.close(); + + const fixture = join(import.meta.dir, "test-fixtures/crash-after-indexing-status.ts"); + const child = Bun.spawn(["bun", fixture, dbPath, indexed.id], { + cwd: import.meta.dir, + stdout: "ignore", + stderr: "ignore", + }); + expect(await child.exited).not.toBe(0); + + const recoveredDb = new Database(dbPath); + recoveredDb.exec("PRAGMA journal_mode = WAL"); + recoveredDb.exec("PRAGMA foreign_keys = ON"); + try { + // The child dies immediately after executing the status update. When + // that write shares the completion transaction, SQLite rolls it back. + // Before the fix, the independently committed sentinel survives here. + expect(getRoot(indexed.id, recoveredDb)!.status).toBe("ready"); + + // Legacy rows from older releases still need the second half of the + // repair: a stale status with no live lock owner must self-heal. + recoveredDb + .prepare( + "UPDATE index_roots SET status = 'indexing', indexing_started_at = ? WHERE id = ?", + ) + .run(new Date(Date.now() - INDEXING_STALL_MS - 60_000).toISOString(), indexed.id); + expect(refreshStaleRoots(5, recoveredDb)).toHaveLength(1); + expect(getRoot(indexed.id, recoveredDb)!.status).toBe("ready"); + } finally { + recoveredDb.close(); + } + }); + test("refreshStaleRoots recovers a root wedged at 'indexing'", () => { write("a.ts", "const alumia = 1;"); const r = addRoot(root, {}, db); @@ -266,6 +313,20 @@ describe("wedged index: recovery", () => { expect(hasReadyRoot(db)).toBe(true); }); + test("a killed first index recovers from pending once its owner is stale", () => { + write("a.ts", "const firstIndexCrash = 1;"); + const r = addRoot(root, {}, db); + db.prepare("UPDATE index_roots SET indexing_started_at = ? WHERE id = ?").run( + new Date(Date.now() - INDEXING_STALL_MS - 60_000).toISOString(), + r.id, + ); + + expect(getRoot(r.id, db)!.status).toBe("pending"); + expect(rootHealth(getRoot(r.id, db)!)).toBe("wedged"); + expect(refreshStaleRoots(5, db)).toHaveLength(1); + expect(getRoot(r.id, db)!.status).toBe("ready"); + }); + test("a wedged root is recovered even when its last index looks fresh", () => { // The 48-day wedge was only visible because lastIndexedAt was ancient. A // root wedged right after a successful pass must still recover, otherwise