mirror of
https://github.com/narrrl/proton-drive-linux.git
synced 2026-11-03 16:41:26 +01:00
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).
This commit is contained in:
1 parent
fcbb15cb4b
commit
937a0a6824
4 files changed
+73
-2
No files matched your search
@@ -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);
|
||||
";
|
||||
@@ -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::<Vec<_>>()
|
||||
.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.
|
||||
|
||||
@@ -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
|
||||
|
||||
+2
-1
@@ -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
|
||||
|
||||
Reference in new issue
Block a user