diff --git a/.changeset/sqlite-wal-journal-mode.md b/.changeset/sqlite-wal-journal-mode.md new file mode 100644 index 0000000000..4de545db08 --- /dev/null +++ b/.changeset/sqlite-wal-journal-mode.md @@ -0,0 +1,65 @@ +--- +'@objectstack/driver-sql': minor +'@objectstack/driver-sqlite-wasm': patch +'@objectstack/cli': patch +--- + +File-backed SQLite now runs `journal_mode = WAL` (#3941). + +`SqlDriver.connect()` set `auto_vacuum` and left the journal mode alone, so +every ObjectStack SQLite database ran SQLite's built-in default — a rollback +journal. That is the worst mode for the shape this platform actually has, which +is **several processes on one file**: a dev server, `os migrate`, +`os meta resync`, a test run. Measured, on the same file: + +| | rollback journal | WAL | +|:---|:---|:---| +| writer while another process holds a read open | `SQLITE_BUSY` — committing needs an exclusive lock | proceeds | +| idle attached connection visible to SQL | no — a lock lasts only as long as its transaction | yes (`locking_mode = EXCLUSIVE` + `BEGIN IMMEDIATE` reports busy) | + +The second row is why the `os migrate` occupancy check had to inspect file +descriptors to see a live server at all (#3940): under a rollback journal there +was nothing in the database to see. That signal stays — it names the process, +which WAL's lock probe cannot — but the SQL probe is now authoritative for +databases ObjectStack created rather than a fallback that was blind in practice. +Concurrent *writers* still serialize; SQLite allows one at a time in any mode. + +Journal mode is a persistent property of the file, so an existing database is +converted in place on the next connect (a header change — no rows are touched) +and stays converted. Two consequences to plan for: + +- `app.db-wal` / `app.db-shm` exist beside the database while a connection is + attached, and `app.db-wal` can hold committed transactions. A clean shutdown + checkpoints them away; a naive copy of `app.db` alone while a server runs does + not. Use `sqlite3 app.db ".backup …"`. +- **WAL does not work on network filesystems** (NFS/SMB). Opt out with + `OS_DATABASE_SQLITE_JOURNAL_MODE=delete`, or per datasource with + `sqliteJournalMode: 'delete'` in the driver config (which outranks the env + var). Either form *applies* `delete`, so it also converts a database that + already adopted WAL back — skipping would have stranded it. + +Nothing here fails a boot, and nothing is assumed: `PRAGMA journal_mode = X` +answers with the mode actually in force rather than raising on refusal, so the +reply is read back; and because a filesystem can accept WAL and then fail the +first read *through* it, the mode is proven with a read and rolled back to +`delete` if that fails — with a warning naming the file and the escape hatch. +`synchronous` is untouched, so durability is exactly what it was. `:memory:` +databases are left alone, as is `auto_vacuum = INCREMENTAL`, which keeps +reclaiming under WAL (ADR-0057). + +`os db clean` now counts `-wal` / `-shm` as part of the database when it measures +what a `VACUUM` reclaimed, so bytes that were sitting in the log do not read as a +reclaim of zero. + +`@objectstack/driver-sqlite-wasm` deliberately stays out of WAL. Its live +database is in the WASM heap and what reaches disk is a byte image it exports, so +nothing reads the database across processes and the pragma buys it nothing — +while still being a persistent header change in the operator's file. sql.js +*accepts* the pragma (its VFS is memory-backed), so this had to be declared +rather than discovered. + +It also now parks a `-wal` left behind by an unclean native-driver exit rather +than loading the image beside it: wasm SQLite cannot read that log, and leaving +it next to a freshly rewritten image would let a later real SQLite replay frames +that no longer belong to it. The warning names the file it parked and how to +recover what was in it. diff --git a/content/docs/data-modeling/drivers.mdx b/content/docs/data-modeling/drivers.mdx index 96641b1ec3..2955712ff6 100644 --- a/content/docs/data-modeling/drivers.mdx +++ b/content/docs/data-modeling/drivers.mdx @@ -279,6 +279,64 @@ export OS_DATABASE_URL=file:./data/app.db export OS_DATABASE_URL=:memory: ``` +### Journal mode: WAL, and cross-process access + +A file-backed SQLite database is switched to **WAL** (write-ahead logging) the +first time ObjectStack connects to it. SQLite's own default is a rollback +journal (`journal_mode = delete`), which is the wrong trade for the way this +platform is actually used — **several processes on one file**: a dev server, +`os migrate`, `os meta resync`, a test run. + +| | rollback journal (SQLite default) | WAL (ObjectStack default) | +|:---|:---|:---| +| Reader while another process writes | allowed until the writer commits | always allowed (last committed snapshot) | +| Writer while another process reads | **blocked** — committing needs an exclusive lock (`SQLITE_BUSY`) | allowed | +| Idle connection visible to other processes | no — a lock lasts only as long as its transaction | yes, which is what makes the [`os migrate` occupancy check](/docs/deployment/cli#occupancy-check-sqlite) reliable | +| Files on disk | `app.db` (plus a transient `app.db-journal`) | `app.db`, plus `app.db-wal` / `app.db-shm` while a connection is attached | + +Concurrent *writers* still serialize — SQLite allows one at a time in any mode. + +Two consequences worth knowing: + +- **Journal mode is stored in the file.** One connection sets it and it stays + set, so an existing database is converted in place on the next boot (a header + change; no rows are touched) and stays in WAL even when opened by other tools. +- **`app.db-wal` can hold committed data.** A clean shutdown checkpoints and + removes it, but do not copy or restore `app.db` on its own while a server is + attached — use `sqlite3 app.db ".backup …"`, as + [Backup & restore](/docs/deployment/backup-restore#sqlite-single-host) + describes. + +**Opting out.** WAL requires shared memory beside the database and therefore +**does not work on network filesystems** (NFS/SMB). Set the journal mode back +per deployment: + +```bash +export OS_DATABASE_SQLITE_JOURNAL_MODE=delete +``` + +…or per datasource, which outranks the env var: + +```typescript +new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: '/mnt/share/app.db' }, + useNullAsDefault: true, + sqliteJournalMode: 'delete', +}); +``` + +Either form *applies* `delete`, so it also converts a database that already +adopted WAL back. The driver never assumes the switch worked: it reads the mode +back and, if WAL is accepted but cannot be read through (the usual network-share +symptom), reverts to `delete` and says so. A database that stays on a rollback +journal is slower under concurrency, never broken. + +`:memory:` databases and the WASM SQLite driver are left on their own journal +mode. Neither is shared between processes: the WASM driver's live database sits +in the WASM heap and what reaches disk is a byte image it exports, so there is no +cross-process concurrency for WAL to buy. + ### Space Reclamation & Table Rotation (ADR-0057) The SQL driver connects SQLite with `auto_vacuum=INCREMENTAL` and exposes diff --git a/content/docs/deployment/backup-restore.mdx b/content/docs/deployment/backup-restore.mdx index 3ebf00e39a..ec2254194b 100644 --- a/content/docs/deployment/backup-restore.mdx +++ b/content/docs/deployment/backup-restore.mdx @@ -57,6 +57,17 @@ sqlite3 /srv/data/app.db ".backup '/backups/app-$(date +%F).db'" The default location when `OS_DATABASE_URL` is unset is `/data/objectstack.db` under the ObjectStack home directory. + +ObjectStack keeps file-backed SQLite in +[WAL mode](/docs/data-modeling/drivers#journal-mode-wal-and-cross-process-access), +so `app.db-wal` beside the database can hold committed transactions that are not +yet in `app.db`. `.backup` accounts for that; copying `app.db` on its own does +not. If you must copy files, stop the server first (a clean shutdown checkpoints +and removes `-wal`/`-shm`), or copy `app.db`, `app.db-wal` and `app.db-shm` +together — a database restored beside a *stale* log is worse than one restored +without it. + + ### Managed databases (hosted Postgres, MongoDB Atlas, …) Use the provider's snapshot, branching, or point-in-time-restore facilities. @@ -87,7 +98,9 @@ Fresh container or VM. Install Node 22+ and `@objectstack/cli`. ```bash pg_restore --clean --if-exists -d "$OS_DATABASE_URL" backup-2026-07-14.dump -# or copy the SQLite file / restore the provider snapshot +# or restore the provider snapshot +# SQLite: put the `.backup` file in place with the server stopped, and make sure +# no stale app.db-wal / app.db-shm is left beside it (see the warning above) ``` ### Provide the original secrets diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index b0e5156a76..7b77db491a 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -541,15 +541,16 @@ schema cookie that the migration invalidates, and its writes can collide as `SQLITE_BUSY`. Before booting, `os migrate` checks two things: 1. **Which processes hold the file open** (`/proc` on Linux, `lsof` on macOS). - This is the signal that fires in practice. ObjectStack's sqlite driver runs - `journal_mode = delete`, and a rollback-journal database keeps no record of - who is attached — an idle server holds no lock at all, so no SQL probe can - see it. An open file descriptor is what it does leave, and it names the - process to go and stop. + The signal that works in every journal mode, and the only one that names the + process to go and stop. It is also the only one that sees an *idle* server on + a rollback-journal database, where a lock lasts no longer than the + transaction that took it. 2. **A SQL lock probe** (`PRAGMA locking_mode = EXCLUSIVE` under - `busy_timeout = 0`). On a WAL database this catches an attached connection - even when step 1 cannot run — a platform without process inspection, or a - database held by another user's process. + `busy_timeout = 0`). ObjectStack keeps file-backed SQLite in + [WAL mode](/docs/data-modeling/drivers#journal-mode-wal-and-cross-process-access), + where this catches any attached connection — idle or not — including the cases + step 1 cannot reach: a platform without process inspection, or a database held + by another user's process. Either one firing counts as busy. Both are non-destructive: no row is read or written. diff --git a/content/docs/deployment/environment-variables.mdx b/content/docs/deployment/environment-variables.mdx index 726e89e521..cc07ce0b4d 100644 --- a/content/docs/deployment/environment-variables.mdx +++ b/content/docs/deployment/environment-variables.mdx @@ -49,6 +49,7 @@ read at startup unless noted otherwise. Boolean variables accept `true` / `false |:---|:---|:---|:---| | `OS_DATABASE_URL` | url | — | Database connection string (e.g. `file:./data.sqlite`, `postgres://…`, `mongodb://…`, `memory://`). `libsql://` (Turso) is not supported. | | `OS_DATABASE_DRIVER` | enum | inferred | Force a specific driver when the URL is ambiguous. `memory` \| `sqlite` \| `sqlite-wasm` \| `postgres` \| `mongodb`. | +| `OS_DATABASE_SQLITE_JOURNAL_MODE` | enum | `wal` | Journal mode for **file-backed** SQLite. `wal` (default) lets a dev server and CLI commands share one file without blocking each other, and is what makes the `os migrate` occupancy check reliable. Set to `delete` for SQLite's rollback journal — required when the database lives on a **network filesystem** (NFS/SMB), where WAL cannot work. The setting is applied, not merely skipped: `delete` converts a database that already adopted WAL back. Ignored for `:memory:`, for the WASM SQLite driver, and for non-SQLite drivers. A per-datasource `sqliteJournalMode` in driver config outranks it. See [Journal mode](/docs/data-modeling/drivers#journal-mode-wal-and-cross-process-access). | | `OS_ALLOW_DRIVER_CONNECT_FAILURE` | boolean | `false` | Escape hatch for the driver-connect boot guard. By default a data driver that fails to connect at startup **refuses the boot** — a server that cannot reach its database must not report itself started and then fail every request. The same guard covers a **declared datasource** that objects bind to via `datasource: '…'`, or an `external` one with `validation.onMismatch: 'fail'`: those objects have no fallback datasource, so an unconnected one means they are all dead. Set to `1` to boot anyway, in an explicitly degraded state logged loudly at startup. There is **no reconnection**: whatever failed stays dead for the process lifetime and every query and schema sync routed to it fails. | | `OS_STORAGE_ROOT` | path | `./.objectstack/data/uploads` | Root directory for the local file storage adapter, relative to the process cwd (used by `os serve`'s default `storage` capability wiring). | | `OS_ARTIFACT_PATH` | path | — | Path or `http(s)://` URL to a compiled `objectstack.json` artifact to boot the kernel from. | diff --git a/content/docs/releases/v17.mdx b/content/docs/releases/v17.mdx index 80664f9391..09119a4a12 100644 --- a/content/docs/releases/v17.mdx +++ b/content/docs/releases/v17.mdx @@ -990,6 +990,17 @@ most of its rows in silence. state, and requires each to be classified (`scope` / `closed` / `open` / `output`) with a rationale. Omission is the commonest authoring error a model makes; it must not also be the widest grant. +- **File-backed SQLite runs in WAL mode** (#3941). SQLite's built-in rollback + journal makes a writer wait for every reader and leaves an idle connection + invisible to SQL — both wrong for the platform's normal shape, several + processes on one file (a dev server, `os migrate`, a test run). The driver now + switches such a database to WAL on connect, which is also what lets the + `os migrate` occupancy check *see* an idle server instead of inferring one from + file descriptors. Two things to know: `app.db-wal` / `app.db-shm` appear beside + the database while a connection is attached (so back up with + `sqlite3 … ".backup"`, never a bare file copy), and WAL cannot work on a + **network filesystem** — set `OS_DATABASE_SQLITE_JOURNAL_MODE=delete` there, + which converts an already-switched database back. - Metadata-plane FLS (per-caller masking) is proposed as ADR-0106. ## New in Console (Studio) — bundled objectui 17.0 diff --git a/packages/cli/src/commands/db/clean.ts b/packages/cli/src/commands/db/clean.ts index d51b321ea4..0a41a3dc62 100644 --- a/packages/cli/src/commands/db/clean.ts +++ b/packages/cli/src/commands/db/clean.ts @@ -68,9 +68,20 @@ export default class DbClean extends Command { const { resolveSqliteDriver } = await import('@objectstack/service-datasource'); + // Count the WAL sidecars as part of the database (#3941): a file-backed + // SQLite database runs in WAL mode, so bytes waiting in `-wal` are bytes on + // disk. Measuring `app.db` alone would compare a before that excludes them + // against an after that has absorbed them — and report a reclaim of zero (or + // less) for a VACUUM that genuinely shrank things. + const sizeOnDisk = (file: string): number => + [file, `${file}-wal`, `${file}-shm`].reduce( + (total, path) => total + (existsSync(path) ? statSync(path).size : 0), + 0, + ); + let failed = false; for (const file of targets) { - const before = statSync(file).size; + const before = sizeOnDisk(file); try { const resolved = await resolveSqliteDriver({ filename: file, @@ -86,7 +97,9 @@ export default class DbClean extends Command { await resolved.driver.execute('VACUUM'); await resolved.driver.disconnect(); - const after = statSync(file).size; + // Measured after `disconnect()`, which checkpoints and removes the + // sidecars — so this is the settled on-disk size either way. + const after = sizeOnDisk(file); const saved = before - after; const fmt = (n: number) => `${(n / 1024 / 1024).toFixed(2)} MB`; console.log( diff --git a/packages/cli/src/utils/sqlite-occupancy.test.ts b/packages/cli/src/utils/sqlite-occupancy.test.ts index 7f47b7f077..cf75cd4419 100644 --- a/packages/cli/src/utils/sqlite-occupancy.test.ts +++ b/packages/cli/src/utils/sqlite-occupancy.test.ts @@ -165,12 +165,15 @@ describe('probeSqliteOccupancy', () => { }); /** - * The dogfood regression. ObjectStack's sqlite driver runs - * `journal_mode = delete`, and an idle connection to a rollback-journal - * database holds NO lock — so the SQL probe reports idle and, before the + * The dogfood regression. An idle connection to a rollback-journal database + * holds NO lock — so the SQL probe reports idle and, before the * file-descriptor signal existed, `os migrate apply` ran unannounced against * a live `os serve`. Verified by hand against a real CRM project before this * test was written; the test is what keeps it fixed. + * + * Still reachable after #3941 made WAL the driver default: a database on a + * filesystem that cannot host WAL, or one explicitly opted back out with + * `OS_DATABASE_SQLITE_JOURNAL_MODE=delete`, lands here again. */ it('reports busy for a ROLLBACK-JOURNAL database an idle connection holds open', async () => { const file = newDb('busy-journal-idle.db', 'delete'); diff --git a/packages/cli/src/utils/sqlite-occupancy.ts b/packages/cli/src/utils/sqlite-occupancy.ts index 41dc99ac66..2ba81769bf 100644 --- a/packages/cli/src/utils/sqlite-occupancy.ts +++ b/packages/cli/src/utils/sqlite-occupancy.ts @@ -15,35 +15,40 @@ * ## Two independent signals, because neither covers the ground alone * * **1. Which processes hold the file open** (`/proc` on Linux, `lsof` on - * macOS). This is the signal that actually fires in practice, and it took - * dogfooding to learn why: ObjectStack's sqlite driver runs - * `journal_mode = delete`, NOT WAL. A rollback-journal database keeps no - * persistent record of who is attached — locks exist only for the duration of - * a transaction — so an idle `os serve` holds no lock at all and no amount of - * SQL can see it. An open file descriptor is the only thing it does leave, and - * it names the culprit, which is what an operator needs anyway. + * macOS). The signal that survives every journal mode, and the only one that + * names the culprit — "stop pid 12367 (node)" is worth more to an operator than + * "something is using it". It is also the only one that fires on a + * rollback-journal database, where locks exist just for the duration of a + * transaction, so an idle `os serve` holds nothing any query could see. That is + * what dogfooding caught (#3940), back when every ObjectStack database ran + * `journal_mode = delete`. * * **2. A SQL lock probe** — `PRAGMA locking_mode = EXCLUSIVE` then - * `BEGIN IMMEDIATE` under `busy_timeout = 0`. On WAL this catches an attached - * connection even when signal 1 cannot run (a foreign platform, a hardened - * container with no `/proc` visibility, a connection held by another *user's* - * process); on a rollback journal it narrows to "someone is mid-write". Kept - * because it is authoritative where it applies and costs a millisecond. + * `BEGIN IMMEDIATE` under `busy_timeout = 0`. Since #3941 the driver keeps + * file-backed databases in **WAL**, where this reports `SQLITE_BUSY` against an + * *attached* connection whether or not it is doing anything — so for a database + * ObjectStack created it is authoritative, and it reaches the cases signal 1 + * cannot: a platform with no process inspection, a hardened container with no + * `/proc` visibility, a connection held by another *user's* process. On a + * database still (or deliberately) on a rollback journal it narrows back to + * "someone is mid-write", which is why signal 1 stays first. * * Either signal firing means busy. Three rejected alternatives, all measured * rather than assumed: * - * - **`-wal` / `-shm` presence alone.** Correct while a connection is open, but - * it cannot tell a live server from a crashed one, and it does not exist at - * all on a rollback journal — which is what we actually ship. Sidecars are - * reported as supporting evidence, never as the verdict. + * - **`-wal` / `-shm` presence alone.** Correct while a connection is open — a + * clean close checkpoints and removes them — but it cannot tell a live server + * from a crashed one, and it says nothing at all about a database on a + * rollback journal. Sidecars are reported as supporting evidence, never as + * the verdict. * - **`PRAGMA wal_checkpoint(TRUNCATE)`.** Reports `busy = 1` only when a * *writer* is mid-transaction; an attached-but-idle connection checkpoints * cleanly. Misses precisely the common case. * - **The SQL probe on its own.** What shipped first, and what dogfooding * caught: against a real `os serve` holding a real project database, it * reported idle and the migration ran unannounced. Correct on WAL, blind on - * the mode the platform defaults to. + * the mode the platform defaulted to then — and still blind on any database + * opted back out to `delete`, so it does not get to be the only signal. * * Both probes are non-destructive: an empty transaction rolled back with * locking mode restored, and read-only filesystem inspection. No row is read diff --git a/packages/plugins/driver-sql/src/index.ts b/packages/plugins/driver-sql/src/index.ts index 964accf2ba..5b5d437a8a 100644 --- a/packages/plugins/driver-sql/src/index.ts +++ b/packages/plugins/driver-sql/src/index.ts @@ -5,6 +5,7 @@ import { SqlDriver } from './sql-driver.js'; export { SqlDriver }; export type { SqlDriverConfig, + SqliteJournalMode, IntrospectedSchema, IntrospectedTable, IntrospectedColumn, diff --git a/packages/plugins/driver-sql/src/sql-driver-sqlite-journal-mode.test.ts b/packages/plugins/driver-sql/src/sql-driver-sqlite-journal-mode.test.ts new file mode 100644 index 0000000000..02b5e326ff --- /dev/null +++ b/packages/plugins/driver-sql/src/sql-driver-sqlite-journal-mode.test.ts @@ -0,0 +1,299 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #3941: every ObjectStack SQLite database used to run SQLite's built-in +// `journal_mode = delete`, which is the worst mode for the platform's normal +// shape — several processes on one file (a dev server, `os migrate`, a test +// run). `SqlDriver.connect()` now asks a file-backed database for WAL, with an +// escape hatch for the filesystems where WAL cannot work. + +import { describe, it, expect, afterEach, beforeEach, vi } from 'vitest'; +import { existsSync, mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { SqlDriver } from './sql-driver.js'; + +const dirs: string[] = []; +const drivers: SqlDriver[] = []; + +function tempDb(): string { + const dir = mkdtempSync(join(tmpdir(), 'os-journal-')); + dirs.push(dir); + return join(dir, 'app.db'); +} + +function make(cfg: Record): SqlDriver & { knex: any; logger: any } { + const d = new SqlDriver({ useNullAsDefault: true, ...cfg } as any); + drivers.push(d); + return d as any; +} + +function sqlite(filename: string, cfg: Record = {}) { + return make({ client: 'better-sqlite3', connection: { filename }, ...cfg }); +} + +/** `PRAGMA journal_mode` as the file itself reports it. */ +async function journalMode(driver: any): Promise { + const rows = await driver.knex.raw('PRAGMA journal_mode'); + return String(rows[0].journal_mode).toLowerCase(); +} + +/** Silence + capture the driver's warnings. */ +function captureWarnings(driver: any): string[] { + const seen: string[] = []; + driver.logger = { warn: (msg: string) => seen.push(String(msg)) }; + return seen; +} + +/** + * Stand in for `knex.raw`, which knex itself defines read-only. `pass` runs the + * real thing, so a stub only has to describe the statements it cares about. + */ +function stubRaw( + driver: any, + impl: (sql: string, pass: () => Promise) => Promise, +): void { + const real = driver.knex.raw.bind(driver.knex); + Object.defineProperty(driver.knex, 'raw', { + configurable: true, + writable: true, + value: (sql: string, ...rest: unknown[]) => + impl(String(sql), () => real(sql, ...rest) as Promise), + }); +} + +const ENV = 'OS_DATABASE_SQLITE_JOURNAL_MODE'; +let savedEnv: string | undefined; + +beforeEach(() => { + savedEnv = process.env[ENV]; + delete process.env[ENV]; +}); + +afterEach(async () => { + if (savedEnv === undefined) delete process.env[ENV]; + else process.env[ENV] = savedEnv; + await Promise.all(drivers.splice(0).map((d) => d.disconnect().catch(() => {}))); + dirs.splice(0).forEach((d) => rmSync(d, { recursive: true, force: true })); +}); + +describe('SqlDriver — SQLite journal mode (#3941)', () => { + it('puts a file-backed database in WAL mode on connect', async () => { + const file = tempDb(); + const d = sqlite(file); + const warnings = captureWarnings(d); + + await d.connect(); + + expect(await journalMode(d)).toBe('wal'); + expect(warnings).toEqual([]); + // The shared-memory index and the log itself only exist while a connection + // is attached — which is exactly what makes occupancy visible (#3940). + expect(existsSync(`${file}-wal`)).toBe(true); + }); + + it('converts an existing rollback-journal database in place', async () => { + const file = tempDb(); + + const first = sqlite(file, { sqliteJournalMode: 'delete' }); + await first.connect(); + await first.knex.raw('CREATE TABLE t (a integer)'); + await first.knex.raw('INSERT INTO t VALUES (1)'); + expect(await journalMode(first)).toBe('delete'); + await first.disconnect(); + + const second = sqlite(file); + await second.connect(); + expect(await journalMode(second)).toBe('wal'); + // Conversion is a header change, not a rebuild: every row survives it. + const rows = await second.knex.raw('SELECT a FROM t'); + expect(rows).toEqual([{ a: 1 }]); + }); + + it('keeps the ADR-0057 auto_vacuum=INCREMENTAL default alongside WAL', async () => { + const d = sqlite(tempDb()); + await d.connect(); + + expect(await journalMode(d)).toBe('wal'); + const av = await d.knex.raw('PRAGMA auto_vacuum'); + expect(av[0].auto_vacuum).toBe(2); // 2 = INCREMENTAL + // The reclaim path ADR-0057 relies on still runs under WAL. + await expect(d.knex.raw('PRAGMA incremental_vacuum')).resolves.toBeDefined(); + }); + + it('leaves `:memory:` alone — it has no journal and nobody to share it with', async () => { + const d = sqlite(':memory:'); + const warnings = captureWarnings(d); + + await d.connect(); + + expect(await journalMode(d)).toBe('memory'); + expect(warnings).toEqual([]); + }); + + it('is a no-op for non-SQLite dialects', async () => { + const d = make({ client: 'pg', connection: 'postgres://u:p@127.0.0.1:1/d' }); + const raw = vi.spyOn(d.knex, 'raw'); + // `ensureDatabaseExists` probes `SELECT 1` on pg and fails to reach the + // host; the point is only that no PRAGMA is ever issued. + await d.connect().catch(() => {}); + for (const call of raw.mock.calls) { + expect(String(call[0])).not.toMatch(/journal_mode/i); + } + }); + + describe('opt-out for filesystems that cannot host WAL', () => { + it('applies `delete` from config, reverting a database already in WAL', async () => { + const file = tempDb(); + const converted = sqlite(file); + await converted.connect(); + expect(await journalMode(converted)).toBe('wal'); + await converted.disconnect(); + + // Journal mode is persistent, so opting out has to *apply* `delete`: + // merely skipping would leave the database in WAL forever. + const optedOut = sqlite(file, { sqliteJournalMode: 'delete' }); + const warnings = captureWarnings(optedOut); + await optedOut.connect(); + + expect(await journalMode(optedOut)).toBe('delete'); + expect(warnings).toEqual([]); + expect(existsSync(`${file}-wal`)).toBe(false); + }); + + it(`honours ${ENV}=delete`, async () => { + process.env[ENV] = 'delete'; + const d = sqlite(tempDb()); + await d.connect(); + expect(await journalMode(d)).toBe('delete'); + }); + + it('lets an explicit driver config outrank the environment', async () => { + process.env[ENV] = 'delete'; + const d = sqlite(tempDb(), { sqliteJournalMode: 'wal' }); + await d.connect(); + expect(await journalMode(d)).toBe('wal'); + }); + + it('reports an unrecognised mode and stays on WAL rather than silently opting out', async () => { + process.env[ENV] = 'wal2'; + const d = sqlite(tempDb()); + const warnings = captureWarnings(d); + + await d.connect(); + + expect(await journalMode(d)).toBe('wal'); + expect(warnings.join('\n')).toMatch(/Ignoring SQLite journal mode 'wal2'/); + }); + }); + + describe('failure modes that are not loud on their own', () => { + it('reports a silent refusal instead of assuming the mode took', async () => { + const file = tempDb(); + const d = sqlite(file); + const warnings = captureWarnings(d); + // What a filesystem that cannot host WAL answers: the old mode, no error. + stubRaw(d, (sql, pass) => + /journal_mode\s*=/i.test(sql) ? Promise.resolve([{ journal_mode: 'delete' }]) : pass(), + ); + + await expect(d.connect()).resolves.toBeUndefined(); + + expect(warnings.join('\n')).toMatch(/kept journal_mode='delete'/); + expect(warnings.join('\n')).toMatch(new RegExp(`${ENV}=delete`)); + }); + + it('reverts to `delete` when WAL is accepted but cannot be read through', async () => { + const file = tempDb(); + const d = sqlite(file); + const warnings = captureWarnings(d); + // The NFS/SMB shape: the mode change lands, then the first read through + // the WAL's shared-memory index fails. + let failReads = true; + stubRaw(d, (sql, pass) => { + if (failReads && /FROM sqlite_master/i.test(sql)) { + failReads = false; // one failure is enough to trip the revert + return Promise.reject( + Object.assign(new Error('disk I/O error'), { code: 'SQLITE_IOERR_SHMOPEN' }), + ); + } + return pass(); + }); + + await expect(d.connect()).resolves.toBeUndefined(); + + expect(await journalMode(d)).toBe('delete'); + expect(warnings.join('\n')).toMatch(/accepted WAL .* but cannot read through it/); + expect(warnings.join('\n')).toMatch(/reverted to journal_mode=delete/); + }); + + it('recovers a database an earlier boot left stranded in WAL', async () => { + const file = tempDb(); + const stranded = sqlite(file); + await stranded.connect(); // the file is WAL from here on + await stranded.disconnect(); + + // Second boot finds it already in the target mode — and must still prove + // the mode works, or the revert that fixes it never gets another attempt. + const d = sqlite(file); + const warnings = captureWarnings(d); + let failReads = true; + stubRaw(d, (sql, pass) => { + if (failReads && /FROM sqlite_master/i.test(sql)) { + failReads = false; + return Promise.reject( + Object.assign(new Error('disk I/O error'), { code: 'SQLITE_IOERR_SHMOPEN' }), + ); + } + return pass(); + }); + + await d.connect(); + + expect(await journalMode(d)).toBe('delete'); + expect(warnings.join('\n')).toMatch(/reverted to journal_mode=delete/); + }); + + it('treats a busy read as contention, not as an unusable WAL', async () => { + const file = tempDb(); + const d = sqlite(file); + const warnings = captureWarnings(d); + // What another connection in EXCLUSIVE locking mode produces — transient, + // and no reason to take a working database out of WAL. + let failReads = true; + stubRaw(d, (sql, pass) => { + if (failReads && /FROM sqlite_master/i.test(sql)) { + failReads = false; + return Promise.reject( + Object.assign(new Error('database is locked'), { code: 'SQLITE_BUSY' }), + ); + } + return pass(); + }); + + await d.connect(); + + expect(await journalMode(d)).toBe('wal'); + expect(warnings).toEqual([]); + }); + + it('does not chase the journal mode when the connection itself cannot come up', async () => { + const d = sqlite(tempDb()); + const warnings = captureWarnings(d); + // A native better-sqlite3 that cannot load fails on the FIRST pragma; + // asking it for more would only repeat the same warning (#2229). + const attempted: string[] = []; + stubRaw(d, (sql) => { + attempted.push(sql); + return Promise.reject( + Object.assign(new Error('NODE_MODULE_VERSION 127'), { code: 'ERR_DLOPEN_FAILED' }), + ); + }); + + await expect(d.connect()).resolves.toBeUndefined(); + + expect(attempted).toEqual(['PRAGMA auto_vacuum = INCREMENTAL']); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toMatch(/native better-sqlite3 unavailable/); + }); + }); +}); diff --git a/packages/plugins/driver-sql/src/sql-driver.ts b/packages/plugins/driver-sql/src/sql-driver.ts index 71fe97a75e..70f3e5df60 100644 --- a/packages/plugins/driver-sql/src/sql-driver.ts +++ b/packages/plugins/driver-sql/src/sql-driver.ts @@ -427,6 +427,13 @@ export interface IntrospectedSchema { */ export type ReadPresentationKind = 'datetime' | 'date' | 'time' | 'boolean' | 'number'; +/** + * Journal modes the driver knows how to ask a file-backed SQLite database for. + * See {@link SqlDriver.applySqliteJournalMode} for why `'wal'` is the default + * and what `'delete'` is for. + */ +export type SqliteJournalMode = 'wal' | 'delete'; + export type SqlDriverConfig = Knex.Config & { schemaMode?: SchemaMode; /** @@ -437,6 +444,18 @@ export type SqlDriverConfig = Knex.Config & { * destructive DDL, and is force-disabled when `NODE_ENV==='production'`. */ autoMigrate?: 'off' | 'safe'; + /** + * Journal mode for a **file-backed** SQLite database (#3941). Defaults to + * `'wal'`, which is what lets a dev server and a CLI command share one file + * without serializing against each other. `'delete'` restores SQLite's + * built-in rollback journal — the escape hatch for a database on a network + * filesystem, where WAL cannot work. Ignored for `:memory:` and for + * non-SQLite dialects; overridable per deployment with + * `OS_DATABASE_SQLITE_JOURNAL_MODE`. + * + * @see {@link SqlDriver.applySqliteJournalMode} + */ + sqliteJournalMode?: SqliteJournalMode; }; // ── SQL Driver ─────────────────────────────────────────────────────────────── @@ -757,6 +776,13 @@ export class SqlDriver implements IDataDriver { */ protected readonly autoMigrate: 'off' | 'safe'; + /** + * The journal mode the host declared, if any (#3941). Kept unresolved so an + * absent declaration can still defer to the environment at connect time — + * see {@link resolveSqliteJournalMode}. + */ + protected readonly declaredJournalMode?: SqliteJournalMode; + /** * Metadata field defs for every table this driver manages, captured during * `initObjects` (tableName → fields). The source of truth that @@ -777,11 +803,12 @@ export class SqlDriver implements IDataDriver { protected deferredSchemaObjects = new Map }>(); constructor(config: SqlDriverConfig) { - // `schemaMode` / `autoMigrate` are ObjectStack concerns, not Knex options — - // strip them before handing the config to Knex. - const { schemaMode, autoMigrate, ...knexConfig } = config; + // `schemaMode` / `autoMigrate` / `sqliteJournalMode` are ObjectStack + // concerns, not Knex options — strip them before handing the config to Knex. + const { schemaMode, autoMigrate, sqliteJournalMode, ...knexConfig } = config; this.schemaMode = schemaMode ?? 'managed'; this.autoMigrate = autoMigrate ?? 'off'; + this.declaredJournalMode = sqliteJournalMode; this.config = knexConfig; this.knex = knex(SqlDriver.withConnectBound(knexConfig)); this.installQueryTiming(); @@ -995,50 +1022,281 @@ export class SqlDriver implements IDataDriver { // better-sqlite3 to open the file (e.g. loadMetaFromDb on startup). await this.ensureDatabaseExists(); - // SQLite space hygiene (ADR-0057). With the default `auto_vacuum=NONE`, - // freed pages are never returned to the OS — a database that briefly grew - // (e.g. high-frequency telemetry before retention sweeps run) stays at its - // high-water mark forever. INCREMENTAL lets a later `PRAGMA incremental_vacuum` - // (run by the lifecycle Reaper, or manually) reclaim that space without a - // full blocking VACUUM. NOTE: auto_vacuum only changes layout on a *fresh* - // database or after a one-time full VACUUM, so this benefits new dev DBs; - // existing files need a single `VACUUM` to adopt it. Harmless / no-op on - // :memory: and on already-incremental databases. if (this.isSqlite) { + // Both pragmas below are persistent properties of the FILE, so one + // connection setting them is enough for every process that follows. + // A `false` means the very first statement on this connection failed — + // in practice a native addon that cannot load — so asking it for another + // pragma would only repeat the same warning. + if (await this.applyAutoVacuumIncremental()) { + await this.applySqliteJournalMode(); + } + } + } + + /** + * SQLite space hygiene (ADR-0057). With the default `auto_vacuum=NONE`, + * freed pages are never returned to the OS — a database that briefly grew + * (e.g. high-frequency telemetry before retention sweeps run) stays at its + * high-water mark forever. INCREMENTAL lets a later `PRAGMA incremental_vacuum` + * (run by the lifecycle Reaper, or manually) reclaim that space without a + * full blocking VACUUM. NOTE: auto_vacuum only changes layout on a *fresh* + * database or after a one-time full VACUUM, so this benefits new dev DBs; + * existing files need a single `VACUUM` to adopt it. Harmless / no-op on + * :memory: and on already-incremental databases. Unaffected by the journal + * mode: an INCREMENTAL database keeps reclaiming under WAL. + * + * @returns whether the PRAGMA went through. This is the first statement any + * connection runs, so `false` says the connection itself is not answering — + * not merely that space hygiene is unavailable. + */ + private async applyAutoVacuumIncremental(): Promise { + try { + await this.knex.raw('PRAGMA auto_vacuum = INCREMENTAL'); + return true; + } catch (e) { + // A native better-sqlite3 load failure surfaces HERE first — this PRAGMA + // is the first query that forces the lazy `.node` addon to load. Two + // real-world variants, both handled by `resolveSqliteDriver`'s probe, + // which catches the failure and steps down to wasm SQLite with a clean + // one-line notice (#2229): + // • ABI mismatch — `ERR_DLOPEN_FAILED` / "…NODE_MODULE_VERSION 127…" + // (a stale prebuilt binary after a Node upgrade) + // • not built — "Could not locate the bindings file" (native addon + // never compiled, e.g. a fresh clone / blocked build) + // Dumping the full multi-line stack for either looks like a fatal crash + // to the reader (it isn't), so log a concise, actionable one-liner and + // let the step-down message that follows explain the outcome. Any OTHER + // PRAGMA failure keeps the full warning (with stack) as before. + const code = (e as { code?: string } | null | undefined)?.code; + const msg = e instanceof Error ? e.message : String(e); + const isNativeLoadFailure = + code === 'ERR_DLOPEN_FAILED' || + code === 'MODULE_NOT_FOUND' || + code === 'ERR_MODULE_NOT_FOUND' || + /NODE_MODULE_VERSION|could not locate the bindings|was compiled against a different/i.test(msg); + if (isNativeLoadFailure) { + this.logger.warn( + 'native better-sqlite3 unavailable (ABI mismatch or not built) — will step down to wasm SQLite; run `pnpm rebuild better-sqlite3` for native speed', + ); + } else { + this.logger.warn('Failed to set PRAGMA auto_vacuum=INCREMENTAL', e); + } + return false; + } + } + + /** + * Env override for the SQLite journal mode — the ops-side escape hatch for a + * deployment whose filesystem cannot host WAL. + */ + private static readonly SQLITE_JOURNAL_MODE_ENV = 'OS_DATABASE_SQLITE_JOURNAL_MODE'; + + /** + * Which journal mode to ask this file-backed database for. + * + * An explicit `sqliteJournalMode` in the driver config outranks the + * environment: it is a decision the host made about this datasource, while + * the env var is a blanket per-deployment setting. Anything unrecognised is + * reported and ignored rather than silently treated as an opt-out — a typo + * must not quietly leave a database serialized. + */ + protected resolveSqliteJournalMode(): SqliteJournalMode { + const fromEnv = (process.env[SqlDriver.SQLITE_JOURNAL_MODE_ENV] ?? '').trim().toLowerCase(); + const requested = this.declaredJournalMode ?? (fromEnv || undefined); + if (requested === undefined) return 'wal'; + if (requested === 'wal' || requested === 'delete') return requested; + this.logger.warn( + `Ignoring SQLite journal mode '${requested}' (expected 'wal' or 'delete') — using 'wal'`, + ); + return 'wal'; + } + + /** + * Whether this transport can hold its database in WAL mode. + * + * True for a real SQLite file. Overridden to `false` by a transport whose + * "file" is a byte image it serializes itself (`SqliteWasmDriver`): no other + * process reads its live database, so a persistent mode change made for + * cross-process concurrency buys it nothing. + */ + protected get supportsWalJournal(): boolean { + return true; + } + + /** + * Put a file-backed SQLite database in WAL mode (#3941). + * + * ObjectStack's normal shape is **several processes on one file**: a dev + * server, `os migrate`, `os meta resync`, a test run. SQLite's built-in + * default — `journal_mode = delete`, a rollback journal — is the worst mode + * for that, in two measured ways: + * + * - **A writer must wait for every reader.** Committing a rollback journal + * takes an EXCLUSIVE lock, which cannot coexist with a reader, so an + * `os migrate` write serializes behind whatever the live server is + * reading (`SQLITE_BUSY`). Under WAL neither direction blocks: readers + * see the last committed snapshot while a writer appends. + * - **An attached connection leaves no trace.** Rollback-journal locks + * exist only for the duration of a transaction, so an idle `os serve` is + * invisible to SQL — which is what defeated the first cut of the + * `os migrate` occupancy check (#3924) and forced it onto file-descriptor + * inspection (#3940). Under WAL the SQL probe + * (`locking_mode = EXCLUSIVE` + `BEGIN IMMEDIATE`) reports `SQLITE_BUSY` + * against an idle attached connection, so it becomes authoritative + * instead of a fallback. + * + * Journal mode is a **persistent property of the file**, not of a connection: + * setting it once is enough, and it stays set after every process detaches. + * That is also why the opt-out has to *apply* `delete` rather than skip — + * skipping would leave an already-converted database in WAL forever. Only + * `journal_mode` changes here; `synchronous` is untouched, so durability is + * exactly what it was before. + * + * Two failure modes, both handled because neither is loud on its own: + * + * 1. **A refusal is an answer, not an error.** `PRAGMA journal_mode = X` + * replies with the mode actually in force — `:memory:` answers `memory`, + * and a filesystem that cannot host WAL answers `delete` — without + * raising anything. So the reply is always read back, never assumed. + * 2. **Accepted, then unusable.** WAL needs a shared-memory index beside + * the database, which network filesystems (NFS/SMB) do not provide; the + * mode change can succeed and the first read *through* the WAL then + * fail. The mode is persisted by that point, so leaving it would strand + * the database — hence the read-back probe and the revert to `delete`. + * + * Never throws and never blocks the boot: a database that stays on a rollback + * journal is slower under concurrency, not broken, and refusing to start over + * a pragma would be worse than the problem this solves. + */ + protected async applySqliteJournalMode(): Promise { + if (!this.supportsWalJournal) return; + + // No file means nobody to share it with: `:memory:` (and every other + // `:`-prefixed pseudo-filename) is private to this process. + const filename = this.sqliteFilename(); + if (!filename) return; + + const target = this.resolveSqliteJournalMode(); + + let current: string | null; + try { + current = SqlDriver.journalModeOf(await this.knex.raw('PRAGMA journal_mode')); + } catch (e) { + this.logger.warn(`Failed to read SQLite journal_mode on ${filename}`, e); + return; + } + + // `memory` / `off` is a deliberate no-journal transport, not a rollback + // journal worth migrating — leave whatever the host chose in place. + if (current === null || current === 'memory' || current === 'off') return; + + if (current === target) { + // Nothing to change — but when the mode is already WAL, still prove it is + // usable. A database an earlier boot left in WAL on a filesystem that + // cannot serve it (failure mode 2) would otherwise fail every query with + // nothing pointing at the cause, and the revert that would fix it only + // ever gets one attempt. + if (target === 'wal') await this.verifyWalReadable(filename); + return; + } + + let applied: string | null; + try { + applied = SqlDriver.journalModeOf( + await this.knex.raw(`PRAGMA journal_mode = ${target.toUpperCase()}`), + ); + } catch (e) { + // SQLITE_BUSY is the benign case: a mode change needs the file to itself, + // and another process has it open. Whoever wins the race sets the mode + // and everyone else reads it back on their next boot. + this.logger.warn( + `Failed to set SQLite journal_mode=${target} on ${filename} (it stays '${current}')`, + e, + ); + return; + } + + if (applied !== target) { + this.logger.warn( + `SQLite kept journal_mode='${applied ?? 'unknown'}' on ${filename} instead of '${target}' — ` + + (target === 'wal' + ? `WAL needs a local filesystem (it does not work on NFS/SMB), so this database stays serialized between processes. ` + + `Set ${SqlDriver.SQLITE_JOURNAL_MODE_ENV}=delete to stop asking.` + : `close every other connection and retry, or run \`sqlite3 ${filename} "PRAGMA journal_mode=delete"\` with nothing attached.`), + ); + return; + } + + if (target === 'wal') await this.verifyWalReadable(filename); + } + + /** + * Prove a WAL database can actually be read through, and undo the mode if + * not — see failure mode 2 in {@link applySqliteJournalMode}. Runs on every + * connect to a WAL database, not only right after a switch, so a database + * some earlier boot stranded still gets recovered. + * + * Any read on a WAL database opens the shared-memory index, which is the part + * a network filesystem cannot provide; `sqlite_master` is the cheapest such + * read and needs no schema to exist. + */ + private async verifyWalReadable(filename: string): Promise { + try { + await this.knex.raw('SELECT count(*) FROM sqlite_master'); + return; + } catch (e) { + // Busy is not unusable. Another connection in EXCLUSIVE locking mode — the + // `os migrate` occupancy probe takes exactly that, briefly — makes this + // read fail for a reason that passes on its own, and changing the journal + // mode underneath it would be both wrong and impossible. + if (SqlDriver.isSqliteBusyError(e)) return; + + const detail = e instanceof Error ? e.message : String(e); + let reverted: string | null = null; try { - await this.knex.raw('PRAGMA auto_vacuum = INCREMENTAL'); - } catch (e) { - // A native better-sqlite3 load failure surfaces HERE first — this PRAGMA - // is the first query that forces the lazy `.node` addon to load. Two - // real-world variants, both handled by `resolveSqliteDriver`'s probe, - // which catches the failure and steps down to wasm SQLite with a clean - // one-line notice (#2229): - // • ABI mismatch — `ERR_DLOPEN_FAILED` / "…NODE_MODULE_VERSION 127…" - // (a stale prebuilt binary after a Node upgrade) - // • not built — "Could not locate the bindings file" (native addon - // never compiled, e.g. a fresh clone / blocked build) - // Dumping the full multi-line stack for either looks like a fatal crash - // to the reader (it isn't), so log a concise, actionable one-liner and - // let the step-down message that follows explain the outcome. Any OTHER - // PRAGMA failure keeps the full warning (with stack) as before. - const code = (e as { code?: string } | null | undefined)?.code; - const msg = e instanceof Error ? e.message : String(e); - const isNativeLoadFailure = - code === 'ERR_DLOPEN_FAILED' || - code === 'MODULE_NOT_FOUND' || - code === 'ERR_MODULE_NOT_FOUND' || - /NODE_MODULE_VERSION|could not locate the bindings|was compiled against a different/i.test(msg); - if (isNativeLoadFailure) { - this.logger.warn( - 'native better-sqlite3 unavailable (ABI mismatch or not built) — will step down to wasm SQLite; run `pnpm rebuild better-sqlite3` for native speed', - ); - } else { - this.logger.warn('Failed to set PRAGMA auto_vacuum=INCREMENTAL', e); - } + reverted = SqlDriver.journalModeOf(await this.knex.raw('PRAGMA journal_mode = DELETE')); + } catch { + // Leaving `reverted` null — reported as "did not take" below, which is + // the honest outcome either way. } + this.logger.warn( + `SQLite accepted WAL on ${filename} but cannot read through it (${detail}) — ` + + (reverted === 'delete' + ? `reverted to journal_mode=delete. Set ${SqlDriver.SQLITE_JOURNAL_MODE_ENV}=delete to skip this on every boot.` + : `and the revert did not take (journal_mode='${reverted ?? 'unknown'}'). Run ` + + `\`sqlite3 ${filename} "PRAGMA journal_mode=delete"\` with nothing attached, then set ` + + `${SqlDriver.SQLITE_JOURNAL_MODE_ENV}=delete.`), + ); } } + /** + * Whether an error is SQLite reporting contention rather than a broken + * database. Mirrors `isBusyError` in the CLI's occupancy probe, which asks the + * same question of the same engine from the other side of the package graph. + */ + private static isSqliteBusyError(e: unknown): boolean { + const code = (e as { code?: string } | null | undefined)?.code ?? ''; + const message = e instanceof Error ? e.message : String(e ?? ''); + return code.startsWith('SQLITE_BUSY') || /database is locked|SQLITE_BUSY/i.test(message); + } + + /** + * The `journal_mode` a PRAGMA reply carries, lower-cased. + * + * Both SQLite dialects answer a PRAGMA with rows (`[{ journal_mode: 'wal' }]`) + * — a reply shaped any other way yields `null`, which every caller treats as + * "leave the database alone" rather than guessing. + */ + private static journalModeOf(result: unknown): string | null { + const rows: unknown = Array.isArray(result) ? result : (result as { rows?: unknown })?.rows; + const row = Array.isArray(rows) ? rows[0] : undefined; + const value = + row && typeof row === 'object' ? (row as { journal_mode?: unknown }).journal_mode : row; + return typeof value === 'string' ? value.toLowerCase() : null; + } + async checkHealth(): Promise { try { await this.knex.raw('SELECT 1'); @@ -5468,13 +5726,26 @@ export class SqlDriver implements IDataDriver { // ── Database helpers ──────────────────────────────────────────────────────── + /** + * The on-disk path backing this SQLite datasource, or `null` when there is + * none: a non-SQLite dialect, `:memory:` (and any other `:`-prefixed + * pseudo-filename), or a function-valued `connection` where the host builds + * each connection itself and the target is not ours to read. + */ + protected sqliteFilename(): string | null { + if (!this.isSqlite) return null; + const conn = (this.config as any).connection; + const filename = typeof conn === 'string' ? conn : conn?.filename; + if (typeof filename !== 'string' || filename === '') return null; + return filename.startsWith(':') ? null : filename; + } + protected async ensureDatabaseExists() { // SQLite auto-creates database files but NOT parent directories. // Ensure the directory exists so better-sqlite3 can create the file. if (this.isSqlite) { - const conn = (this.config as any).connection; - const filename = typeof conn === 'string' ? conn : conn?.filename; - if (filename && filename !== ':memory:' && !filename.startsWith(':')) { + const filename = this.sqliteFilename(); + if (filename) { const { dirname } = await import('node:path'); const { mkdir } = await import('node:fs/promises'); const dir = dirname(filename); diff --git a/packages/plugins/driver-sqlite-wasm/src/sqlite-wasm-driver-journal-mode.test.ts b/packages/plugins/driver-sqlite-wasm/src/sqlite-wasm-driver-journal-mode.test.ts new file mode 100644 index 0000000000..52cf6062b4 --- /dev/null +++ b/packages/plugins/driver-sqlite-wasm/src/sqlite-wasm-driver-journal-mode.test.ts @@ -0,0 +1,93 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #3941: the base SqlDriver puts a file-backed SQLite database in WAL mode so +// several processes can share one file. This transport must stay out of it — +// its persistence is a byte-image export, and sql.js *accepts* WAL (its VFS is +// memory-backed), so nothing refuses on our behalf. + +import { describe, it, expect, afterEach } from 'vitest'; +import { mkdtempSync, rmSync, writeFileSync, existsSync, readdirSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { SqliteWasmDriver } from '../src/index.js'; + +const dirs: string[] = []; +const drivers: SqliteWasmDriver[] = []; + +function newDriver(): { driver: SqliteWasmDriver; dir: string; file: string } { + const dir = mkdtempSync(join(tmpdir(), 'wasm-journal-')); + const file = join(dir, 'test.db'); + const driver = new SqliteWasmDriver({ filename: file, persist: 'on-write' }); + dirs.push(dir); + drivers.push(driver); + return { driver, dir, file }; +} + +afterEach(async () => { + await Promise.all(drivers.splice(0).map((d) => d.disconnect().catch(() => {}))); + for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true }); +}); + +describe('SqliteWasmDriver — stays out of WAL (#3941)', () => { + it('leaves the journal mode alone on a file-backed database', async () => { + const { driver, dir } = newDriver(); + await driver.connect(); + + const rows: any = await (driver as any).knex.raw('PRAGMA journal_mode'); + expect(String(rows[0].journal_mode).toLowerCase()).not.toBe('wal'); + // A WAL switch here would also start writing a log this driver never reads. + expect(readdirSync(dir).filter((f) => f.endsWith('-wal'))).toEqual([]); + }); + + it('parks a write-ahead log a real SQLite left behind, and says what was lost', async () => { + const { driver, dir, file } = newDriver(); + const warnings: string[] = []; + + // What an unclean kill of the native driver leaves: an image plus a log + // holding commits only a real SQLite can checkpoint. + await driver.initObjects([{ name: 'acct', fields: { name: { type: 'string' } } }]); + await driver.disconnect(); + writeFileSync(`${file}-wal`, Buffer.alloc(4096, 1)); + + const reopened = new SqliteWasmDriver({ + filename: file, + persist: 'on-write', + logger: { warn: (m: string) => warnings.push(String(m)) } as any, + }); + drivers.push(reopened); + await reopened.connect(); + // Force the connection open (the pool is lazy) so the loader actually runs. + await (reopened as any).knex.raw('SELECT 1'); + + expect(existsSync(`${file}-wal`)).toBe(false); + expect(readdirSync(dir).some((f) => f.includes('-wal.orphaned-'))).toBe(true); + expect(warnings.join('\n')).toMatch(/write-ahead log that wasm SQLite cannot read/); + expect(warnings.join('\n')).toMatch(/wal_checkpoint\(TRUNCATE\)/); + // The image itself still opened — a parked log is not a corrupt database. + const rows: any = await (reopened as any).knex.raw( + "SELECT name FROM sqlite_master WHERE name = 'acct'", + ); + expect(rows).toHaveLength(1); + }); + + it('ignores an empty log — a checkpointed one carries nothing', async () => { + const { driver, file } = newDriver(); + const warnings: string[] = []; + await driver.initObjects([{ name: 'acct', fields: { name: { type: 'string' } } }]); + await driver.disconnect(); + writeFileSync(`${file}-wal`, Buffer.alloc(0)); + + const reopened = new SqliteWasmDriver({ + filename: file, + persist: 'on-write', + logger: { warn: (m: string) => warnings.push(String(m)) } as any, + }); + drivers.push(reopened); + await reopened.connect(); + await (reopened as any).knex.raw('SELECT 1'); + + expect(warnings).toEqual([]); + expect(existsSync(`${file}-wal`)).toBe(true); + }); +}); diff --git a/packages/plugins/driver-sqlite-wasm/src/sqlite-wasm-driver.ts b/packages/plugins/driver-sqlite-wasm/src/sqlite-wasm-driver.ts index 944961f503..837bc0a97b 100644 --- a/packages/plugins/driver-sqlite-wasm/src/sqlite-wasm-driver.ts +++ b/packages/plugins/driver-sqlite-wasm/src/sqlite-wasm-driver.ts @@ -77,6 +77,30 @@ export class SqliteWasmDriver extends SqlDriver { return true; } + /** + * Never WAL (#3941). The base driver switches a file-backed SQLite database to + * WAL so several processes can share one file. Nothing here is shared: the live + * database sits in this process's WASM heap, and what reaches disk is a byte + * image {@link flush} exports from it — another process reads that snapshot, + * never the database. So the pragma buys this transport nothing. + * + * It is also not free. Journal mode is a persistent header change in the + * operator's file, and under WAL the export path's correctness would rest on + * sql.js checkpointing the log while `export()` closes and reopens the + * database. Measured, it does — no row is lost today — which is why this is a + * declined default and not a bug report. But a transport that persists by + * serializing an image should not be one implementation detail away from + * dropping committed rows for a concurrency benefit it cannot use. + * + * Declared rather than discovered: sql.js *accepts* `journal_mode = WAL`, + * because its VFS is memory-backed, so the refusal the base class gets from + * `:memory:` never comes — and an image whose header already says WAL (one a + * native run left behind) reports `wal` here too. + */ + protected override get supportsWalJournal(): boolean { + return false; + } + private wasmConfig: SqliteWasmDriverConfig; private beforeExitHandler: (() => void) | null = null; diff --git a/packages/plugins/driver-sqlite-wasm/src/wasm-connection.ts b/packages/plugins/driver-sqlite-wasm/src/wasm-connection.ts index 826395381c..c421dc04c2 100644 --- a/packages/plugins/driver-sqlite-wasm/src/wasm-connection.ts +++ b/packages/plugins/driver-sqlite-wasm/src/wasm-connection.ts @@ -189,6 +189,8 @@ export class WasmSqliteConnection { return; } + await this.quarantineOrphanedWal(); + // Open the on-disk bytes, but guard against a corrupt image. A torn write // (process killed mid-flush before atomic writes existed) or otherwise // damaged file makes `new SQL.Database(bytes)` either throw ("file is not a @@ -251,6 +253,53 @@ export class WasmSqliteConnection { } } + /** + * Move a write-ahead log left behind by a *real* SQLite aside (#3941). + * + * The native driver keeps file-backed databases in WAL mode, and a clean close + * checkpoints the log away — so a non-empty `-wal` here means the last + * process died without one. That log is a problem in both directions, and + * neither is something wasm SQLite can fix: it cannot read the log (we load + * only the main image, so any transaction still in there is invisible), and it + * must not leave it in place either — the next {@link flush} rewrites the + * image, and a real SQLite opening a fresh image beside a stale log would + * replay frames that no longer belong to it. + * + * So rename it, which loses nothing recoverable (the bytes are preserved for a + * real `sqlite3` to recover from) and disarms the mismatch. Best-effort: this + * is a dev-only step-down path and must never prevent a boot. + */ + private async quarantineOrphanedWal(): Promise { + if (!this.fs) return; + const wal = `${this.filename}-wal`; + let size: number; + try { + size = (await this.fs.stat(wal)).size; + } catch { + return; // no sidecar (the normal case) or an unreadable one + } + if (size <= 0) return; // checkpointed-and-truncated: nothing in it + + const parked = `${wal}.orphaned-${Date.now()}`; + try { + await this.fs.rename(wal, parked); + this.logger.warn( + `[driver-sqlite-wasm] ${wal} holds ${size} bytes of write-ahead log that wasm SQLite ` + + `cannot read — this database was last used in WAL mode and closed uncleanly. Parked it ` + + `at ${parked} and loaded the main image without it, so anything committed only to the ` + + `log is NOT in this session. To recover it, rebuild better-sqlite3 (or use \`sqlite3\`), ` + + `restore the log next to the database, and run \`PRAGMA wal_checkpoint(TRUNCATE)\`.`, + ); + } catch (renameErr) { + this.logger.warn( + `[driver-sqlite-wasm] ${wal} holds ${size} bytes of write-ahead log that wasm SQLite ` + + `cannot read, and it could not be moved aside (${String(renameErr)}). Data committed ` + + `only to the log is missing from this session; checkpoint it with a real sqlite3 before ` + + `writing further.`, + ); + } + } + /** * Update transaction state from a transaction-control statement and, when a * transaction has just fully closed, run any flush that was deferred while