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).
This commit is contained in:
nvrl committed 2026-10-16 09:56:25 +02:00
1 parent 0f279c7536
commit 2bea1f7672
2 files changed
+66 -3

No files matched your search

+44 -3
View File
@@ -91,8 +91,13 @@ impl Db {
/// Returns an empty vector when the database is sound; otherwise each entry /// 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, /// 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. /// 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<Vec<String>> { pub fn integrity_check(&self) -> Result<Vec<String>> {
let conn = self.conn.lock(); let conn = self.read();
let mut stmt = conn.prepare("PRAGMA integrity_check")?; let mut stmt = conn.prepare("PRAGMA integrity_check")?;
let rows = stmt.query_map([], |r| r.get::<_, String>(0))?; let rows = stmt.query_map([], |r| r.get::<_, String>(0))?;
let mut problems = Vec::new(); let mut problems = Vec::new();
@@ -107,9 +112,11 @@ impl Db {
Ok(problems) 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<DbStats> { pub fn stats(&self) -> Result<DbStats> {
let conn = self.conn.lock(); let conn = self.read();
let page_count: i64 = conn.query_row("PRAGMA page_count", [], |r| r.get(0))?; 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 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))?; 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()); 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] #[test]
fn stats_report_the_schema_and_the_known_tables() { fn stats_report_the_schema_and_the_known_tables() {
let db = Db::open_in_memory().unwrap(); let db = Db::open_in_memory().unwrap();
+22
View File
@@ -100,6 +100,7 @@ a live account since.
| Entry | Title | | 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 | | [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 | | [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 | | [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. 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 ### B217 — The search indexes grow to many times their size and slow search down
**Status:** Fixed, not verified live **Status:** Fixed, not verified live