Skip to content

fix(turso): checkpoint the WAL before closing a database - #440

Open
jkemp814 wants to merge 3 commits into
tickernelz:mainfrom
jkemp814:fix/wal-checkpoint-before-close
Open

jkemp814 wants to merge 3 commits into
tickernelz:mainfrom
jkemp814:fix/wal-checkpoint-before-close

Conversation

@jkemp814

@jkemp814 jkemp814 commented Oct 26, 2026 •

Copy link
Copy Markdown

Summary

Checkpoints the WAL into the main database file before closing a pooled connection, so a process that dies ungracefully cannot leave a -wal that disagrees with turso's -tshm index. Relates to #435; complements #441 (prevention on the way out, recovery on the way in).

Test plan

  • Exercised the change locally (or N/A for docs/meta-only) — patched build loaded into a running server, service stopped, sidecars inspected with no process alive (metadata.db 74,192 -> 0, user-prompts.db 45,352 -> 0)
  • Noted any follow-up or known limitations — see Risks below: the exit path cannot checkpoint, and a genuine SIGKILL mid-write is still out of reach

Checklist

  • Branch is based on main (do not open PRs from commits already on main)
  • bun test passes (or N/A) — 16 pass across the four turso/shutdown files; full suite down to 2 pre-existing config failures
  • bun run typecheck passes (or N/A)
  • bun run check passes (format + lint + typecheck; or N/A)
  • Docs / README updated when behavior or contribution process changes — N/A: no user-facing config or documented behaviour changes

Checkpoints the WAL into the main database file before closing a pooled connection, so a process
that dies ungracefully cannot leave a -wal whose state disagrees with turso's -tshm index.

Why

Turso keeps its multiprocess WAL index in <db>-tshm. When a host kills the process mid-WAL —
SIGKILL after a shutdown timeout, an OOM kill, a power-off — the -wal can end up truncated or
partially written while the index still claims frames exist. Every later open then fails with:

I/O error: short read on WAL frame at offset N: expected 4096 bytes, got 0

and because ensureTursoReady() gates memory tools, chat capture and the web server behind it, one
torn sidecar pair takes the whole memory system down (issue #435, which documents the same classes
of failure). The main .db files stay intact, so no data is lost — but nothing in the plugin
closes databases in a way that flushes the WAL first.

OpenCode makes this reachable in normal use: its service restart sends SIGTERM and, after a fixed
5 s poll, SIGKILLs the shared server. That server is where plugin databases live, so a restart can
land on hot WALs across every database at once. Checkpointing before close shrinks that window to
the duration of the close itself.

What changed

  • src/services/turso/turso-db.ts — TursoDb.close() runs PRAGMA wal_checkpoint(TRUNCATE) before
    database.close().
    • Bounded at 1500 ms (CHECKPOINT_TIMEOUT_MS, overridable via an optional constructor argument
      used only by tests) so a contended checkpoint cannot eat the whole shutdown budget.
    • Never throws: a busy or timed-out checkpoint is logged (Turso WAL checkpoint before close failed) and the close proceeds.
    • Logs the pragma result row (busy / log / checkpointed) on success, so a partially
      checkpointed WAL is visible instead of silent.
  • src/services/turso/connection-manager.ts — closeAll() closes pooled databases in parallel
    instead of sequentially, so N × the per-DB bound cannot exceed the host's shutdown window.
  • tests/turso-close-checkpoint.test.ts — checkpoint-then-close ordering; close still happens when
    the pragma rejects; late rejection after the timeout is observed (no unhandled rejection).

Verification

  • bun run format:check, bun run lint, bun run typecheck — clean.
  • bun test tests/turso-close-checkpoint.test.ts — 3 pass.
  • bun test tests/turso-close-checkpoint.test.ts tests/turso-connection-manager.test.ts tests/turso-connection-lifecycle.test.ts tests/plugin-shutdown.test.ts — 13 pass, 0 fail.
  • End-to-end, against a patched build loaded by the running OpenCode server, stopping the service
    and inspecting sidecars with no server process alive so nothing could write in between:
                         before    while stopped
metadata.db    wal=     74192  ->  0
user-prompts.db wal=    45352  ->  0
user-profiles.db wal=       0  ->  0        (already clean)
new "short read on WAL frame" errors after restart: 0

Full bun test: 860 pass / 4 skip / 6 fail; the 6 are pre-existing (2 config > CONFIG defaults,
4 plugin-v2-loader-contract because dist/v2 is not built in this checkout) and reproduce on
main with the patch stashed.

Risks / limits

  • This does not fix the host side: an actual SIGKILL mid-write still leaves whatever SQLite had on
    disk. It reduces the window rather than closing it. A plugin shutdown hook plus a SIGTERM grace
    period in the host is the real fix.
  • wal_checkpoint(TRUNCATE) needs no other readers; it can return busy on a store observed by
    another process. That is logged, not retried, and does not block shutdown.
  • On the plugin-reload path (dispose → close → reopen) this adds a small per-database latency.

Relates to #435.

