feat(libcalibre): enable WAL + busy_timeout PRAGMAs and add connection pools#134
Merged
phildenhoff merged 1 commit intoJul 1, 2026
Merged
Conversation
…n pools Every connection now gets sane PRAGMAs (busy_timeout 3000, WAL, synchronous NORMAL, foreign_keys ON, wal_autocheckpoint 1000), applied before anything else so the WAL switch itself waits out contention. foreign_keys=ON is future-proofing: Calibre's schema declares no FK constraints today. Adds pooled connection paths via diesel's r2d2 feature: - create_write_pool: exactly one connection (SQLite has one write lock); registers Calibre's triggers once — triggers are persistent database objects, so per-checkout DDL would serialize opens on the write lock and open windows where inserts fire no trigger. - create_read_pool: N connections with PRAGMA query_only, so writing through a read connection is a hard error, not a convention. - Per-connection state (PRAGMAs + custom SQL functions) is applied by a CustomizeConnection on every physical connection. Also: - establish_connection returns CalibreError instead of Result<_, ()>, surfacing SQLite's real open error; Library::new propagates it. - retry_on_busy helper for the locked errors busy_timeout cannot cover (read->write transaction upgrades fail immediately). - Tests: PRAGMAs + sidecar files, :memory: fallback, pool setup, query_only enforcement, and multi-connection contention (reader unblocked during write txn; busy_timeout waiting out a held lock vs failing instantly at timeout 0; retry_on_busy under contention). The desktop app's single-connection path is unchanged apart from the PRAGMAs and improved error. Fixes CDL-18 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
libcalibre Test Coverage ReportOverall coverage: 82.07% Coverage breakdown available in the artifacts. |
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
1 participant
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.
Why
libcalibreopened SQLite as a single bare connection with no PRAGMAs — no WAL, nobusy_timeout, default rollback journal. Fine for one desktop user; it blocks any concurrent-access scenario (a futurecitadel-server, or the app and a server sharing onemetadata.db). Both halves of this are independently useful to the desktop app today: WAL +busy_timeoutremove the spuriousSQLITE_BUSYrisk from the DB-write +metadata.opf-write pair per book edit.What
PRAGMAs on every connection (
apply_pragmas), in order:busy_timeout = 3000first (so the WAL switch itself waits out contention), thenjournal_mode = WAL,synchronous = NORMAL,foreign_keys = ON,wal_autocheckpoint = 1000.foreign_keys = ONis future-proofing — neither Calibre's base schema nor the custom-column DDL declares any FK constraint, verified against the fixture and a generated stress library.Pooled paths (diesel
r2d2feature):create_write_pool— exactly one connection (SQLite has one write lock; a multi-writer pool only queues on it). Registers Calibre's triggers once: triggers are persistent database objects, so per-checkout DDL would both serialize opens on the write lock and open windows where a concurrent insert fires no trigger.create_read_pool— N connections withPRAGMA query_only = ON; writing through a read connection is a hardreadonlyerror rather than a documented convention.CustomizeConnectionapplies the genuinely per-connection state (PRAGMAs + custom SQL functions) to every physical connection.Error handling:
establish_connectionreturnsCalibreErrorinstead ofResult<_, ()>, surfacing SQLite's real open error ("Unable to open the database file");Library::newpropagates it. No caller matched the oldLibraryNotInitializedcollapse.retry_on_busyhelper for the locked errorsbusy_timeoutcannot cover (read→write transaction upgrades fail immediately to avoid deadlock).The desktop app's single-connection path is unchanged apart from the PRAGMAs and the improved error.
Tests
11 tests in
connection_pragmas_test.rs: WAL +busy_timeout+-wal/-shmsidecars on a file DB;:memory:fallback; write-pool setup (PRAGMAs, functions, triggers, trigger-drivensort/uuidgeneration);query_onlyenforcement; and multi-connection contention — a reader proceeding with snapshot isolation during an open write transaction, a second writer failing instantly atbusy_timeout = 0but waiting out a 300ms-held lock at the 3000ms default, andretry_on_busyretrying through a held lock. Contention tests ran 5× consecutively without flakes.Soak: generated a 2000-book stress library with all writes under WAL —
foreign_key_checkandintegrity_checkclean. Full libcalibre suite (17 binaries) green; workspace clippy/fmt clean.Out of scope (per ticket)
The server crate, the cross-process write policy (research doc D6), and the pre-existing trigger-DDL-on-open race between two processes opening the same library.
Fixes CDL-18
🤖 Generated with Claude Code