Merge pull request #1692 from Runfusion/fix/sqlite-read-contention
Reduce SQLite read contention: bound WAL growth + release vacuum lock
This commit is contained in:
@@ -219,3 +219,35 @@ describe("ArchiveDatabase FTS maintenance", () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("ArchiveDatabase WAL durability PRAGMAs", () => {
|
||||
let dir: string;
|
||||
let archive: ArchiveDatabase;
|
||||
|
||||
beforeEach(() => {
|
||||
dir = makeTmpDir("kb-archive-pragma-");
|
||||
archive = new ArchiveDatabase(dir);
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
archive.close();
|
||||
await rm(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("bounds WAL growth and durability like the per-project DB", () => {
|
||||
const rawDb = (archive as unknown as { db: { prepare(sql: string): { get(): unknown } } }).db;
|
||||
const synchronous = rawDb.prepare("PRAGMA synchronous").get() as { synchronous: number };
|
||||
const autoCheckpoint = rawDb
|
||||
.prepare("PRAGMA wal_autocheckpoint")
|
||||
.get() as { wal_autocheckpoint: number };
|
||||
const journalSizeLimit = rawDb
|
||||
.prepare("PRAGMA journal_size_limit")
|
||||
.get() as { journal_size_limit: number };
|
||||
|
||||
expect(synchronous.synchronous).toBe(2); // FULL
|
||||
expect(autoCheckpoint.wal_autocheckpoint).toBe(1000);
|
||||
// Previously unset (-1 / unbounded), which let the archive WAL bloat and
|
||||
// slow every reader. Now capped at 4 MB to match db.ts/central-db.ts.
|
||||
expect(journalSizeLimit.journal_size_limit).toBe(4_194_304);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -52,6 +52,24 @@ describe("CentralDatabase", () => {
|
||||
expect(Object.values(busyTimeout)[0]).toBe(5000);
|
||||
});
|
||||
|
||||
it("should bound WAL growth and durability like the per-project DB", () => {
|
||||
db.init();
|
||||
|
||||
const synchronous = db.prepare("PRAGMA synchronous").get() as { synchronous: number };
|
||||
const autoCheckpoint = db
|
||||
.prepare("PRAGMA wal_autocheckpoint")
|
||||
.get() as { wal_autocheckpoint: number };
|
||||
const journalSizeLimit = db
|
||||
.prepare("PRAGMA journal_size_limit")
|
||||
.get() as { journal_size_limit: number };
|
||||
|
||||
expect(synchronous.synchronous).toBe(2); // FULL
|
||||
expect(autoCheckpoint.wal_autocheckpoint).toBe(1000);
|
||||
// Previously unset (-1 / unbounded), which let the central WAL bloat and
|
||||
// slow every reader. Now capped at 4 MB to match db.ts.
|
||||
expect(journalSizeLimit.journal_size_limit).toBe(4_194_304);
|
||||
});
|
||||
|
||||
it("should seed lastModified on init", () => {
|
||||
db.init();
|
||||
const lastModified = db.getLastModified();
|
||||
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
MIGRATION_ONLY_TABLE_SCHEMAS,
|
||||
SCHEMA_VERSION,
|
||||
} from "../db.js";
|
||||
import { DatabaseSync } from "../sqlite-adapter.js";
|
||||
import { DEFAULT_PROJECT_SETTINGS } from "../types.js";
|
||||
import { TaskStore } from "../store.js";
|
||||
import { mkdtempSync, existsSync, readFileSync, rmSync, statSync, openSync, writeSync, closeSync } from "node:fs";
|
||||
@@ -691,6 +692,32 @@ describe("Database", () => {
|
||||
expect(result.afterBytes).toBe(expectedAfterBytes);
|
||||
});
|
||||
|
||||
it("releases the EXCLUSIVE lock so other connections can read immediately after", () => {
|
||||
const now = new Date().toISOString();
|
||||
db.prepare(
|
||||
"INSERT INTO tasks (id, description, \"column\", createdAt, updatedAt) VALUES (?, ?, ?, ?, ?)",
|
||||
).run("FN-VACUUM-LOCK", "vacuum lock task", "todo", now, now);
|
||||
|
||||
db.vacuum();
|
||||
|
||||
// vacuum() runs under PRAGMA locking_mode=EXCLUSIVE. Resetting to NORMAL
|
||||
// does not drop the file lock until the connection next touches the DB, so
|
||||
// without the forced post-vacuum read every OTHER connection would be
|
||||
// locked out (SQLITE_BUSY) until some unrelated query happened to run.
|
||||
// Probe with a second connection whose busy_timeout is 0 so a lingering
|
||||
// exclusive lock fails fast instead of blocking for the default 5s.
|
||||
const probe = new DatabaseSync(join(fusionDir, "fusion.db"));
|
||||
try {
|
||||
probe.exec("PRAGMA busy_timeout = 0");
|
||||
const row = probe
|
||||
.prepare("SELECT id FROM tasks WHERE id = ?")
|
||||
.get("FN-VACUUM-LOCK") as { id: string } | undefined;
|
||||
expect(row?.id).toBe("FN-VACUUM-LOCK");
|
||||
} finally {
|
||||
probe.close();
|
||||
}
|
||||
});
|
||||
|
||||
it("throws a descriptive error when checkpointing fails", () => {
|
||||
const checkpointSpy = vi
|
||||
.spyOn(db, "walCheckpoint")
|
||||
|
||||
@@ -73,6 +73,16 @@ export class ArchiveDatabase {
|
||||
this.db.exec("PRAGMA busy_timeout = 5000");
|
||||
if (!inMemory) {
|
||||
this.db.exec("PRAGMA journal_mode = WAL");
|
||||
// FNXC:Database 2026-06-20-12:30:
|
||||
// Mirror the per-project DB durability/maintenance PRAGMAs (db.ts). Without
|
||||
// journal_size_limit the archive WAL defaults to -1 (unbounded) and never
|
||||
// truncates back down after a checkpoint, so every reader pays an
|
||||
// ever-growing WAL-index scan — the same read-contention source bounded in
|
||||
// db.ts/central-db.ts. synchronous=FULL/wal_autocheckpoint=1000 are already
|
||||
// SQLite's defaults; set explicitly so the durability posture is intentional.
|
||||
this.db.exec("PRAGMA synchronous = FULL");
|
||||
this.db.exec("PRAGMA wal_autocheckpoint = 1000");
|
||||
this.db.exec("PRAGMA journal_size_limit = 4194304");
|
||||
}
|
||||
this._fts5Available = probeFts5(this.db);
|
||||
}
|
||||
|
||||
@@ -566,6 +566,22 @@ export class CentralDatabase implements CentralClaimStore {
|
||||
this.db.exec(`PRAGMA busy_timeout = ${this.busyTimeoutMs}`);
|
||||
// Enable WAL mode for concurrent reader/writer access
|
||||
this.db.exec("PRAGMA journal_mode = WAL");
|
||||
// FNXC:Database 2026-06-20-12:30:
|
||||
// Mirror the per-project DB durability/maintenance PRAGMAs (see db.ts). The
|
||||
// central DB is shared across every project and cluster node, so it sees the
|
||||
// most cross-process read/write traffic.
|
||||
// - synchronous=FULL and wal_autocheckpoint=1000 are already SQLite's
|
||||
// compiled-in defaults (FULL stays in effect under WAL — NORMAL is a
|
||||
// common WAL *recommendation* but not the default). Set explicitly so the
|
||||
// durability posture is intentional and visible, and so a future change to
|
||||
// synchronous=NORMAL is a deliberate edit, not an accidental drift.
|
||||
// - journal_size_limit is the load-bearing one: it defaults to -1
|
||||
// (unbounded), so without it the central WAL never truncates back down
|
||||
// after a checkpoint and every reader pays an ever-growing WAL-index scan
|
||||
// — a direct read-contention source. Cap it at 4 MB like the per-project DB.
|
||||
this.db.exec("PRAGMA synchronous = FULL");
|
||||
this.db.exec("PRAGMA wal_autocheckpoint = 1000");
|
||||
this.db.exec("PRAGMA journal_size_limit = 4194304");
|
||||
// Enable foreign key enforcement
|
||||
this.db.exec("PRAGMA foreign_keys = ON");
|
||||
}
|
||||
|
||||
@@ -2158,16 +2158,46 @@ export class Database {
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
throw new Error(`Database vacuum maintenance failed during VACUUM (dbPath=${this.dbPath}): ${message}`);
|
||||
}
|
||||
|
||||
const afterBytes = existsSync(this.dbPath) ? statSync(this.dbPath).size : 0;
|
||||
return {
|
||||
beforeBytes,
|
||||
afterBytes,
|
||||
durationMs: Date.now() - startedAt,
|
||||
};
|
||||
} finally {
|
||||
this.db.exec("PRAGMA locking_mode=NORMAL");
|
||||
// FNXC:Database 2026-06-20-12:30:
|
||||
// Switching locking_mode back to NORMAL does NOT drop the EXCLUSIVE file
|
||||
// lock immediately — in WAL mode SQLite keeps holding it until the
|
||||
// connection performs an operation that re-establishes the shared WAL
|
||||
// index. Until then every OTHER process is locked out of reads
|
||||
// (SQLITE_BUSY), so a vacuum's read-contention blast radius would extend
|
||||
// well past the vacuum itself, until some unrelated write happens to run.
|
||||
// A plain SELECT is NOT enough (it keeps running in exclusive mode); a
|
||||
// checkpoint or write is what forces the downgrade. Run a PASSIVE
|
||||
// checkpoint here — it releases the lock, is non-blocking, and keeps the
|
||||
// (already tiny, post-vacuum) WAL trimmed.
|
||||
//
|
||||
// Guard the locking_mode reset independently: if it threw, it would both
|
||||
// mask the original VACUUM/checkpoint error AND skip the lock-releasing
|
||||
// checkpoint below, leaving the EXCLUSIVE lock held — the exact failure
|
||||
// this method exists to prevent. Best-effort by design.
|
||||
try {
|
||||
this.db.exec("PRAGMA locking_mode=NORMAL");
|
||||
} catch (error) {
|
||||
console.warn("[fusion:db] vacuum: failed to reset locking_mode=NORMAL", error);
|
||||
}
|
||||
try {
|
||||
this.db.exec("PRAGMA wal_checkpoint(PASSIVE)");
|
||||
} catch (error) {
|
||||
// Lock release is best-effort (the next write drops it anyway), but log
|
||||
// it: a swallowed failure here means other processes stay locked out.
|
||||
console.warn("[fusion:db] vacuum: passive checkpoint failed; EXCLUSIVE lock may linger until the next write", error);
|
||||
}
|
||||
}
|
||||
|
||||
// Sample the file size AFTER the lock-release checkpoint above so afterBytes
|
||||
// reflects the final on-disk size (the passive checkpoint can fold a WAL
|
||||
// page back into the main db file).
|
||||
const afterBytes = existsSync(this.dbPath) ? statSync(this.dbPath).size : 0;
|
||||
return {
|
||||
beforeBytes,
|
||||
afterBytes,
|
||||
durationMs: Date.now() - startedAt,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user