From 2bea1f76720078272a371ad25e2df13aa68f1ea7 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Fri, 16 Oct 2026 09:56:25 +0200 Subject: [PATCH] fix(core): check and count the database on a read connection PRAGMA integrity_check and the row counts behind pdfs cache inspect read the whole file, 34 s on a 567 MiB database, and they held the write connection while they did. Every write in the daemon waited behind them. They now read a WAL snapshot on a pooled read connection (B218). --- crates/pdfs-core/src/db/maintenance.rs | 47 ++++++++++++++++++++++++-- docs/BUGS.md | 22 ++++++++++++ 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/crates/pdfs-core/src/db/maintenance.rs b/crates/pdfs-core/src/db/maintenance.rs index 09e2005..8f0f9dc 100644 --- a/crates/pdfs-core/src/db/maintenance.rs +++ b/crates/pdfs-core/src/db/maintenance.rs @@ -91,8 +91,13 @@ impl Db { /// Returns an empty vector when the database is sound; otherwise each entry /// is one problem SQLite found. Read-only and safe against a live daemon, /// though it reads every page, so it is not free on a large file. + /// + /// Runs on a read connection: it reads every page, which took 34 s on a + /// 567 MiB file, and on the write connection every write in the daemon + /// waited that long behind it (`docs/BUGS.md` B218). WAL lets it read a + /// consistent snapshot while the writer goes on. pub fn integrity_check(&self) -> Result> { - let conn = self.conn.lock(); + let conn = self.read(); let mut stmt = conn.prepare("PRAGMA integrity_check")?; let rows = stmt.query_map([], |r| r.get::<_, String>(0))?; let mut problems = Vec::new(); @@ -107,9 +112,11 @@ impl Db { Ok(problems) } - /// Size and row-count snapshot for `pdfs cache inspect`. + /// Size and row-count snapshot for `pdfs cache inspect`. On a read + /// connection, for the reason [`integrity_check`](Self::integrity_check) + /// gives: counting the rows of every table reads most of the file. pub fn stats(&self) -> Result { - let conn = self.conn.lock(); + let conn = self.read(); let page_count: i64 = conn.query_row("PRAGMA page_count", [], |r| r.get(0))?; let free_pages: i64 = conn.query_row("PRAGMA freelist_count", [], |r| r.get(0))?; let page_size: i64 = conn.query_row("PRAGMA page_size", [], |r| r.get(0))?; @@ -218,6 +225,40 @@ mod tests { assert!(db.integrity_check().unwrap().is_empty()); } + /// `pdfs diagnose` and `pdfs cache inspect` read the whole file. Behind the + /// write lock they froze every write in the daemon for as long as that + /// took (`docs/BUGS.md` B218). + #[test] + fn the_integrity_check_and_stats_do_not_wait_for_the_writer() { + let path = std::env::temp_dir().join(format!( + "pdfs-check-{}-{}.db", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + let db = std::sync::Arc::new(Db::open(&path).unwrap()); + let writer = db.conn.lock(); + let (tx, rx) = std::sync::mpsc::channel(); + let reader = std::sync::Arc::clone(&db); + std::thread::spawn(move || { + let checked = reader.integrity_check().map(|p| p.is_empty()); + let counted = reader.stats().map(|s| s.page_count > 0); + let _ = tx.send((checked.ok(), counted.ok())); + }); + let answer = rx.recv_timeout(std::time::Duration::from_secs(10)); + drop(writer); + assert_eq!(answer.ok(), Some((Some(true), Some(true)))); + drop(db); + for suffix in ["", "-wal", "-shm"] { + let mut file = path.as_os_str().to_owned(); + file.push(suffix); + let _ = std::fs::remove_file(std::path::PathBuf::from(file)); + } + let _ = std::fs::remove_file(path.with_extension("db.lock")); + } + #[test] fn stats_report_the_schema_and_the_known_tables() { let db = Db::open_in_memory().unwrap(); diff --git a/docs/BUGS.md b/docs/BUGS.md index 0a470f8..40df4e3 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -100,6 +100,7 @@ a live account since. | Entry | Title | |---|---| +| [B218](#b218--pdfs-diagnose-freezes-the-mount-while-it-checks-the-database) | `pdfs diagnose` freezes the mount while it checks the database | | [B217](#b217--the-search-indexes-grow-to-many-times-their-size-and-slow-search-down) | The search indexes grow to many times their size and slow search down | | [B216](#b216--a-damaged-database-is-rebuilt-without-the-writes-still-queued-in-it) | A damaged database is rebuilt without the writes still queued in it | | [B215](#b215--a-remote-name-holding--or--becomes-a-path-outside-the-sync-folder) | A remote name holding `/` or `..` becomes a path outside the sync folder | @@ -320,6 +321,27 @@ a live account since. Newest first. +### B218 — `pdfs diagnose` freezes the mount while it checks the database + +**Status:** Fixed, not verified live +**Found:** 2026-10-16, architecture audit (`FINDINGS.md` 2.2.2). `PRAGMA integrity_check` took +33.9 s on a copy of a live 567 MiB `cache.db`. +**Where:** `Db::integrity_check` and `Db::stats` in `crates/pdfs-core/src/db/maintenance.rs` + +**Repro:** Run `pdfs diagnose` or `pdfs cache inspect` on a large database while copying files +into the mount. + +**Cause.** Both read the whole file through the single write connection. Every write in the +daemon took that connection's mutex in the meantime: the write-through of the tree, the drain's +claims, every enqueue. So the mount looked hung for as long as the check ran. + +**Fix.** Both run on a pooled read connection. WAL lets them read a consistent snapshot while the +writer goes on. `VACUUM` still needs the write connection and remains a deliberate command. + +**Test:** `db::maintenance::tests::the_integrity_check_and_stats_do_not_wait_for_the_writer`. + +--- + ### B217 — The search indexes grow to many times their size and slow search down **Status:** Fixed, not verified live