feat(libcalibre): enable WAL + busy_timeout PRAGMAs and add connection pools

crates/libcalibre/Cargo.toml

@@ -7,7 +7,7 @@ edition = "2021"
 
 [dependencies]
 chrono = { version = "0.4.31", features = ["serde"] }
-diesel = { version = "2.2.4", features = ["sqlite", "chrono", "returning_clauses_for_sqlite_3_35"] }
+diesel = { version = "2.2.4", features = ["sqlite", "chrono", "r2d2", "returning_clauses_for_sqlite_3_35"] }
 diesel_migrations = { version = "2.2.0", features = ["sqlite"] }
 epub = "2.1.1"
 mobi = "0.8.0"

crates/libcalibre/src/library.rs

@@ -204,8 +204,7 @@ pub struct TagSummary {
 
 impl Library {
     pub fn new(db_path: ValidDbPath) -> Result<Self, CalibreError> {
-        let conn = establish_connection(&db_path.database_path)
-            .map_err(|_| CalibreError::LibraryNotInitialized)?;
+        let conn = establish_connection(&db_path.database_path)?;
 
         Ok(Self { db_path, conn })
     }

crates/libcalibre/src/persistence.rs

@@ -1,7 +1,11 @@
+use std::time::Duration;
+
+use diesel::connection::SimpleConnection;
 use diesel::prelude::*;
+use diesel::r2d2::{ConnectionManager, CustomizeConnection, Pool};
 use diesel::sql_query;
-use diesel::sql_types::Text;
 
+use crate::error::CalibreError;
 use crate::sorting;
 
 /// Converts book title to sortable format for SQL.
@@ -84,24 +88,185 @@ pub fn register_triggers(conn: &mut SqliteConnection) -> Result<(), diesel::resu
     Ok(())
 }
 
-pub fn establish_connection(db_path: &str) -> Result<diesel::SqliteConnection, ()> {
-    // Setup custom SQL functions. Required because Calibre does this.
-    // See: https://github.com/kovidgoyal/calibre/blob/7f3ccb333d906f5867636dd0dc4700b495e5ae6f/src/calibre/library/database.py#L55-L70
-    define_sql_function!(fn title_sort(title: Text) -> Text);
-    define_sql_function!(fn uuid4() -> Text);
-    define_sql_function!(fn author_to_author_sort(name: Text) -> Text);
+// Custom SQL functions Calibre relies on. Declared at module scope (rather than
+// inside the establishing fn) so they can be (re)registered on any connection —
+// a single owned one or every checkout from a pool.
+// See: https://github.com/kovidgoyal/calibre/blob/7f3ccb333d906f5867636dd0dc4700b495e5ae6f/src/calibre/library/database.py#L55-L70
+define_sql_function!(fn title_sort(title: diesel::sql_types::Text) -> diesel::sql_types::Text);
+define_sql_function!(fn uuid4() -> diesel::sql_types::Text);
+define_sql_function!(fn author_to_author_sort(name: diesel::sql_types::Text) -> diesel::sql_types::Text);
+
+/// Register Calibre's custom SQL functions (`title_sort`, `uuid4`,
+/// `author_to_author_sort`) on `conn`. These are per-connection, so they must
+/// be registered on every connection — including each one a pool hands out.
+pub fn register_sql_functions(conn: &mut SqliteConnection) -> Result<(), diesel::result::Error> {
+    title_sort_utils::register_impl(conn, sort_book_title)?;
+    uuid4_utils::register_impl(conn, || uuid::Uuid::new_v4().to_string())?;
+    author_to_author_sort_utils::register_impl(conn, sort_author_name)?;
+    Ok(())
+}
+
+/// Apply the PRAGMAs libcalibre wants on every SQLite connection.
+///
+/// Run in order as individual statements: `busy_timeout` goes first so the
+/// subsequent WAL switch (which may briefly need a write lock) waits instead of
+/// failing with `SQLITE_BUSY`. WAL plus a non-zero busy timeout removes the
+/// spurious `SQLITE_BUSY` errors that the DB-write + `metadata.opf`-write pair
+/// per book edit can otherwise provoke.
+///
+/// `foreign_keys = ON` is future-proofing: Calibre's schema (including the
+/// custom-column tables) declares no FK constraints today, so enabling
+/// enforcement changes nothing for existing libraries.
+///
+/// On `:memory:` and other temp databases SQLite silently keeps its in-memory
+/// journal (reporting `journal_mode` = `memory`); this is not an error.
+///
+/// NOTE: WAL coordinates concurrent access only between connections **on the
+/// same host** (via the `-shm` file). A library on a network/cloud filesystem
+/// is unsupported under WAL.
+fn apply_pragmas(conn: &mut SqliteConnection) -> Result<(), diesel::result::Error> {
+    conn.batch_execute(
+        "PRAGMA busy_timeout = 3000;\
+         PRAGMA journal_mode = WAL;\
+         PRAGMA synchronous = NORMAL;\
+         PRAGMA foreign_keys = ON;\
+         PRAGMA wal_autocheckpoint = 1000;",
+    )
+}
 
-    let mut connection = diesel::SqliteConnection::establish(db_path).or(Err(()))?;
+/// Per-connection setup: PRAGMAs plus the custom SQL functions. Both are
+/// scoped to a single SQLite connection, so every connection — owned or
+/// pooled — needs them.
+///
+/// Triggers are deliberately NOT registered here: they are persistent database
+/// objects (stored in `sqlite_master`), and re-running their DROP/CREATE DDL
+/// from every connection would both serialize checkouts on the write lock and
+/// open a window where a concurrent insert fires no trigger. Callers register
+/// triggers once per database instead (see [`establish_connection`] and
+/// [`create_write_pool`]).
+///
+/// Function registration is best-effort (matching the historical behaviour);
+/// PRAGMA failures propagate.
+fn prepare_connection(conn: &mut SqliteConnection) -> Result<(), diesel::result::Error> {
+    apply_pragmas(conn)?;
 
     // Register SQL function implementations. Ignore any errors.
-    let _ = title_sort_utils::register_impl(&mut connection, sort_book_title);
-    let _ = uuid4_utils::register_impl(&mut connection, || uuid::Uuid::new_v4().to_string());
-    let _ = author_to_author_sort_utils::register_impl(&mut connection, sort_author_name);
+    let _ = register_sql_functions(conn);
+
+    Ok(())
+}
+
+pub fn establish_connection(db_path: &str) -> Result<SqliteConnection, CalibreError> {
+    let mut connection = SqliteConnection::establish(db_path).map_err(CalibreError::database)?;
+    prepare_connection(&mut connection)?;
 
-    // Register triggers for data integrity and automatic field generation
+    // Register triggers for data integrity and automatic field generation.
+    // Best-effort: a fresh or non-Calibre database may lack the `books` table
+    // the triggers attach to, and that must not fail opening the connection.
     register_triggers(&mut connection)
         .map_err(|e| eprintln!("Failed to register triggers: {}", e))
         .ok();
 
     Ok(connection)
 }
+
+/// An r2d2 pool of Calibre connections. Every connection it hands out has the
+/// PRAGMAs applied and the custom SQL functions registered.
+pub type CalibrePool = Pool<ConnectionManager<SqliteConnection>>;
+
+/// Applies libcalibre's per-connection setup (PRAGMAs + custom SQL functions)
+/// to every connection the pool creates — SQLite scopes both to a single
+/// connection, so the pool's first connection is not enough. Read-only
+/// customizers additionally set `PRAGMA query_only`, making writes through a
+/// read pool a hard error rather than a documentation violation.
+#[derive(Debug)]
+struct CalibreConnectionCustomizer {
+    read_only: bool,
+}
+
+impl CustomizeConnection<SqliteConnection, diesel::r2d2::Error> for CalibreConnectionCustomizer {
+    fn on_acquire(&self, conn: &mut SqliteConnection) -> Result<(), diesel::r2d2::Error> {
+        prepare_connection(conn).map_err(diesel::r2d2::Error::QueryError)?;
+        if self.read_only {
+            conn.batch_execute("PRAGMA query_only = ON")
+                .map_err(diesel::r2d2::Error::QueryError)?;
+        }
+        Ok(())
+    }
+}
+
+/// Build the single-writer pool for the Calibre database at `db_path`.
+///
+/// The pool holds exactly **one** connection: SQLite has one write lock, so a
+/// multi-writer pool would only queue on it (and collapses under contention —
+/// ~20× in published benchmarks). Callers that need parallel reads pair this
+/// with [`create_read_pool`]. The desktop app keeps using the single owned
+/// connection from [`establish_connection`].
+///
+/// Calibre's triggers are (re)registered here, once, through the writer —
+/// they persist in the database, so per-checkout DDL is both unnecessary and
+/// racy.
+pub fn create_write_pool(db_path: &str) -> Result<CalibrePool, CalibreError> {
+    let manager = ConnectionManager::<SqliteConnection>::new(db_path);
+    let pool = Pool::builder()
+        .max_size(1)
+        .connection_customizer(Box::new(CalibreConnectionCustomizer { read_only: false }))
+        .build(manager)
+        .map_err(CalibreError::database)?;
+
+    let mut conn = pool.get().map_err(CalibreError::database)?;
+    register_triggers(&mut conn)
+        .map_err(|e| eprintln!("Failed to register triggers: {}", e))
+        .ok();
+
+    Ok(pool)
+}
+
+/// Build a pool of `max_size` **read-only** connections (`PRAGMA query_only`)
+/// for the Calibre database at `db_path`. Attempting to write through one of
+/// these connections fails; route writes through [`create_write_pool`].
+///
+/// Read connections never run trigger DDL — triggers live in the database
+/// itself and are registered by the write path.
+pub fn create_read_pool(db_path: &str, max_size: u32) -> Result<CalibrePool, CalibreError> {
+    let manager = ConnectionManager::<SqliteConnection>::new(db_path);
+    Pool::builder()
+        .max_size(max_size)
+        .connection_customizer(Box::new(CalibreConnectionCustomizer { read_only: true }))
+        .build(manager)
+        .map_err(CalibreError::database)
+}
+
+/// True for the errors SQLite raises when a required lock is held elsewhere
+/// (`SQLITE_BUSY` "database is locked" / `SQLITE_LOCKED` "database table is
+/// locked").
+fn is_busy_error(error: &diesel::result::Error) -> bool {
+    matches!(
+        error,
+        diesel::result::Error::DatabaseError(_, info) if info.message().contains("locked")
+    )
+}
+
+/// Run `op`, retrying (with short linear backoff) when SQLite reports the
+/// database is locked. `busy_timeout` already makes connections wait for the
+/// lock, but a busy error can still surface — most notably when a transaction
+/// tries to upgrade from read to write while another writer is active, which
+/// SQLite fails immediately to avoid deadlock. Wrap short write transactions
+/// in this when they must coexist with other writers.
+///
+/// Non-busy errors and the final busy error (after `max_attempts`) propagate.
+pub fn retry_on_busy<T>(
+    max_attempts: u32,
+    mut op: impl FnMut() -> Result<T, diesel::result::Error>,
+) -> Result<T, diesel::result::Error> {
+    let mut attempt = 1;
+    loop {
+        match op() {
+            Err(e) if is_busy_error(&e) && attempt < max_attempts => {
+                std::thread::sleep(Duration::from_millis(10 * u64::from(attempt)));
+                attempt += 1;
+            }
+            result => return result,
+        }
+    }
+}