feat(database): add walMode config option#58
Conversation
Expose WAL journal mode as a Database.configure() prop (default true) instead of hardcoding it, so reads via useQuery are never blocked by native background sync writes without requiring a manual PRAGMA. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cpp/database/DatabaseManager.cpp`:
- Around line 7-10: Update SQLiteConnection initialization used by
DatabaseManager::open to explicitly set the journal mode when walMode is false,
selecting the non-WAL mode instead of leaving an existing database in WAL mode.
Preserve the current WAL setup when walMode is true.
In `@cpp/database/SQLiteConnection.cpp`:
- Around line 19-21: Update the SQLite initialization logic around the walMode
setting to explicitly switch an existing database back to DELETE when walMode is
false, including the required coordination and error handling for the
journal-mode change. Preserve WAL configuration when walMode is true, and add a
regression test that reopens the same database with walMode disabled and
verifies WAL is no longer active.
- Around line 19-21: Update the WAL initialization in SQLiteConnection to
capture the result of the `PRAGMA journal_mode=WAL` execution, read the returned
journal mode, and throw when it is not `wal`; do not silently continue in DELETE
mode.
In `@cpp/tests/database/HybridSalveDatabaseTests.cpp`:
- Around line 199-206: Update the assertion in the walMode-disabled test around
HybridDatabaseHarness::run to require the exact SQLite default journal-mode
result, "delete", rather than merely checking that the result is not "wal".
In `@cpp/tests/database/SQLiteConnectionWalModeTests.cpp`:
- Around line 14-18: Update the assertion in the SQLiteConnection(walMode=false)
test to require that the journal mode is exactly "delete", rather than merely
checking it is not "wal". Keep the existing query and result extraction
unchanged.
In `@src/database/classes/ConfigureDb/types/IConfigureProps.ts`:
- Around line 10-16: Update the walMode documentation in IConfigureProps to
describe WAL as reducing read/write contention and generally allowing concurrent
access, without guaranteeing that reads are never blocked. Preserve the existing
purpose, option name, and default value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c114c3aa-657d-4a65-9590-2c041993a339
⛔ Files ignored due to path filters (1)
nitrogen/generated/shared/c++/ConfigureParams.hppis excluded by!**/generated/**
📒 Files selected for processing (10)
cpp/database/DatabaseManager.cppcpp/database/DatabaseManager.hppcpp/database/HybridSalveDatabase.cppcpp/database/SQLiteConnection.cppcpp/database/SQLiteConnection.hppcpp/tests/database/HybridSalveDatabaseTests.cppcpp/tests/database/SQLiteConnectionWalModeTests.cppsrc/database/classes/ConfigureDb/ConfigureDb.class.tssrc/database/classes/ConfigureDb/types/IConfigureProps.tssrc/specs/types/ConfigureParams.ts
journal_mode persists in the database file itself, not just the connection, so walMode:false only skipping the WAL pragma left a previously-WAL database stuck in WAL. Now always sets the pragma explicitly (WAL or DELETE) and validates what SQLite actually applied instead of assuming success. Tightens the wal-mode tests to assert the exact resulting mode, and softens the walMode doc to not overclaim reads are never blocked. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Summary
SQLite's default journal mode blocks reads while a write is in progress (exclusive lock on the whole file). The native sync engine writes to SQLite in the background — sometimes across multiple pages of a sync session — while the JS side may be reading through
useQueryat the same time. Without WAL, those background writes can make the UI's queries wait until the write commits.WAL (Write-Ahead Logging) mode fixes this: writes are appended to a separate
.walfile instead of overwriting the database in place, so concurrent reads keep seeing a consistent snapshot without blocking on an in-flight write.This was already being turned on unconditionally in
SQLiteConnection's constructor (PRAGMA journal_mode=WAL), with no way to opt out and no visibility for the consumer. This PR turns it into a first-class, documented option onDatabase.configure(), in the same spirit as an ORM config option — not something the developer has to know to enable by running a raw PRAGMA themselves (which is howexpo-sqlitecurrently does it).Changes
IConfigureProps.walMode?: boolean(TS-facing, defaulttrue) — new option onDatabase.configure({...}).ConfigureParams.walMode?: boolean— added to the Nitro spec type (src/specs/types/ConfigureParams.ts) and regenerated vianitrogen(nitrogen/generated/shared/c++/ConfigureParams.hpp).ConfigureDb.class.ts— defaultswalModetotruewhen calling the native bridge if the caller didn't pass it.SQLiteConnection(path, walMode = true)— only runsPRAGMA journal_mode=WALwhenwalModeistrue; otherwise leaves SQLite's own default journal mode untouched. Default behavior is unchanged (WAL was already always-on before).DatabaseManager::open(dbName, walMode = true)andHybridSalveDatabase::configure— thread the option from the JS-facingconfigure()call down to the connection.Test plan
npm run test:native— 73 test cases / 200 assertions (was 69/196), including two new suites:cpp/tests/database/SQLiteConnectionWalModeTests.cpp— assertsPRAGMA journal_modeiswalby default and stays non-walwhenwalMode: falseis passed directly toSQLiteConnection.cpp/tests/database/HybridSalveDatabaseTests.cppexercising the full path end-to-end through the JSI bridge (db.configure({ walMode: false })→db.execute('PRAGMA journal_mode', [])).npm test(Jest) — 84 tests passing, no regressions.npx tsc --noEmit— no type errors.Summary by CodeRabbit
New Features
walModeconfiguration setting for database connections.Tests