fix(db): stop 'set sqlite pragmas' lock flake on concurrent connects

Set busy_timeout before the WAL switch and retry the pragma batch on
'database is locked' within the same 5 s budget: while a file is still in
rollback mode, a shared-lock holder upgrading for the journal_mode change
gets SQLITE_BUSY without the busy handler running, so busy_timeout alone
doesn't cover parallel opens of a fresh file. Adds a 16-thread stress test
and drops the per-module DB_INIT workaround in ai::handlers tests.

Closes #157

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Cameron CordesandClaude Opus 5.5 committed 2026-10-21 10:16:14 -04:00
1 parent e8aa337442
commit e194c02393
2 files changed
+63 -16

No files matched your search

-8
View File
@@ -2129,15 +2129,7 @@ mod turn_replay_tests {
use actix_web::{App, web::Data};
use std::sync::Arc;
/// Serialize `AppState::test_state()` construction across the parallel
/// tests in this module: each build opens ~10 DAO connections to the one
/// shared `DATABASE_URL` file, and doing several at once races the WAL
/// `journal_mode` switch into a spurious "database is locked". The test
/// bodies themselves still run in parallel; only the open is gated.
static DB_INIT: std::sync::Mutex<()> = std::sync::Mutex::new(());
fn build_state() -> Data<AppState> {
let _guard = DB_INIT.lock().unwrap_or_else(|p| p.into_inner());
Data::new(AppState::test_state())
}
+63 -8
View File
@@ -167,7 +167,11 @@ impl UserDao for SqliteUserDao {
pub fn connect() -> SqliteConnection {
let db_url = dotenv::var("DATABASE_URL").expect("DATABASE_URL must be set");
let mut conn = SqliteConnection::establish(&db_url).expect("Error connecting to DB");
connect_to(&db_url)
}
fn connect_to(db_url: &str) -> SqliteConnection {
let mut conn = SqliteConnection::establish(db_url).expect("Error connecting to DB");
// Each DAO opens its own connection (13+ across the app) and they all
// share one DB file. Without WAL, a writer holds an exclusive lock
// that blocks readers — `load_persons` racing the face-watch write
@@ -186,13 +190,33 @@ pub fn connect() -> SqliteConnection {
// automatically and code that previously did manual two-step
// cleanup (delete child rows, then parent) becomes redundant but
// still correct.
conn.batch_execute(
"PRAGMA journal_mode = WAL; \
PRAGMA busy_timeout = 5000; \
PRAGMA synchronous = NORMAL; \
PRAGMA foreign_keys = ON;",
)
.expect("set sqlite pragmas");
//
// busy_timeout goes first so the WAL switch waits out ordinary
// contention. That isn't enough on its own: while the file is still in
// rollback mode, a connection that holds a shared lock and needs to
// upgrade gets SQLITE_BUSY immediately (deadlock avoidance skips the
// busy handler), so several connections opening a fresh file at once
// still fail with "database is locked" (#157). Retry for the same 5 s
// budget; once one connection has switched the file to WAL (persistent),
// the pragma is a no-op for the rest.
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5);
loop {
match conn.batch_execute(
"PRAGMA busy_timeout = 5000; \
PRAGMA journal_mode = WAL; \
PRAGMA synchronous = NORMAL; \
PRAGMA foreign_keys = ON;",
) {
Ok(()) => break,
Err(e)
if e.to_string().contains("database is locked")
&& std::time::Instant::now() < deadline =>
{
std::thread::sleep(std::time::Duration::from_millis(10));
}
Err(e) => panic!("set sqlite pragmas: {e:?}"),
}
}
conn
}
@@ -2564,6 +2588,37 @@ impl ExifDao for SqliteExifDao {
}
}
#[cfg(test)]
mod connect_tests {
use super::connect_to;
/// Many connections opening one fresh DB file at once (what parallel
/// `AppState::test_state()` builds do) must all get their pragmas set;
/// the WAL switch has to wait out contention, not fail with
/// "database is locked" (#157).
#[test]
fn concurrent_connects_to_fresh_file_all_succeed() {
for _ in 0..20 {
let dir = tempfile::tempdir().unwrap();
let url = dir.path().join("t.db").to_string_lossy().into_owned();
let barrier = std::sync::Arc::new(std::sync::Barrier::new(16));
let handles: Vec<_> = (0..16)
.map(|_| {
let url = url.clone();
let barrier = barrier.clone();
std::thread::spawn(move || {
barrier.wait();
connect_to(&url);
})
})
.collect();
for h in handles {
h.join().expect("connect panicked");
}
}
}
}
#[cfg(test)]
mod exif_dao_tests {
use super::*;