A process killed mid-WAL (SIGKILL after a shutdown timeout, OOM kill,
power-off) can leave a `-wal` that disagrees with turso's `-tshm` index;
every later open then fails with "I/O error: short read on WAL frame",
which takes the whole memory system down because ensureTursoReady()
gates tools, capture and the web server behind it (see tickernelz#435).

Run `PRAGMA wal_checkpoint(TRUNCATE)` before database.close(), bounded at
1500 ms so a contended checkpoint cannot consume a host shutdown budget,
never throwing, and logging the pragma result row so a partial checkpoint
is visible. Close pooled databases in parallel so N * the bound still fits
inside the window.

Verified: format/lint/typecheck clean; 13 pass across the four turso and
plugin-shutdown test files; against a patched build loaded by a running
server, stopping the service leaves metadata.db 74192 -> 0 and
user-prompts.db 45352 -> 0 bytes of WAL with no process alive to write.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The checkpoint await breaks synchronous exit cleanup, and logging failures can prevent database handles from closing.

2 open issues
What changed in this PR

This PR adds best-effort WAL checkpointing before Turso database closure to reduce exposure to the stale-sidecar failures described in #435. It mitigates shutdown risk rather than repairing existing corruption.

Changes:

  • Adds a 1500 ms checkpoint timeout and result logging before database closure.
  • Closes pooled databases in parallel to avoid accumulating checkpoint delays.
  • Adds tests for checkpoint ordering, failure handling, and late rejection after timeout.
File Description
tests/​turso-close-checkpoint.test.ts Tests checkpoint ordering and failure paths.
src/​services/​turso/​turso-db.ts Adds bounded, best-effort checkpointing before close.
src/​services/​turso/​connection-manager.ts Changes pooled database closure to run in parallel.

Review effort: Balanced

Comment thread src/services/turso/turso-db.ts Outdated
checkpointed: result?.checkpointed,
});
} catch (error) {
log("Turso WAL checkpoint before close failed", { error: String(error) });

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 924a6c1: both diagnostic writes in checkpointBeforeClose() now go through a local safeLog() that swallows logger failures, so nothing in the helper can throw and database.close() is always reached. Regression test points OPENCODE_MEM_LOG_FILE under a regular file to force ENOTDIR from the logger and asserts the sequence is still ["PRAGMA wal_checkpoint(TRUNCATE)", "close"].

}

async close(): Promise<void> {
await this.checkpointBeforeClose();

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 924a6c1: added TursoDb.closeSync() and closeAllSync() now uses it. turso's async close() calls the native close in its first statement with nothing awaited before it, so an un-awaited call does release the handle — there is no public sync API on the async class, and the compat build is a different class that would require reopening. A test asserts Database.open === false synchronously after closeAllSync(), which also fails if turso ever puts an await ahead of the native close. The residual limitation is documented in the JSDoc: the exit path still cannot checkpoint, because there is no event loop.

@jkemp814

Copy link
Copy Markdown
Author

Measurement and the read-only/Class-B evidence are in #435: #435 (comment) — includes the before/while-stopped WAL table and why sidecar presence alone isn't a valid corruption signal.

… path

Review findings on tickernelz#440:
- checkpointBeforeClose: swallow logger failures through a local safeLog, so
  an ENOSPC/EACCES diagnostic write cannot skip database.close()
- add TursoDb.closeSync() and use it from closeAllSync(): the async close()
  starts with the awaited checkpoint, so the synchronous exit handler never
  reached the native close. closeSync() calls the native close on the same
  tick (turso's async close() runs db.close() before any await) and skips the
  checkpoint, which cannot run without an event loop
- tests: log failure still closes; closeSync skips the checkpoint and
  closeAllSync flips Database.open on the same tick
@jkemp814

Copy link
Copy Markdown
Author

Both review findings are addressed in 924a6c1:

  1. Logging failure could skip the close — the checkpoint's diagnostic writes now go through a local safeLog() that swallows ENOSPC/EACCES, so nothing in checkpointBeforeClose() can throw. New test points OPENCODE_MEM_LOG_FILE at a path under a regular file to force ENOTDIR and asserts the close still happens.
  2. closeAllSync() no longer closed anything — correct diagnosis. turso's async close() runs the native close in its first statement with nothing awaited before it, so I added closeSync() (un-awaited close, errors swallowed) and closeAllSync() uses it. A test asserts Database.open === false synchronously after closeAllSync(), which also guards that assumption if turso ever moves an await in front of the native close. The limitation is documented: the exit path still cannot checkpoint, because there is no event loop.

Verified: format/lint/typecheck clean, 16 pass across the four turso + shutdown test files, full suite down to the 2 pre-existing config failures.

Follow-up to the review on tickernelz#440: the checkpoint path already swallowed logger
failures locally, but the close-error handlers in TursoConnectionManager still
called log() directly. A logger that throws on ENOSPC/EACCES there would escape
into the process `exit` handler.

Move the helper into src/services/logger.ts as an exported safeLog() and use it
for both the checkpoint diagnostics and every close-error log in the connection
manager, so the rule is stated once instead of being re-implemented per file.
No behaviour change on the success path.
@jkemp814

Copy link
Copy Markdown
Author

One asymmetry left over from the same review. The checkpoint diagnostics were already guarded, but the close-error handlers in TursoConnectionManager still called log() directly — a logger that throws on ENOSPC/EACCES would escape into the process exit handler from there.

9ac81d3 moves the guard into src/services/logger.ts as an exported safeLog() and uses it for the checkpoint diagnostics and all three close-error logs, so the rule lives in one place rather than a local copy per file. No behaviour change on the success path.

Re-verified after the change: format:check, lint, typecheck clean; 16 pass across the four turso/shutdown files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants