From fee154172e64df57c35436fffe319a2982198362 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 07:55:02 +0000 Subject: [PATCH 1/2] feat(driver-sql): run file-backed SQLite in WAL journal mode (#3941) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `connect()` set `auto_vacuum` and never touched `journal_mode`, so every ObjectStack SQLite database ran SQLite's built-in rollback journal. That is the worst mode for the shape this platform has — several processes on one file (a dev server, `os migrate`, `os meta resync`, a test run). Measured on one file: a writer must wait for every open reader (committing needs an exclusive lock), and an idle attached connection leaves nothing any query can see, which is why the occupancy check had to inspect file descriptors to notice a live server at all (#3940). Under WAL neither holds: readers and writers no longer block each other, and `locking_mode = EXCLUSIVE` + `BEGIN IMMEDIATE` reports busy against an idle connection, so the SQL probe becomes authoritative instead of blind. Neither failure mode is loud, so neither is assumed: - `PRAGMA journal_mode = X` answers with the mode actually in force rather than raising on refusal (`:memory:` answers `memory`; a filesystem that cannot host WAL answers `delete`), so the reply is always read back. - A filesystem can accept WAL and then fail the first read *through* it, and the mode is already persisted by then. So a WAL database is proven with a read on every connect and reverted to `delete` when that fails — tolerating SQLITE_BUSY, which is contention rather than an unusable WAL. Opt out per deployment with `OS_DATABASE_SQLITE_JOURNAL_MODE=delete`, or per datasource with `sqliteJournalMode: 'delete'` (which outranks the env var). Both *apply* `delete`, so a database that already adopted WAL converts back; skipping would have stranded it. `synchronous` is untouched, so durability is unchanged, and `auto_vacuum = INCREMENTAL` keeps reclaiming under WAL (ADR-0057). Nothing here can fail a boot. The WASM SQLite driver declines WAL: no other process reads its live database, so a persistent header change buys it nothing — and sql.js accepts the pragma rather than refusing, so the refusal the base class relies on for `:memory:` never comes. It also parks a `-wal` an unclean native exit left behind instead of loading the image beside it: wasm SQLite cannot read that log, and leaving it next to a rewritten image would let a later real SQLite replay frames that no longer belong to it. `os db clean` counts `-wal`/`-shm` when measuring what a VACUUM reclaimed, so bytes sitting in the log no longer read as a reclaim of zero. Closes #3941 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SiCoKVAYsXvD7cfUZ8iDQ4 --- .changeset/sqlite-wal-journal-mode.md | 65 ++++ content/docs/data-modeling/drivers.mdx | 58 +++ content/docs/deployment/backup-restore.mdx | 11 + content/docs/deployment/cli.mdx | 17 +- .../docs/deployment/environment-variables.mdx | 1 + content/docs/releases/v17.mdx | 11 + packages/cli/src/commands/db/clean.ts | 17 +- .../cli/src/utils/sqlite-occupancy.test.ts | 9 +- packages/cli/src/utils/sqlite-occupancy.ts | 39 +- packages/plugins/driver-sql/src/index.ts | 1 + .../sql-driver-sqlite-journal-mode.test.ts | 299 +++++++++++++++ packages/plugins/driver-sql/src/sql-driver.ts | 359 +++++++++++++++--- .../sqlite-wasm-driver-journal-mode.test.ts | 93 +++++ .../src/sqlite-wasm-driver.ts | 24 ++ .../driver-sqlite-wasm/src/wasm-connection.ts | 49 +++ 15 files changed, 979 insertions(+), 74 deletions(-) create mode 100644 .changeset/sqlite-wal-journal-mode.md create mode 100644 packages/plugins/driver-sql/src/sql-driver-sqlite-journal-mode.test.ts create mode 100644 packages/plugins/driver-sqlite-wasm/src/sqlite-wasm-driver-journal-mode.test.ts 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..3a26f513fb 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. 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 1b6355f563..2ec9dc25a8 100644 --- a/content/docs/releases/v17.mdx +++ b/content/docs/releases/v17.mdx @@ -922,6 +922,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 From 137de08940962323626f4ec6bea6fddd1857c206 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 08:10:37 +0000 Subject: [PATCH 2/2] docs(backup-restore): the SQLite restore step names the WAL sidecars The restore drill said "copy the SQLite file", which under WAL (#3941) is the exact hazard the warning above it describes: a restored database sitting beside a stale -wal makes a later open replay frames that do not belong to it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SiCoKVAYsXvD7cfUZ8iDQ4 --- content/docs/deployment/backup-restore.mdx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/content/docs/deployment/backup-restore.mdx b/content/docs/deployment/backup-restore.mdx index 3a26f513fb..ec2254194b 100644 --- a/content/docs/deployment/backup-restore.mdx +++ b/content/docs/deployment/backup-restore.mdx @@ -98,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