From 7120da16442fcb5354945d8e6c985cd9f3c6c799 Mon Sep 17 00:00:00 2001 From: Max Hsu Date: Sun, 5 Jul 2026 21:58:58 +0800 Subject: [PATCH] fix(db): probe WAL support and fall back to DELETE on unsupported filesystems (#990) configureConnection set journal_mode=WAL unconditionally. On filesystems where mmap(MAP_SHARED) returns EOPNOTSUPP for the WAL-index (ntfs3, WSL2 /mnt, some CIFS/NFS), PRAGMA journal_mode=WAL reports "wal" but the first write fails with SQLITE_IOERR and the connection is already corrupted, so `codegraph init` dies with "disk I/O error". Probe WAL on a throwaway database up front (walAvailable) and only enable it on the real connection when the probe survives an actual write; otherwise fall back to DELETE mode with synchronous=FULL. configureConnection now takes an explicit useWal flag computed at the two call sites. --- __tests__/wal-fallback.test.ts | 73 ++++++++++++++++++++++++++++++++++ src/db/index.ts | 70 +++++++++++++++++++++++++++----- 2 files changed, 132 insertions(+), 11 deletions(-) create mode 100644 __tests__/wal-fallback.test.ts diff --git a/__tests__/wal-fallback.test.ts b/__tests__/wal-fallback.test.ts new file mode 100644 index 000000000..f0eba6976 --- /dev/null +++ b/__tests__/wal-fallback.test.ts @@ -0,0 +1,73 @@ +import { describe, it, expect } from 'vitest'; +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; +import { DatabaseConnection, walAvailable, configureConnection } from '../src/db'; +import { createDatabase } from '../src/db/sqlite-adapter'; + +/** + * Regression coverage for #990: WAL mode must be probed before it is enabled, + * and the connection must fall back to DELETE mode (still writable) when the + * filesystem can't sustain WAL. The EOPNOTSUPP path itself (ntfs3 / WSL2 /mnt / + * some CIFS/NFS) cannot be reproduced on APFS, so these tests exercise the + * branching logic and the fallback's write path directly. + */ +describe('WAL fallback (#990)', () => { + it('walAvailable returns true on a normal writable dir and cleans up its probe', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-wal-ok-')); + try { + expect(walAvailable(dir)).toBe(true); + const leftovers = fs.readdirSync(dir).filter((f) => f.startsWith('.cg-wal-probe-')); + expect(leftovers).toEqual([]); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('walAvailable returns false when the dir cannot host a database', () => { + const missing = path.join(os.tmpdir(), `cg-wal-missing-${Date.now()}`, 'nested'); + expect(walAvailable(missing)).toBe(false); + }); + + it('configureConnection(useWal=true) enables WAL', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-wal-on-')); + try { + const { db } = createDatabase(path.join(dir, 't.db')); + configureConnection(db, true); + expect(db.pragma('journal_mode', { simple: true })).toBe('wal'); + expect(db.pragma('synchronous', { simple: true })).toBe(1); // NORMAL, safe under WAL + db.close(); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('configureConnection(useWal=false) falls back to DELETE and stays writable', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-wal-off-')); + try { + const { db } = createDatabase(path.join(dir, 't.db')); + configureConnection(db, false); + expect(db.pragma('journal_mode', { simple: true })).toBe('delete'); + expect(db.pragma('synchronous', { simple: true })).toBe(2); // FULL, durable under DELETE + // The point of the fallback: writes must still succeed in DELETE mode. + db.exec('CREATE TABLE t (x); INSERT INTO t (x) VALUES (1);'); + const row = db.prepare('SELECT x FROM t').get() as { x: number }; + expect(row.x).toBe(1); + expect(db.pragma('journal_mode', { simple: true })).toBe('delete'); + db.close(); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('DatabaseConnection.initialize produces a working DB (WAL on a normal fs)', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-wal-init-')); + try { + const conn = DatabaseConnection.initialize(path.join(dir, 'test.db')); + expect(conn.getDb().pragma('journal_mode', { simple: true })).toBe('wal'); + conn.close(); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/src/db/index.ts b/src/db/index.ts index 6b13be467..c89f06b89 100644 --- a/src/db/index.ts +++ b/src/db/index.ts @@ -27,16 +27,63 @@ export { SqliteDatabase, SqliteBackend } from './sqlite-adapter'; * on a writer, so this timeout only governs cross-process write contention * (e.g. the git-hook `codegraph sync` running while the MCP server writes). */ -function configureConnection(db: SqliteDatabase): void { +export function configureConnection(db: SqliteDatabase, useWal: boolean): void { db.pragma('busy_timeout = 5000'); // MUST be first — see above db.pragma('foreign_keys = ON'); - db.pragma('journal_mode = WAL'); // node:sqlite supports WAL on every platform - db.pragma('synchronous = NORMAL'); // safe with WAL mode + if (useWal) { + db.pragma('journal_mode = WAL'); + db.pragma('synchronous = NORMAL'); // safe with WAL mode + } else { + // Filesystem can't sustain WAL (see walAvailable / #990) — fall back to the + // rollback journal. synchronous=NORMAL is only crash-safe under WAL, so use + // FULL here to keep durability guarantees in DELETE mode. + db.pragma('journal_mode = DELETE'); + db.pragma('synchronous = FULL'); + } db.pragma('cache_size = -64000'); // 64 MB page cache db.pragma('temp_store = MEMORY'); // temp tables in memory db.pragma('mmap_size = 268435456'); // 256 MB memory-mapped I/O } +/** + * Probe whether the filesystem backing `dir` can actually sustain WAL mode. + * + * `PRAGMA journal_mode = WAL` reports success ("wal") even on filesystems that + * cannot support it: ntfs3, WSL2 `/mnt`, and some CIFS/NFS return EOPNOTSUPP for + * `mmap(MAP_SHARED)` on the WAL-index, and the failure only surfaces on the + * first write (SQLITE_IOERR) — by which point the real connection is already + * corrupted and can no longer be switched to another mode. So we exercise WAL + * on a throwaway database first and only enable it on the real connection when + * the probe survives an actual write. See #990. + */ +export function walAvailable(dir: string): boolean { + const probePath = path.join(dir, `.cg-wal-probe-${process.pid}-${Date.now()}.db`); + const cleanup = (): void => { + for (const suffix of ['', '-wal', '-shm']) { + try { + fs.unlinkSync(probePath + suffix); + } catch { + // best-effort: nothing to remove if the file was never created + } + } + }; + try { + const { db } = createDatabase(probePath); + try { + db.pragma('journal_mode = WAL'); + // The write is what actually exercises the WAL-index mmap. + db.exec('CREATE TABLE _cg_wal_probe (x); DROP TABLE _cg_wal_probe;'); + } finally { + db.close(); + } + cleanup(); + return true; + } catch { + cleanup(); + return false; + } +} + /** * Database connection wrapper with lifecycle management */ @@ -74,7 +121,7 @@ export class DatabaseConnection { // Create and configure database const { db, backend } = createDatabase(dbPath); - configureConnection(db); + configureConnection(db, walAvailable(dir)); // Run schema initialization const schemaPath = path.join(__dirname, 'schema.sql'); @@ -102,7 +149,7 @@ export class DatabaseConnection { const { db, backend } = createDatabase(dbPath); - configureConnection(db); + configureConnection(db, walAvailable(path.dirname(dbPath))); // Check and run migrations if needed const conn = new DatabaseConnection(db, dbPath, backend); @@ -141,12 +188,13 @@ export class DatabaseConnection { /** * The journal mode actually in effect (e.g. 'wal', 'delete'). * - * SQLite silently keeps the prior mode if WAL can't be enabled — e.g. on - * filesystems without shared-memory support (some network/virtualized mounts, - * WSL2 /mnt). So the effective mode can differ - * from what `configureConnection` requested. Surfaced in `codegraph status` so - * a "database is locked" report is triageable: 'wal' ⇒ readers never block on a - * writer; anything else ⇒ they can. See issue #238. + * `configureConnection` picks the mode up front: WAL only when `walAvailable` + * proved the filesystem can sustain it, otherwise an explicit DELETE fallback + * (see #990). So this reflects that deliberate choice — not a silent SQLite + * downgrade. SQLite does NOT quietly fall back: it reports WAL as active and + * then fails on the first write, which is exactly why we probe. Surfaced in + * `codegraph status` so a "database is locked" report is triageable: 'wal' ⇒ + * readers never block on a writer; anything else ⇒ they can. See issues #238, #990. */ getJournalMode(): string { const raw = this.db.pragma('journal_mode');