Share DB connection pragmas across the pool, and wait for DB locks by ara4n · Pull Request #186 · matrix-org/seshat

ara4n · 2026-06-24T11:42:00Z

Calculate the sqlcipher passphrase PBKDF once for the DB, rather than on every DB connection (saves ~14s of CPU on my laptop on launch)
Don't set the WAL journal pragma on every connection, but on the DB (means you don't have to wait for a lock, and presumably means you only truncate WALs on launch rather than repeatedly on every connection)
Wait 5s for DB locks rather than failing instantly if the DB is locked (breaking search / crawling on a busy machine)

Fixes bits of element-hq/element-web#32119

in an attempt to reduce db locks. Also, wait 5s for a lock if needed.

`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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JP4yoxKeGLRDLkCPTu3DFq

richvdh

Seems plausible in principle, but there's a bunch of stuff that needs cleaning up, please.

On the subject of the commit messages:

Claude-Session: https://claude.ai/code/session_01JP4yoxKeGLRDLkCPTu3DFq

should this link do something useful?

unlock becomes verify_unlocked, which only confirms the (already-keyed) connection decrypts,

I can't find a verify_unlocked ?

richvdh · 2026-07-16T20:12:20Z

+    /// 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<()> {


could we maybe give set_pragmas and init_pragmas better names, which better reflect how they are used? I'm slightly strugging for good names, but maybe set_per_connection_pragmas and set_global_pragmas?

richvdh · 2026-07-16T20:13:12Z

+    /// 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(())
    }


I'd actually be tempted to inline this, rather than have a separate method. It's only two lines, and it's only called in one place.

richvdh · 2026-07-16T20:14:23Z

+    /// 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`).
+    ///


These are connection-scoped settings that are cheap and take no database lock, so they must run on every connection

Why does the fact they are cheap mean that they have to be run on every connection?

well, this seems to have been addressed in the second commit :-S

richvdh · 2026-07-16T21:24:57Z

+    /// 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`.


This feels unnecessarily verbose for the doc-comment of this method. I think I'd just get rid of it.

richvdh · 2026-07-16T21:33:56Z

+        // Connections from the pool are already keyed and have their per-connection
+        // pragmas applied by the manager's init hook (see `build_manager`).


I don't think we need a comment like this (or similar) every time we get a connection from the pool.

richvdh · 2026-07-16T22:13:51Z

-                // 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


It would be nice to retain at least the link to the cipher_migrate doc.

richvdh · 2026-07-16T22:14:47Z

+            return Ok(());
+        }
+
+        // Otherwise it was created with older cipher defaults; migrate it in place.


Suggested change

// Otherwise it was created with older cipher defaults; migrate it in place.

// Otherwise it may have been created with an older SQLCipher version; try migrating it.

richvdh · 2026-07-16T22:15:57Z

+        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()?;


To me, a "bare connection" would imply not going via a pool. Indeed, AFAICT we never reuse connections from this pool. Can't we get rid of the pool here?

Suggested change

let probe = bare.get()?;

let probe = rusqlite::Connection::open(db_path)?;

richvdh · 2026-07-16T22:16:09Z

+        // 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()?;


Suggested change

// 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.

let connection = rusqlite::Connection::open(db_path)?;

richvdh · 2026-07-16T22:17:01Z

-                // The cipher_migrate pragma returns a single row/column with the value set to `0`
-                // if we succeeded.


only set journal_mode WAL at init, not on every connection

71f76dc

in an attempt to reduce db locks. Also, wait 5s for a lock if needed.

ara4n mentioned this pull request Jun 24, 2026

Seshat/Tantivy reliably pegs CPU at 100-200% for 30s at a time element-hq/element-web#32119

Open

ara4n force-pushed the matthew/connection-pragmas branch from dd92c4d to b3d6b5f Compare June 24, 2026 13:41

ara4n force-pushed the matthew/connection-pragmas branch from b3d6b5f to 5d0211c Compare June 24, 2026 15:27

richvdh requested changes Jul 16, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Share DB connection pragmas across the pool, and wait for DB locks#186

Share DB connection pragmas across the pool, and wait for DB locks#186
ara4n wants to merge 2 commits into
mainfrom
matthew/connection-pragmas

ara4n commented Jun 24, 2026 •

edited

Loading

Uh oh!

richvdh left a comment •

edited

Loading

Uh oh!

richvdh Jul 16, 2026

Uh oh!

richvdh Jul 16, 2026 •

edited

Loading

Uh oh!

richvdh Jul 16, 2026

Uh oh!

richvdh Jul 16, 2026

Uh oh!

richvdh Jul 16, 2026

Uh oh!

richvdh Jul 16, 2026

Uh oh!

richvdh Jul 16, 2026

Uh oh!

richvdh Jul 16, 2026 •

edited

Loading

Uh oh!

richvdh Jul 16, 2026

Uh oh!

richvdh Jul 16, 2026

Uh oh!

richvdh Jul 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// Connections from the pool are already keyed and have their per-connection
		// pragmas applied by the manager's init hook (see `build_manager`).

	// Otherwise it was created with older cipher defaults; migrate it in place.
	// Otherwise it may have been created with an older SQLCipher version; try migrating it.

	let probe = bare.get()?;
	let probe = rusqlite::Connection::open(db_path)?;

		// The cipher_migrate pragma returns a single row/column with the value set to `0`
		// if we succeeded.

Uh oh!

Conversation

ara4n commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

richvdh left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

richvdh Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

richvdh Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ara4n commented Jun 24, 2026 •

edited

Loading

richvdh left a comment •

edited

Loading

richvdh Jul 16, 2026 •

edited

Loading

richvdh Jul 16, 2026 •

edited

Loading