From 937a0a6824fda26f904832f223e1f1a1e953dfdb Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Fri, 16 Oct 2026 11:50:56 +0200 Subject: [PATCH] perf(core): index queued ops by kind and name (schema 43) The name checks the drain makes before each create, rename and trash, and the wake of ops waiting for a name, read the whole queue, which made a large offline batch quadratic to drain. Schema 43 adds an index on pending_op(kind, name) (B238). --- crates/pdfs-core/src/db/migrations.rs | 23 ++++++++++++++++++++++- crates/pdfs-core/src/db/tests.rs | 24 ++++++++++++++++++++++++ docs/BUGS.md | 25 +++++++++++++++++++++++++ docs/CHANGELOG.md | 3 ++- 4 files changed, 73 insertions(+), 2 deletions(-) diff --git a/crates/pdfs-core/src/db/migrations.rs b/crates/pdfs-core/src/db/migrations.rs index 5a4d5b2..1689224 100644 --- a/crates/pdfs-core/src/db/migrations.rs +++ b/crates/pdfs-core/src/db/migrations.rs @@ -11,7 +11,7 @@ use super::Db; use crate::Result; /// Current schema version. Bump on every forward migration added below. -pub(super) const SCHEMA_VERSION: i64 = 42; +pub(super) const SCHEMA_VERSION: i64 = 43; impl Db { pub(super) fn migrate(&self) -> Result<()> { @@ -440,6 +440,16 @@ impl Db { tx.execute_batch(MIGRATION_V42)?; } } + if current < 43 { + let has_ops: bool = tx.query_row( + "SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = 'pending_op'", + [], + |row| row.get::<_, i64>(0), + )? > 0; + if has_ops { + tx.execute_batch(MIGRATION_V43)?; + } + } tx.execute( "INSERT INTO sync_state (key, value) VALUES ('schema_version', ?1) ON CONFLICT(key) DO UPDATE SET value = excluded.value", @@ -1330,3 +1340,14 @@ const MIGRATION_V42: &str = " ALTER TABLE pending_op ADD COLUMN sent_sha1 TEXT; ALTER TABLE pending_op ADD COLUMN sent_to TEXT; "; + +/// Schema v43: queued ops by kind and name. +/// +/// Before each create, rename and trash the drain asks whether a queued trash +/// or rename holds the name it wants, and a released name wakes the ops +/// waiting for it. Each of those scanned the whole queue, so a large offline +/// batch made draining it quadratic. The kind leads, so a lookup of every +/// queued rename uses the index too. +const MIGRATION_V43: &str = " +CREATE INDEX IF NOT EXISTS pending_op_kind_name ON pending_op(kind, name); +"; diff --git a/crates/pdfs-core/src/db/tests.rs b/crates/pdfs-core/src/db/tests.rs index 21a7f59..63d4c1c 100644 --- a/crates/pdfs-core/src/db/tests.rs +++ b/crates/pdfs-core/src/db/tests.rs @@ -7531,6 +7531,30 @@ fn a_refused_change_names_its_node_by_local_id() { ); } +/// The name checks the drain makes before each op look the name up in an +/// index. Scanning the queue instead made a large offline batch quadratic. +#[test] +fn queue_name_checks_use_an_index() { + let db = Db::open_in_memory().unwrap(); + let conn = db.conn.lock(); + let plan = |sql: &str| -> String { + let mut stmt = conn.prepare(&format!("EXPLAIN QUERY PLAN {sql}")).unwrap(); + stmt.query_map([], |r| r.get::<_, String>(3)) + .unwrap() + .map(|row| row.unwrap()) + .collect::>() + .join("; ") + }; + for sql in [ + "SELECT 1 FROM pending_op WHERE kind = 'trash' AND name = 'a'", + "SELECT 1 FROM pending_op WHERE name = 'a' AND kind IN ('create', 'mkdir', 'rename')", + "SELECT 1 FROM pending_op WHERE kind = 'rename'", + ] { + let plan = plan(sql); + assert!(plan.contains("pending_op_kind_name"), "{sql}: {plan}"); + } +} + /// A reconnect moves up the ops that failed for want of a link. One Drive /// refused, for quota or access, keeps its backoff: the link coming back /// changes nothing about it. "Retry all" still moves both. diff --git a/docs/BUGS.md b/docs/BUGS.md index 282a941..7037d55 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -100,6 +100,7 @@ a live account since. | Entry | Title | |---|---| +| [B238](#b238--draining-a-large-offline-batch-is-quadratic-in-its-size) | Draining a large offline batch is quadratic in its size | | [B237](#b237--every-reconnect-resends-refused-uploads-all-in-the-same-second) | Every reconnect resends refused uploads, all in the same second | | [B236](#b236--imported-re-dated-and-app-uploaded-photos-have-no-thumbnail) | Imported, re-dated and app-uploaded photos have no thumbnail | | [B235](#b235--scanning-a-large-takeout-folder-takes-half-a-minute) | Scanning a large Takeout folder takes half a minute | @@ -340,6 +341,30 @@ a live account since. Newest first. +### B238 — Draining a large offline batch is quadratic in its size + +**Status:** Fixed, not verified live +**Found:** 2026-10-16, architecture audit (`FINDINGS.md` 2.2.5). +**Where:** `MIGRATION_V43` in `crates/pdfs-core/src/db/migrations.rs`; the queries in +`has_pending_trash_named`, `has_withdrawn_create`, `has_pending_move_from` and +`wake_ops_waiting_for_tx` in `crates/pdfs-core/src/db/ops.rs` + +**Repro:** Offline, copy a folder of 20,000 small files into the mount, then go online and watch +the queue drain. + +**Cause.** Before each create, rename and trash, the drain asks whether a queued trash or rename +holds the name it wants, and a name that is let go wakes the ops waiting for it. Nothing indexed +the queue by name, so each of those questions read the whole queue. Draining a large offline +batch therefore took time quadratic in its size, under the database's write lock for the wake. + +**Fix.** Schema 43 adds an index on `pending_op(kind, name)`. The kind leads, so the lookup of +every queued rename uses it too. The `json_extract` on a rename's original name still reads the +renames, which are few. + +**Test:** `db::tests::queue_name_checks_use_an_index`, which checks the query plans. + +--- + ### B237 — Every reconnect resends refused uploads, all in the same second **Status:** Fixed, not verified live diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 91baeb9..f56578d 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -11,7 +11,8 @@ scratch (user data in `staging/` and `recovery/` is never touched by this). ## [Unreleased] -Schema: **42**, unchanged. +Schema: **43**. Queued changes get an index by kind and name. Nothing is rewritten, and the +upgrade takes a moment. ### Fixed - **A folder deleted on another device took files with it that had never reached Drive.** A new