Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions src/db-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,37 @@ export function closeDB(db: DatabaseInstance): void {
}
}

/**
* Start a periodic PASSIVE WAL checkpoint on `db`, returning a stop function.
*
* `closeDB()`'s `wal_checkpoint(TRUNCATE)` is the only WAL-truncation path, and
* it only runs on graceful shutdown. A server killed hard (crash, reboot,
* SIGKILL, or a Windows parent-death before the lifecycle guard fires) never
* reaches it, so under multi-session load the shared content-store WAL can grow
* unbounded — the reader-starvation failure ADR 0001 attributes to #560. A
* PASSIVE checkpoint reclaims whatever WAL frames it can between reader gaps; it
* never blocks and touches no locking, so it stays fully within ADR 0001's
* multi-writer contract (no EXCLUSIVE, no lockfile).
*
* The timer is `.unref()`'d so it never keeps the event loop alive. A
* non-positive or non-finite interval disables it and returns a no-op stopper.
*/
export function startWalCheckpointTimer(
db: DatabaseInstance,
intervalMs: number,
): () => void {
if (!Number.isFinite(intervalMs) || intervalMs <= 0) return () => {};
const timer = setInterval(() => {
try {
db.pragma("wal_checkpoint(PASSIVE)");
} catch {
/* best-effort — a busy or closing DB just retries next tick */
}
}, intervalMs);
timer.unref?.();
return () => clearInterval(timer);
}

// ─────────────────────────────────────────────────────────
// Default path helper
// ─────────────────────────────────────────────────────────
Expand Down
8 changes: 8 additions & 0 deletions src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,14 @@ function getStore(): ContentStore {
// Server just opens whatever DB exists (or creates new if hook deleted it).
const dbPath = getStorePath();
_store = new ContentStore(dbPath);
// #985: keep the shared content-store WAL bounded even on a hard exit that
// never reaches close()'s TRUNCATE checkpoint. PASSIVE never blocks and adds
// no locking (ADR 0001-safe). Default 60s; CONTEXT_MODE_WAL_CHECKPOINT_MS
// tunes it, 0 disables.
{
const raw = process.env.CONTEXT_MODE_WAL_CHECKPOINT_MS;
_store.startCheckpointTimer(raw === undefined ? 60000 : Number(raw));
}

// Wire deny-policy hook: store re-checks the Read deny list before
// re-reading any file_path during auto-refresh. Catches policy edits
Expand Down
19 changes: 18 additions & 1 deletion src/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
*/

import type { Database as DatabaseInstance } from "better-sqlite3";
import { loadDatabase, applyWALPragmas, closeDB, cleanOrphanedWALFiles, withRetry, deleteDBFiles, isSQLiteCorruptionError } from "./db-base.js";
import { loadDatabase, applyWALPragmas, closeDB, cleanOrphanedWALFiles, withRetry, deleteDBFiles, isSQLiteCorruptionError, startWalCheckpointTimer } from "./db-base.js";
import type { PreparedStatement } from "./db-base.js";
import { readFileSync, readdirSync, unlinkSync, existsSync, statSync, openSync, fstatSync, closeSync } from "node:fs";
import { createHash } from "node:crypto";
Expand Down Expand Up @@ -416,6 +416,8 @@ export class ContentStore {
// entries only become stale when new words enter — we clear on actual insert.
#fuzzyCache = new Map<string, string | null>();
static readonly FUZZY_CACHE_SIZE = 256;
/** Stops the opportunistic PASSIVE WAL checkpoint timer (#985); null when off. */
#checkpointStop: (() => void) | null = null;

constructor(dbPath?: string) {
const Database = loadDatabase();
Expand Down Expand Up @@ -450,6 +452,8 @@ export class ContentStore {

/** Delete this session's DB files. Call on process exit. */
cleanup(): void {
this.#checkpointStop?.();
this.#checkpointStop = null;
try {
this.#db.close();
} catch { /* ignore */ }
Expand Down Expand Up @@ -1612,7 +1616,20 @@ export class ContentStore {
} catch { /* best effort — don't block indexing */ }
}

/**
* Begin opportunistic PASSIVE WAL checkpoints (#985). The server calls this
* for the shared content store so the WAL stays bounded even when the process
* is killed before close() can run its TRUNCATE checkpoint. Idempotent — a
* second call replaces the prior timer. Stopped by close()/cleanup().
*/
startCheckpointTimer(intervalMs: number): void {
this.#checkpointStop?.();
this.#checkpointStop = startWalCheckpointTimer(this.#db, intervalMs);
}

close(): void {
this.#checkpointStop?.();
this.#checkpointStop = null;
this.#optimizeFTS(); // defragment before close
closeDB(this.#db); // WAL checkpoint before close — important for persistent DBs
}
Expand Down
60 changes: 60 additions & 0 deletions tests/util/wal-checkpoint-timer.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/**
* startWalCheckpointTimer — issue #985.
*
* Opportunistic PASSIVE WAL checkpoint so the shared content-store WAL stays
* bounded even when a process is killed before closeDB()'s TRUNCATE runs.
* Must: fire PASSIVE (never TRUNCATE/EXCLUSIVE — ADR 0001), stop on request,
* disable on non-positive/non-finite intervals, and swallow pragma errors.
*/
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
import { startWalCheckpointTimer } from "../../src/db-base.js";

describe("startWalCheckpointTimer (#985)", () => {
beforeEach(() => vi.useFakeTimers());
afterEach(() => vi.useRealTimers());

function fakeDb() {
return { pragma: vi.fn() } as unknown as import("better-sqlite3").Database & { pragma: ReturnType<typeof vi.fn> };
}

it("issues a PASSIVE checkpoint on each interval", () => {
const db = fakeDb();
const stop = startWalCheckpointTimer(db, 1000);
expect(db.pragma).not.toHaveBeenCalled();
vi.advanceTimersByTime(3000);
expect(db.pragma).toHaveBeenCalledTimes(3);
expect(db.pragma).toHaveBeenCalledWith("wal_checkpoint(PASSIVE)");
// ADR 0001: never a blocking/locking variant.
for (const [arg] of (db.pragma as ReturnType<typeof vi.fn>).mock.calls) {
expect(arg).not.toMatch(/TRUNCATE|EXCLUSIVE|locking_mode/);
}
stop();
});

it("stops firing after the returned stopper is called", () => {
const db = fakeDb();
const stop = startWalCheckpointTimer(db, 1000);
vi.advanceTimersByTime(1000);
expect(db.pragma).toHaveBeenCalledTimes(1);
stop();
vi.advanceTimersByTime(5000);
expect(db.pragma).toHaveBeenCalledTimes(1); // no further ticks
});

it("is a no-op for non-positive or non-finite intervals", () => {
const db = fakeDb();
for (const bad of [0, -1, NaN, Infinity]) {
const stop = startWalCheckpointTimer(db, bad);
vi.advanceTimersByTime(10000);
stop(); // must be safe to call
}
expect(db.pragma).not.toHaveBeenCalled();
});

it("swallows pragma errors so a busy/closing DB never crashes the timer", () => {
const db = { pragma: vi.fn(() => { throw new Error("SQLITE_BUSY"); }) } as unknown as import("better-sqlite3").Database;
const stop = startWalCheckpointTimer(db, 1000);
expect(() => vi.advanceTimersByTime(2000)).not.toThrow();
stop();
});
});