Repository navigation
feat(sqlite): use WAL with synchronous NORMAL for file databases - #87
Merged
Merged
Conversation
The adapter used SQLite's defaults: a rollback journal with synchronous=FULL, about four fsync calls for each commit. node:sqlite is synchronous, so on a slow disk each commit stopped the event loop, including timers, HTTP handlers, heartbeats, and lease renewals. A CI run with a slow disk timed out a 16 ms test after 5 seconds. The Ruby gem already gets WAL with synchronous=NORMAL from Active Record. The switch to WAL needs a lock, and SQLite refuses it at once while another connection writes. So the adapter switches on its first access, with the same bounded retries and deadline rules as a transaction, instead of in the constructor.
The adapter now switches a file to WAL on its first access. These tests are about transaction retries and deadlines under writer contention, so their files start in WAL mode, as a file that the adapter made does. The switch under contention has its own test.
A user who wants each commit flushed to disk before it returns can pass synchronous: "full". The database still uses WAL, so a commit costs about one fsync instead of four.
The value goes into a PRAGMA statement, so the adapter accepts only the two documented levels and fails at construction, like lockRetryAttempts.
SQLite answers "memory" when an in-memory database asks for WAL. This guard keeps sqlite({ path: ":memory:" }) working after the WAL default.
The configuration guide states the durability trade-off of synchronous NORMAL and FULL, the -wal and -shm files, and the switch on first access. The parity ledger records that both runtimes now use WAL with synchronous NORMAL for a file database.
|
Docs7 for cardmagic/solid-objects-js
Commit |
[High impact] The PR appears safe to merge; both previous findings are fixed and no new actionable issues were found. SummaryFile-backed SQLite databases now use WAL with
Reviews (2) · Last reviewed commit: "style: format the SQLite validation test" · Reviewed by Greptile |
Review found that closing one adapter does not write the WAL into the database file while another connection still has it open, and that a copy of three live files is not a consistent snapshot. The guide now points to VACUUM INTO or the SQLite backup command for a database in use. The validation test also casts its bad value without unknown, as the repository rule asks.
Owner
Author
|
@greptileai review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
synchronous = NORMAL, the settings that the Ruby gem gets from Active Record 7.1 and later. Before, the adapter used SQLite's defaults: a rollback journal withsynchronous = FULL, about fourfsynccalls for each commit.node:sqliteis synchronous, so each slowfsyncstopped the whole event loop: timers, HTTP handlers, heartbeats, and lease renewals. In CI run 39180349972, a slow disk madecompare-agent-sessiontime out a test that takes 16 ms; only the test files with a file-backed database got 3 to 6 times slower. In a Linux container limited to 30 write IOPS, the same test timed out; with WAL and NORMAL at 20 write IOPS it took 398 ms.sqlite({ path, synchronous: "full" })flushes each commit before it returns (about onefsyncwith WAL). Any other value throws aTypeError.BEGIN IMMEDIATE, not in the constructor. One shared helper now does the busy retry for both.Durability
"normal", a power loss or an operating system crash can lose the last commits."full"keeps the old guarantee for each commit.<path>-waland<path>-shmnext to the database.docs/configuration.mdsays how to copy the files.Tests, written first
deleteand 2 before).synchronous: "full"gives WAL and FULL.synchronousvalue other than"normal"or"full"throws.sqlite.test.tsanddatabase-deadline.test.tsnow start from a WAL file, so they still test transaction retries and deadlines, not the switch.Checks
pnpm run check,pnpm test(709 passed, 32 skipped), andprettier --check .pass.CHANGELOG.mdhas the entry under "Unreleased". This changes a default, so it needs a release decision.