From 71f76dc042dcecfe93d715b4f3ef3e24bab0e472 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 23 Jun 2026 16:50:09 +0100 Subject: [PATCH 1/2] only set journal_mode WAL at init, not on every connection in an attempt to reduce db locks. Also, wait 5s for a lock if needed. --- src/database/mod.rs | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/src/database/mod.rs b/src/database/mod.rs index d5700cb1..a40a48d8 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -161,6 +161,10 @@ impl Database { let writer_connection = pool.get()?; Database::unlock(&writer_connection, config)?; Database::set_pragmas(&writer_connection)?; + // Establish WAL mode (persistent) and truncate the WAL once at startup, + // on the writer connection only - these take a database lock so must not + // run per connection acquisition. + Database::init_pragmas(&writer_connection)?; // Load the set of event IDs that have been replaced by edit events. let replaced_event_ids = Database::load_replaced_event_ids(&writer_connection); @@ -230,10 +234,34 @@ impl Database { } } + /// Apply the per-connection PRAGMAs. These are connection-scoped settings + /// that are cheap and take no database lock, so they must run on every + /// connection (the writer and every pooled reader acquired via + /// `get_connection`). + /// + /// Note: this deliberately does NOT set `journal_mode` or run + /// `wal_checkpoint`. Both of those take a database-level lock and contend + /// with the writer; running them on every reader acquisition was a major + /// source of "database is locked" errors (every search / isRoomIndexed + /// would briefly fight the writer). `journal_mode=WAL` is persistent in the + /// database file, so it only needs to be set once - see `init_pragmas`. fn set_pragmas(connection: &rusqlite::Connection) -> Result<()> { connection.pragma_update(None, "foreign_keys", &1 as &dyn ToSql)?; - connection.pragma_update(None, "journal_mode", "WAL")?; connection.pragma_update(None, "synchronous", "NORMAL")?; + // Wait for locks instead of failing immediately with "database is + // locked". With the default (0), any momentary contention makes the + // operation fail instantly, which wedges the crawler. + connection.busy_timeout(std::time::Duration::from_secs(5))?; + Ok(()) + } + + /// One-time, database-level initialisation run once at startup on the writer + /// connection. These operations take a database lock, so they must NOT run + /// on every connection acquisition. `journal_mode=WAL` is persistent in the + /// database file (it applies to all subsequent connections automatically), + /// and the WAL is truncated once here rather than on every read. + fn init_pragmas(connection: &rusqlite::Connection) -> Result<()> { + connection.pragma_update(None, "journal_mode", "WAL")?; connection.execute_batch("PRAGMA wal_checkpoint(TRUNCATE);")?; Ok(()) } From 5d0211c0dae1b031a06000681ff49fe77d09ada0 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Wed, 24 Jun 2026 00:20:46 +0100 Subject: [PATCH 2/2] Derive the SQLCipher key once per connection, not per checkout `get_connection()` ran `Database::unlock()` -> `PRAGMA key` on every pool checkout, and r2d2 reuses physical connections, so the key was re-derived on every read. SQLCipher's `PRAGMA key` runs PBKDF2 (~256k iterations, ~20ms), so the startup reconciliation scan - one `get_connection` per `isRoomIndexed` across thousands of rooms - spent ~14s of CPU (24% of a core for the first minute) re-deriving the same key hundreds of times. Move the keying into the r2d2 connection manager's `with_init` hook (`build_manager`), which runs once per physical connection, alongside the cheap per-connection pragmas. `get_connection`/`new_with_config`/recovery no longer re-key on checkout. `unlock` becomes `verify_unlocked`, which only confirms the (already-keyed) connection decrypts, without the expensive `PRAGMA key`. The rare cipher-migration path still runs the required "PRAGMA key; PRAGMA cipher_migrate" sequence on a bare connection. Profiling confirms kdf_pbkdf2_derive drops from ~14.4s to 0 at launch. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01JP4yoxKeGLRDLkCPTu3DFq --- src/database/mod.rs | 185 +++++++++++++++++++++------------------ src/database/recovery.rs | 7 +- 2 files changed, 102 insertions(+), 90 deletions(-) diff --git a/src/database/mod.rs b/src/database/mod.rs index a40a48d8..0fa844a2 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -131,11 +131,10 @@ impl Database { let db_path = path.as_ref().join(EVENTS_DB_NAME); let pool = Self::get_pool(&db_path, config)?; + // Connections from the pool are already keyed and have their per-connection + // pragmas applied by the manager's init hook (see `build_manager`). let mut connection = pool.get()?; - Database::unlock(&connection, config)?; - Database::set_pragmas(&connection)?; - let (version, reindex_needed) = match Database::get_version(&mut connection) { Ok(ret) => ret, Err(e) => return Err(Error::DatabaseOpenError(e.to_string())), @@ -159,8 +158,7 @@ impl Database { // a new database and we'll end up with two connections using differing // keys and writes/reads to one of the connections might fail. let writer_connection = pool.get()?; - Database::unlock(&writer_connection, config)?; - Database::set_pragmas(&writer_connection)?; + // Keyed + per-connection pragmas already applied by the pool init hook. // Establish WAL mode (persistent) and truncate the WAL once at startup, // on the writer connection only - these take a database lock so must not // run per connection acquisition. @@ -191,61 +189,111 @@ impl Database { }) } - fn get_pool(db_path: &PathBuf, config: &Config) -> Result> { - let manager = SqliteConnectionManager::file(db_path); - let pool = r2d2::Pool::new(manager)?; - let connection = pool.get()?; - - // Try to unlock a single connection. - match Database::unlock(&connection, config) { - // We're fine, the connection was returned successfully, we can return the pool. - Ok(_) => Ok(pool), - Err(_) => { - let Some(passphrase) = &config.passphrase else { - // No passphrase was provided, and we failed to unlock a connection, return an - // error. - return Err(Error::DatabaseUnlockError("Invalid passphrase".to_owned())); - }; - - // Ok, let's see if the unlock of the connection failed because of new default - // settings for the cipher settings, let's see if we can migrate the cipher settings. - // Take a look at the documentation of cipher_migrate[1] for more info. - // - // [1]: https://www.zetetic.net/sqlcipher/sqlcipher-api/#cipher_migrate - let connection = pool.get()?; + /// Build the connection manager for the events DB pool. + /// + /// The `with_init` hook runs once per *physical* connection (r2d2 pools and + /// reuses connections), so the expensive SQLCipher `PRAGMA key` - which runs + /// PBKDF2 with ~256k iterations - happens once per connection rather than on + /// every checkout. Keying on every `get_connection` instead made startup burn + /// seconds of CPU re-deriving the key for each `isRoomIndexed`/search during + /// the reconciliation scan. + fn build_manager(db_path: &Path, config: &Config) -> SqliteConnectionManager { + #[cfg(feature = "encryption")] + let passphrase = config.passphrase.clone(); + #[cfg(not(feature = "encryption"))] + let _ = config; + + SqliteConnectionManager::file(db_path).with_init(move |connection| { + #[cfg(feature = "encryption")] + if let Some(ref passphrase) = passphrase { connection.pragma_update(None, "key", &passphrase.as_str() as &dyn ToSql)?; + } + Database::set_pragmas(connection) + }) + } - let mut statement = connection.prepare("PRAGMA cipher_migrate")?; - let result = statement.query_row([], |row| row.get::(0))?; + fn get_pool(db_path: &Path, config: &Config) -> Result> { + // Ensure the database opens under the current cipher settings, migrating it + // on a bare connection if it predates them. This must happen BEFORE we open + // the init-hook pool below: that pool keys every connection at connect time, + // and keying a connection against an older-cipher database and holding it + // open while we migrate corrupts the in-place `cipher_migrate` (after which + // no connection can decrypt the database). + Self::migrate_cipher_settings_if_needed(db_path, config)?; - // The cipher_migrate pragma returns a single row/column with the value set to `0` - // if we succeeded. - if result == "0" { - // In this case the migration was successful and we can now recreate the pool - // so the new settings come into play. - let manager = SqliteConnectionManager::file(db_path); - let pool = r2d2::Pool::new(manager)?; + Ok(r2d2::Pool::new(Self::build_manager(db_path, config))?) + } - Ok(pool) - } else { - Err(Error::DatabaseUnlockError("Invalid passphrase".to_owned())) - } - } + /// Open the database on a bare (non-init-hook) connection and, if it was + /// created with older SQLCipher cipher defaults, migrate it in place to the + /// current ones so the init-hook pool can subsequently key it normally. + /// + /// [1]: https://www.zetetic.net/sqlcipher/sqlcipher-api/#cipher_migrate + #[cfg(feature = "encryption")] + fn migrate_cipher_settings_if_needed(db_path: &Path, config: &Config) -> Result<()> { + let Some(passphrase) = config.passphrase.as_ref() else { + return Ok(()); + }; + + let bare = r2d2::Pool::new(SqliteConnectionManager::file(db_path))?; + + // Probe on one connection: key it and see if it decrypts the database. + let probe = bare.get()?; + + let mut statement = probe.prepare("PRAGMA cipher_version")?; + let results = statement.query_map([], |row| row.get::(0))?; + if results.count() != 1 { + return Err(Error::SqlCipherError( + "Sqlcipher support is missing".to_string(), + )); + } + + probe.pragma_update(None, "key", &passphrase.as_str() as &dyn ToSql)?; + + // If we can read the schema the database already uses the current cipher + // settings and there's nothing to migrate. + let unlocked = probe + .query_row("SELECT COUNT(*) FROM sqlite_master", [], |row| { + row.get::(0) + }) + .is_ok(); + if unlocked { + return Ok(()); + } + + // Otherwise it was created with older cipher defaults; migrate it in place. + // `cipher_migrate` has to run on a connection whose only prior operation is + // `PRAGMA key` - the failed read above would break it - so do it on a fresh + // connection. The `probe` connection is still checked out, so this is a + // distinct physical connection. cipher_migrate returns "0" on success. + let connection = bare.get()?; + connection.pragma_update(None, "key", &passphrase.as_str() as &dyn ToSql)?; + let mut statement = connection.prepare("PRAGMA cipher_migrate")?; + let result = statement.query_row([], |row| row.get::(0))?; + if result == "0" { + Ok(()) + } else { + Err(Error::DatabaseUnlockError("Invalid passphrase".to_owned())) } } - /// Apply the per-connection PRAGMAs. These are connection-scoped settings - /// that are cheap and take no database lock, so they must run on every - /// connection (the writer and every pooled reader acquired via - /// `get_connection`). + #[cfg(not(feature = "encryption"))] + fn migrate_cipher_settings_if_needed(_: &Path, _: &Config) -> Result<()> { + Ok(()) + } + + /// Apply the cheap, connection-scoped PRAGMAs. Run once per *physical* + /// connection from the pool's init hook (see `build_manager`), since these + /// settings persist for the connection's lifetime - there's no need to + /// re-apply them on every pool checkout. /// /// Note: this deliberately does NOT set `journal_mode` or run /// `wal_checkpoint`. Both of those take a database-level lock and contend - /// with the writer; running them on every reader acquisition was a major - /// source of "database is locked" errors (every search / isRoomIndexed - /// would briefly fight the writer). `journal_mode=WAL` is persistent in the - /// database file, so it only needs to be set once - see `init_pragmas`. - fn set_pragmas(connection: &rusqlite::Connection) -> Result<()> { + /// with the writer; running them on every connection was a major source of + /// "database is locked" errors (every search / isRoomIndexed would briefly + /// fight the writer). `journal_mode=WAL` is persistent in the database file, + /// so it only needs to be set once - see `init_pragmas`. + fn set_pragmas(connection: &rusqlite::Connection) -> std::result::Result<(), rusqlite::Error> { connection.pragma_update(None, "foreign_keys", &1 as &dyn ToSql)?; connection.pragma_update(None, "synchronous", "NORMAL")?; // Wait for locks instead of failing immediately with "database is @@ -298,39 +346,6 @@ impl Database { Ok(()) } - #[cfg(feature = "encryption")] - fn unlock(connection: &rusqlite::Connection, config: &Config) -> Result<()> { - let passphrase: &String = if let Some(ref p) = config.passphrase { - p - } else { - return Ok(()); - }; - - let mut statement = connection.prepare("PRAGMA cipher_version")?; - let results = statement.query_map([], |row| row.get::(0))?; - - if results.count() != 1 { - return Err(Error::SqlCipherError( - "Sqlcipher support is missing".to_string(), - )); - } - - connection.pragma_update(None, "key", passphrase as &dyn ToSql)?; - - let count: std::result::Result = - connection.query_row("SELECT COUNT(*) FROM sqlite_master", [], |row| row.get(0)); - - match count { - Ok(_) => Ok(()), - Err(_) => Err(Error::DatabaseUnlockError("Invalid passphrase".to_owned())), - } - } - - #[cfg(not(feature = "encryption"))] - fn unlock(_: &rusqlite::Connection, _: &Config) -> Result<()> { - Ok(()) - } - /// Get the size of the database. /// This returns the number of bytes the database is using on disk. pub fn get_size(&self) -> Result { @@ -626,9 +641,9 @@ impl Database { /// Get a database connection. /// Note that this connection should only be used for reading. pub fn get_connection(&self) -> Result { + // Already keyed + pragma'd by the pool's init hook (see `build_manager`). + // Crucially this means no per-checkout SQLCipher key derivation (PBKDF2). let connection = self.pool.get()?; - Database::unlock(&connection, &self.config)?; - Database::set_pragmas(&connection)?; Ok(Connection { inner: connection, diff --git a/src/database/recovery.rs b/src/database/recovery.rs index bb82c4ca..385eb033 100644 --- a/src/database/recovery.rs +++ b/src/database/recovery.rs @@ -11,7 +11,6 @@ use std::{convert::TryInto, io::Error as IoError}; use r2d2::PooledConnection; use r2d2_sqlite::SqliteConnectionManager; -use rusqlite::ToSql; use crate::{ config::Config, @@ -94,9 +93,8 @@ impl RecoveryDatabase { let db_path = path.as_ref().join(EVENTS_DB_NAME); let pool = Database::get_pool(&db_path, config)?; + // Keyed + per-connection pragmas already applied by the pool init hook. let mut connection = pool.get()?; - Database::unlock(&connection, config)?; - connection.pragma_update(None, "foreign_keys", &1 as &dyn ToSql)?; let (version, _) = match Database::get_version(&mut connection) { Ok(ret) => ret, @@ -285,9 +283,8 @@ impl RecoveryDatabase { /// /// Note that this connection should only be used for reading. pub fn get_connection(&self) -> Result { + // Already keyed + pragma'd by the pool's init hook (see `build_manager`). let connection = self.pool.get()?; - Database::unlock(&connection, &self.config)?; - Database::set_pragmas(&connection)?; Ok(Connection { inner: connection,