From 4cd49168c4836a138bbc279d0db590458803998c Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Sun, 16 Aug 2026 22:02:27 +0200 Subject: [PATCH] fix(drain) failed attempts --- crates/pdfs-fuse/src/drain.rs | 226 +++++++++-- crates/pdfs-fuse/src/lib.rs | 78 +++- docs/BUGS.md | 44 +- docs/plans/audit-2026-08-15-perf-locks.md | 474 ++++++++++++++++++++++ 4 files changed, 766 insertions(+), 56 deletions(-) create mode 100644 docs/plans/audit-2026-08-15-perf-locks.md diff --git a/crates/pdfs-fuse/src/drain.rs b/crates/pdfs-fuse/src/drain.rs index dab957a..5b9a416 100644 --- a/crates/pdfs-fuse/src/drain.rs +++ b/crates/pdfs-fuse/src/drain.rs @@ -41,9 +41,9 @@ use super::state::Intervals; use super::transfers::CountingReader; use super::{ Core, DRAIN_BACKOFF_MAX, DRAIN_BACKOFF_MIN, DRAIN_IDLE_POLL, DRAIN_REVISION_DEBOUNCE, - DRAIN_REVISION_DEBOUNCE_MAX, ROOT_INO, UPLOAD_TIME_MEMORY, conflict_name, is_already_exists, - is_gone, is_local_uid_str, media_type_for, node_revision_id, node_size, now_millis, now_secs, - parse_node_uid, + DRAIN_REVISION_DEBOUNCE_MAX, ROOT_INO, UPLOAD_TIME_MEMORY, WriteAuthority, conflict_name, + is_already_exists, is_gone, is_local_uid_str, media_type_for, node_revision_id, node_size, + now_millis, now_secs, parse_node_uid, }; const DRAIN_ACCESS_RECHECK: Duration = Duration::from_secs(5); @@ -53,14 +53,18 @@ const DRAIN_ACCESS_RECHECK: Duration = Duration::from_secs(5); /// /// An access deferral costs no attempt and records no error, which is right for /// the case it was written for: a share downgraded while the queue was moving, -/// undone a moment later. It is wrong for anything that does not clear — -/// notably a node the local tree no longer has a row for, which -/// [`Db::effective_node_access`] cannot distinguish from a revoked permission -/// and so reports as `EACCES` forever. Until this window existed such an op was -/// re-deferred every [`DRAIN_ACCESS_RECHECK`] indefinitely with `attempts` at -/// zero, `last_error` null and only a `debug!` line to show for it, so -/// `pdfs status` counted it among ordinary queued uploads and nothing ever said -/// otherwise. +/// undone a moment later. It is wrong for anything that does not clear. Until +/// this window existed such an op was re-deferred every +/// [`DRAIN_ACCESS_RECHECK`] indefinitely with `attempts` at zero, `last_error` +/// null and only a `debug!` line to show for it, so `pdfs status` counted it +/// among ordinary queued uploads and nothing ever said otherwise. +/// +/// The condition that produced that in practice — a node absent from the local +/// tree, which [`Db::effective_node_access`] used to report indistinguishably +/// from a revoked permission — is now answered directly by +/// [`Core::resolve_unknown_authority`]. This window remains the backstop for +/// every deferral that does not resolve, including a refetch that keeps not +/// helping. /// /// Five minutes is far longer than a permission change takes to settle and far /// shorter than a user's patience with bytes that are not moving. Past it each @@ -116,10 +120,52 @@ fn adaptive_debounce(measured: Option) -> Duration { .clamp(DRAIN_REVISION_DEBOUNCE, DRAIN_REVISION_DEBOUNCE_MAX) } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] enum DrainDisposition { Applied, AccessDeferred, + /// An authority this op needs is missing from the local tree, so no access + /// answer exists for it yet. Carries the uid to ask the remote about. + AuthorityUnknown(NodeUid), +} + +/// Why an op went back on the clock without consuming an attempt. Only affects +/// what the user is told — every variant is deferred and reported identically. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum AccessDeferral { + /// An authority the tree knows about refuses the write. + NotWritable, + /// An authority missing from the tree was just re-read from the remote and + /// re-interned; the access check gets another look on the next pass. + AuthorityRefetched, + /// An authority missing from the tree could not be asked about. + RemoteUnreachable, +} + +impl AccessDeferral { + fn log_reason(self) -> &'static str { + match self { + Self::NotWritable => "the node is not writable", + Self::AuthorityRefetched => "the node was missing locally and has been re-read", + Self::RemoteUnreachable => "the node is missing locally and the remote is unreachable", + } + } + + /// Phrased to complete " for 300s." in a recorded `last_error`, so + /// the text the user sees names the actual condition rather than the + /// permission question it used to be flattened into. + fn failure_reason(self) -> &'static str { + match self { + Self::NotWritable => "the share has not been writable", + Self::AuthorityRefetched => { + "the node has been missing from the local tree, and re-reading it from the remote \ + has not restored it" + } + Self::RemoteUnreachable => { + "the node has been missing from the local tree and the remote has been unreachable" + } + } + } } fn pending_op_authorities(op: &PendingOp) -> Result, Box> { @@ -162,7 +208,7 @@ fn pending_op_authorities(op: &PendingOp) -> Result, Box Result<(), fuser::Errno>, + mut authority_of: impl FnMut(&NodeUid) -> WriteAuthority, apply: impl FnOnce() -> Result<(), Box>, ) -> Result> { // This check is the admission/linearization point. A downgrade that lands @@ -170,15 +216,14 @@ fn run_authorized_drain( // attempt. Every later queued operation rechecks. No permission lock is // held across remote latency. for authority in pending_op_authorities(op)? { - match require(&authority) { - Ok(()) => {} - Err(error) if error.code() == libc::EACCES => { - return Ok(DrainDisposition::AccessDeferred); - } - Err(error) => { - return Err( - format!("drain access check failed with errno {}", error.code()).into(), - ); + match authority_of(&authority) { + WriteAuthority::Writable => {} + WriteAuthority::Denied => return Ok(DrainDisposition::AccessDeferred), + // Nothing in the tree can answer for this node. Deferring on it + // waits for a permission change that has no reason to come, so the + // caller asks the remote instead (B83). + WriteAuthority::Unknown => { + return Ok(DrainDisposition::AuthorityUnknown(authority)); } } } @@ -254,11 +299,15 @@ impl Core { }; let outcome = run_authorized_drain( &op, - |uid| self.require_uid_writable(uid), + |uid| self.uid_write_authority(uid), || self.drain_op(&op), ); if matches!(outcome, Ok(DrainDisposition::AccessDeferred)) { - self.defer_for_access(&op); + self.defer_for_access(&op, AccessDeferral::NotWritable); + continue; + } + if let Ok(DrainDisposition::AuthorityUnknown(ref authority)) = outcome { + self.resolve_unknown_authority(&op, authority); continue; } // The access check let this attempt through, so any earlier run of @@ -313,7 +362,7 @@ impl Core { /// /// [`Db::record_op_failure`]: pdfs_core::db::Db::record_op_failure /// [`FAILING_ATTEMPTS`]: pdfs_core::db::FAILING_ATTEMPTS - fn defer_for_access(&self, op: &PendingOp) { + fn defer_for_access(&self, op: &PendingOp, reason: AccessDeferral) { let now = now_millis(); let next_attempt_at = now + DRAIN_ACCESS_RECHECK.as_millis() as i64; let since = match self.db.defer_op_for_access(op.id, now, next_attempt_at) { @@ -337,7 +386,8 @@ impl Core { warn!( uid = %op.uid, kind = %op.kind, - "pending operation deferred: its node is not writable" + reason = reason.log_reason(), + "pending operation deferred" ); } if blocked >= DRAIN_ACCESS_DEFER_LIMIT { @@ -346,8 +396,8 @@ impl Core { .saturating_mul(1u32 << attempts.min(6)) .min(DRAIN_BACKOFF_MAX); let error = format!( - "not writable locally for {}s; the node is missing from the local tree or the \ - share is no longer writable. The staged bytes are kept.", + "{} for {}s. The staged bytes are kept.", + reason.failure_reason(), blocked.as_secs() ); warn!(uid = %op.uid, attempts, blocked_secs = blocked.as_secs(), @@ -363,12 +413,72 @@ impl Core { uid = %op.uid, next_attempt_at, blocked_secs = blocked.as_secs(), + reason = reason.log_reason(), "pending operation deferred until access is writable" ); } self.release_claim(op); } + /// Answer an authority the local tree has no row for by asking the remote, + /// rather than deferring on a permission question nobody asked (B83). + /// + /// Three outcomes, and the point of the whole exercise is that they are + /// three: + /// + /// * the node is still there — the tree simply lost it (a pruned subtree, a + /// mount whose registration went away). Re-intern it and the next pass has + /// a real access answer to work with. + /// * the node is gone remotely. That is a permanent, *known* condition, so + /// it is recorded as a failure immediately instead of waiting out + /// [`DRAIN_ACCESS_DEFER_LIMIT`] to say so. The row and its staged blob are + /// kept, as on every other failure path — the bytes are still the user's, + /// and `pdfs recover` is how they get them back. + /// * we could not ask. Ordinary deferral; the network is the network. + fn resolve_unknown_authority(&self, op: &PendingOp, authority: &NodeUid) { + match self.fetch_node_remote(authority) { + Ok(Some(node)) => { + if let Err(error) = self.db.upsert_node(&node) { + error!(uid = %op.uid, %authority, %error, + "re-interning a queued operation's missing authority failed"); + } else { + info!(uid = %op.uid, %authority, + "re-interned an authority missing from the local tree; retrying"); + } + // Deferred, not applied: the re-read has to go through the same + // access check as everything else, on the next pass. The window + // keeps running, so a refetch that never actually helps still + // gets reported instead of looping in silence. + self.defer_for_access(op, AccessDeferral::AuthorityRefetched); + } + Ok(None) => { + let attempts = op.attempts + 1; + let backoff = DRAIN_BACKOFF_MIN + .saturating_mul(1u32 << attempts.min(6)) + .min(DRAIN_BACKOFF_MAX); + let error = format!( + "{authority} no longer exists remotely, so this operation has nothing to \ + apply to. The staged bytes are kept." + ); + warn!(uid = %op.uid, %authority, attempts, + "a queued operation's authority is gone remotely; reporting it"); + if let Err(error) = self.db.record_op_failure( + op.id, + &error, + now_millis() + backoff.as_millis() as i64, + ) { + error!(uid = %op.uid, %error, "recording a missing-authority failure failed"); + } + self.release_claim(op); + } + Err(error) => { + debug!(uid = %op.uid, %authority, %error, + "could not ask the remote about a missing authority"); + self.defer_for_access(op, AccessDeferral::RemoteUnreachable); + } + } + } + /// Hand a claimed op back to the queue, logging rather than propagating — /// there is no caller left to propagate to, and the drain must not stop. fn release_claim(&self, op: &PendingOp) { @@ -1512,12 +1622,10 @@ mod tests { let applied = Cell::new(false); let disposition = run_authorized_drain( &op, - |uid| { - db.effective_node_access(uid) - .unwrap() - .filter(|access| access.writable()) - .map(|_| ()) - .ok_or(fuser::Errno::EACCES) + |uid| match db.effective_node_access(uid).unwrap() { + Some(access) if access.writable() => WriteAuthority::Writable, + Some(_) => WriteAuthority::Denied, + None => WriteAuthority::Unknown, }, || { applied.set(true); @@ -1557,7 +1665,7 @@ mod tests { let disposition = run_authorized_drain( &op, - |_| Err(fuser::Errno::EACCES), + |_| WriteAuthority::Denied, || { applied.set(true); Ok(()) @@ -1676,13 +1784,59 @@ mod tests { let _ = std::fs::remove_dir_all(dir); } + #[test] + fn an_authority_the_tree_does_not_have_is_reported_as_unknown_not_deferred() { + // The B83 distinction, at the point it is made: a uid with no row must + // not come back as `AccessDeferred`, because deferring waits on a + // permission change that nothing is going to make. The caller answers + // an `AuthorityUnknown` by asking the remote instead. + let op = pending(OP_REVISION); + let applied = Cell::new(false); + let disposition = run_authorized_drain( + &op, + |_| WriteAuthority::Unknown, + || { + applied.set(true); + Ok(()) + }, + ) + .unwrap(); + assert_eq!( + disposition, + DrainDisposition::AuthorityUnknown(parse_node_uid(&op.uid).unwrap()) + ); + assert!(!applied.get(), "an unresolved authority never applies"); + } + + #[test] + fn a_missing_parent_authority_names_the_parent_not_the_node() { + // A create's authority is its parent, so that is the uid the remote has + // to be asked about — reporting the node instead would refetch the + // thing that does not exist yet. + let mut op = pending(OP_CREATE); + let parent = folder_node("vanished-parent", None); + op.parent_uid = Some(parent.uid.to_string()); + let disposition = + run_authorized_drain(&op, |_| WriteAuthority::Unknown, || Ok(())).unwrap(); + assert_eq!( + disposition, + DrainDisposition::AuthorityUnknown(parent.uid.clone()) + ); + } + #[test] fn owner_and_editor_drain_authorities_apply_normally() { for access in [Access::Owner, Access::Editor] { let applied = Cell::new(false); let disposition = run_authorized_drain( &pending(OP_REVISION), - |_| access.writable().then_some(()).ok_or(fuser::Errno::EACCES), + |_| { + if access.writable() { + WriteAuthority::Writable + } else { + WriteAuthority::Denied + } + }, || { applied.set(true); Ok(()) diff --git a/crates/pdfs-fuse/src/lib.rs b/crates/pdfs-fuse/src/lib.rs index ea52976..992a594 100644 --- a/crates/pdfs-fuse/src/lib.rs +++ b/crates/pdfs-fuse/src/lib.rs @@ -973,6 +973,27 @@ impl Core { /// downgrade cannot be hidden by stale live state, and a live downgrade /// cannot be hidden by a restored DB row. Missing UIDs fail closed. fn require_uid_writable(&self, uid: &NodeUid) -> Result<(), Errno> { + match self.uid_write_authority(uid) { + WriteAuthority::Writable => Ok(()), + // A uid nobody can speak for is refused here just as a denied one + // is: a syscall is a live request against a live tree, and a stale + // handle must not be admitted because the tree forgot the node. + // The drain, which replays intent recorded long ago, is the one + // caller that has to tell the two apart — see + // [`Core::uid_write_authority`]. + WriteAuthority::Denied | WriteAuthority::Unknown => Err(Errno::EACCES), + } + } + + /// The full answer behind [`Core::require_uid_writable`]: whether every + /// authority agrees this node is writable, refuses it, or has never heard + /// of it. + /// + /// The third case is the one worth naming. A node absent from the local + /// tree yields no access row, and collapsing that into "denied" told the + /// drain to wait for a permission change that was never coming, because + /// nothing about the node's permissions was ever the problem (B83). + pub(crate) fn uid_write_authority(&self, uid: &NodeUid) -> WriteAuthority { let mut live_access = Vec::new(); { let state = self.state(); @@ -988,22 +1009,38 @@ impl Core { live_access.push(access); } } - require_uid_access(&self.db, uid, &live_access) + uid_write_authority(&self.db, uid, &live_access) } } -fn require_uid_access(db: &Db, uid: &NodeUid, live_access: &[Access]) -> Result<(), Errno> { +/// Whether a node may be written, as agreed by every authority that has an +/// opinion — or the fact that none of them does. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum WriteAuthority { + /// The persisted tree and every resident mount agree: writable. + Writable, + /// At least one authority refuses it, or could not be consulted. Fails + /// closed, so a lookup error lands here. + Denied, + /// The node is not in the local tree, so there is no authority to consult. + /// Not the same as a refusal, and callers that can ask the remote should. + Unknown, +} + +fn uid_write_authority(db: &Db, uid: &NodeUid, live_access: &[Access]) -> WriteAuthority { let persisted = match db.effective_node_access(uid) { Ok(Some(access)) => access, - Ok(None) => return Err(Errno::EACCES), + Ok(None) => return WriteAuthority::Unknown, Err(error) => { error!(%uid, %error, "db effective access lookup failed"); - return Err(Errno::EACCES); + return WriteAuthority::Denied; } }; - (persisted.writable() && live_access.iter().all(|access| access.writable())) - .then_some(()) - .ok_or(Errno::EACCES) + if persisted.writable() && live_access.iter().all(|access| access.writable()) { + WriteAuthority::Writable + } else { + WriteAuthority::Denied + } } /// One kernel notification, recorded while the State lock is held so it can be @@ -5754,9 +5791,19 @@ mod tests { preserve_on_access_denied, publish_virtual_root_in_listing, reconcile_virtual_root_in_listing, release_can_discard_unlinked, release_must_retain_queued_trash, release_unlinked_entry, rename_needs_queue, - require_node_parent_access, require_rename_access, require_uid_access, - resolve_anywhere_with, shared_with_me_uid, take_self_change, virtual_node, + require_node_parent_access, require_rename_access, resolve_anywhere_with, + shared_with_me_uid, take_self_change, uid_write_authority, virtual_node, }; + use super::{Db, WriteAuthority}; + + /// The collapse [`Core::require_uid_writable`] performs, without a `Core`: + /// every non-writable authority is one `EACCES` on the syscall path. + fn require_uid_access(db: &Db, uid: &NodeUid, live: &[Access]) -> Result<(), Errno> { + match uid_write_authority(db, uid, live) { + WriteAuthority::Writable => Ok(()), + WriteAuthority::Denied | WriteAuthority::Unknown => Err(Errno::EACCES), + } + } use crate::filesystem::access_allowed; use pdfs_core::cache::{Baseline, StagedWrite}; use pdfs_core::db::{OP_REVISION, OP_TRASH, PendingOp}; @@ -6763,6 +6810,19 @@ mod tests { require_uid_access(&db, &stale, &[]).unwrap_err().code(), libc::EACCES ); + // Same refusal on the syscall path, but a distinguishable one + // underneath: the drain asks the remote about `Unknown` rather than + // waiting on a permission change (B83). + assert_eq!( + uid_write_authority(&db, &stale, &[]), + WriteAuthority::Unknown, + "a uid the tree has no row for is unknown, not denied" + ); + assert_eq!( + uid_write_authority(&db, &uid, &[]), + WriteAuthority::Denied, + "a uid the tree knows and refuses is denied" + ); assert_eq!(db.pending_ops().unwrap().len(), before); } diff --git a/docs/BUGS.md b/docs/BUGS.md index e04186e..403796f 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -14,9 +14,10 @@ Conventions: ## B83 — An access-deferred op retries forever and is reported as an ordinary queued upload -**Status:** Partly fixed (unverified) 2026-08-16 — the reporting half is in and -unit-tested; the cause (an unknown node reading as `EACCES`) is still open, see -"Still open" below. +**Status:** Fixed (partly verified) 2026-08-16 — both halves are in and +unit-tested. The reporting half was confirmed against the live 28-op backlog on +1.8.0 (schema migrated to v27, all 28 rows window-stamped, `WARN` lines per op); +the resolution half has not yet been observed draining that backlog. **Found:** 2026-08-16, running `scripts/fuse-acceptance.sh --live` on a throwaway CLI-made mount. Four regression cases (B69, B70, B74, B79) failed with `TimeoutError: daemon mutation queue did not drain` — the suite's @@ -25,7 +26,7 @@ been sitting at 28 for 30 days on the development machine. `pdfs status` called them 28 queued uploads holding 18.4 GiB; `pdfs transfers` showed nothing; `failing_ops` and `parked_uploads` were both 0. **Where:** `run_pending_drain` / `run_authorized_drain` -(`crates/pdfs-fuse/src/drain.rs`), `require_uid_access` +(`crates/pdfs-fuse/src/drain.rs`), `require_uid_writable` (`crates/pdfs-fuse/src/lib.rs`), `Db::effective_node_access` (`crates/pdfs-core/src/db/share_access.rs`), `Db::defer_op_without_attempt` (`crates/pdfs-core/src/db/ops.rs`). @@ -48,7 +49,26 @@ select (select count(*) from pending_op p -- 0 | 28 ``` -**Fix (this half).** Schema v27 adds `pending_op.access_deferred_since`. +**Fix, part 1 — stop guessing.** `WriteAuthority { Writable, Denied, Unknown }` +replaces the `Result<(), Errno>` the drain's access check used to return +(`Core::uid_write_authority`). `require_uid_writable` still collapses both +refusals to `EACCES`, which is right for a syscall: a stale handle must not be +admitted because the tree forgot the node. The drain does not collapse them — +`Unknown` becomes `DrainDisposition::AuthorityUnknown(uid)`, and +`Core::resolve_unknown_authority` asks the remote: + +- node still there → `upsert_node` re-interns it and the op is deferred for one + recheck, so the next pass gets a real access answer; +- node gone (`Ok(None)` / `is_gone`) → `record_op_failure` **immediately**, with + a message naming the vanished uid, rather than waiting out the 5-minute window + to say something vaguer; +- could not ask → ordinary deferral. + +The authority asked about is the op's own (`pending_op_authorities`), so a +`create`/`mkdir` refetches its *parent*, not the node it has not made yet. The +row and its staged blob survive every branch. + +**Fix, part 2 — say something.** Schema v27 adds `pending_op.access_deferred_since`. `Db::defer_op_for_access` stamps it on the first deferral of a run and returns the stamp on every one; `Core::defer_for_access` warns on the first deferral and, once the run passes `DRAIN_ACCESS_DEFER_LIMIT` (5 min), reports it through @@ -59,12 +79,14 @@ access check admits clears it too (`clear_op_access_deferral`). The row and its staged blob are untouched on every path — this changes only what the user can see. -**Still open:** the conflation itself. A revision op whose node has left the -local tree should refetch the node and either proceed or fail on a real remote -answer (404/trashed), not be deferred on a local guess. Same trap applies to -`create`/`mkdir`, whose authority is the parent uid. Also unaddressed: the -acceptance runner waits on the *global* queue, so any unrelated stuck op fails -those four cases on a machine that has one. +The deferral window stays as the backstop for everything that does not resolve, +including a refetch that keeps not helping, and `AccessDeferral` now carries the +reason into both the log line and the recorded `last_error`. + +**Still open:** the acceptance runner waits on the *global* queue +(`ManagedSyncPair.wait_for_queue`), so any unrelated stuck op fails those four +cases on a machine that has one — that is what turned this backlog into four +apparent FUSE regressions. ## B82 — A `parent_uid` cycle hangs the daemon on any short search diff --git a/docs/plans/audit-2026-08-15-perf-locks.md b/docs/plans/audit-2026-08-15-perf-locks.md new file mode 100644 index 0000000..e2e351e --- /dev/null +++ b/docs/plans/audit-2026-08-15-perf-locks.md @@ -0,0 +1,474 @@ +# Architecture audit — performance, locking, robustness (2026-08-15) + +Scope: `pdfs-fuse`, `pdfs-core`, and `proton-sdk-rs` as consumed by the mount. +Method: five parallel deep-read passes (locks, read path, write path, DB/IPC, SDK +transport), each finding re-verified against the tree before landing here. + +Line numbers are as of commit `b37ab78`. + +--- + +## Implementation status (2026-08-15, uncommitted working tree) + +| Landed | Item | +|---|---| +| ✅ | DB5 depth-capped `is_pinned`, with a cycle regression test | +| ✅ | DB6 daemon `set_write_timeout`; every client-supplied `limit` clamped | +| ✅ | T7 `"http2"` added to the SDK's reqwest features (`h2` now in its lock file) | +| ✅ | D1 sweep no longer discards queued ops after the trash round trip | +| ✅ | D2 `path == blob` guard shared by all three pending-entry drops (`Core::release_pending`) | +| ✅ | D3 `ENOSPC` propagated from `write`/`fsync`/staging, `emergency_evict` wired up, scratch marked durable before any staging error | +| ✅ | D4 `record_pending_write` returns its failure; `release` fails with `ENOSPC` | +| ✅ | D5 `rescue_scratch` moves a blob with an unreadable sidecar to `recovery/` unaccompanied instead of fabricating one | +| ✅ | F2 `open` write path on `Lane::Transfer`, staged blob duplicated by reflink (`ContentCache::clone_or_copy`) | +| ✅ | F3 `fsync` on `Lane::Transfer` | +| ✅ | F5 mount registry lock released before FUSE teardown + join | +| ✅ | F6 kernel notifications batched under the State lock, flushed after it (`NotifyBatch`) | +| 🟡 | T6 `Filesystem::init` negotiates `max_readahead`/`max_background`/`PARALLEL_DIROPS`/`CACHE_SYMLINKS`; `blksize` now 1 MiB; `statfs` implemented against the account quota. **`n_threads`/`clone_fd`, `ATOMIC_O_TRUNC`, `READDIRPLUS` and `FOPEN_KEEP_CACHE` deliberately deferred** — each needs a handler or ordering change first (see `spawn_session`) | +| ✅ | T4 LRU touches buffered in memory, flushed in one transaction (and before every eviction pass) | +| ✅ | T5 no fsync per cached block; `cached_block` validates by length instead | +| 🟡 | DB8 three of the four indexes. `idx_pending_op_due` is **not** wanted: `next_due_op` orders by rowid and already exits on the first row, and offering the planner a `next_attempt_at` index makes it sort — `next_due_op_does_not_scale_with_queue_length` measures the regression. `activity_time` dropped | +| ✅ | DB9 `cache_size`, `mmap_size`, `temp_store=MEMORY`, `wal_autocheckpoint` | + +Second batch (uncommitted on top of `e396c6b`): + +| Landed | Item | +|---|---| +| ✅ | DB10 `flock(LOCK_EX\|LOCK_NB)` single-writer lock in `Db::open`; a `SQLITE_CORRUPT`/`NOTADB` file is quarantined (`cache.db.corrupt-`, WAL sidecars too) and rebuilt | +| ✅ | DB7 `VACUUM` and the deep `integrity_check` ack-then-background (`Core::start_vacuum`, `Core::start_or_read_integrity_check`, visible as jobs); `Response::CacheReport` gained `integrity_running`, `pdfs doctor` polls for the verdict | +| ✅ | DB7 `local_finish_scan` no longer rebuilds the whole FTS index: `local_upsert_batch` maintains `local_fts` incrementally (valid because `path` is the key and `name` derives from it), migration V23 does one last rebuild | +| ✅ | §5 `recover_fsynced_writes` no longer `read_dir`s per idle pass (`ContentCache::recovery_pending`) | +| ✅ | §5 `own_sealed_revs` persisted (migration V24, `own_sealed_rev` table, 7-day TTL) — a restart no longer reopens the B70-layer-B fork window | +| ✅ | §5 staging reconcile at mount (`Core::reconcile_staging`): staged blobs no queued op refers to are re-queued, or named in the log when they cannot be addressed | +| ✅ | §5 queue/staging reporting — `Response::Status` gained `parked_uploads`, `failing_ops`, `failing_error`, `staged_bytes`, `staged_oldest_secs`; `pdfs status` prints them when non-zero | + +Third batch (uncommitted on top of `7ad9231`): + +| Landed | Item | +|---|---| +| ✅ | F1 `queue_revision` no longer gap-fills from the network. It stages an incomplete blob (`fill_gaps_cached` resolves only what the local caches already hold, capped at 64 MiB) and `drain_revision` finishes it off the dispatch loop | +| ✅ | F1 a partial write over an undrained one is merged instead of refused: `merge_over_pending` folds the superseded blob's authored ranges in and inherits its `base_mtime`/`base_size`/`based_on`. `release` keeps its inherited ordering, which is now affordable because only local I/O is left on it | +| ✅ | F4 size-upgrade waiters no longer occupy a pool thread: `getattr`/`lookup` park their `Reply` on `SizeWaitQueue`, served by one thread that exits when nothing is parked. `getattr` no longer needs a `Lane::Meta` job at all | +| ✅ | F4 upgrade threads bounded by `MAX_SIZE_UPGRADES` (8 folders in flight); past the cap a `stat` answers provisionally, the same fallback the deadline takes | + +Corrected while implementing: the audit's claim that F1's `fill_gaps` bypasses +the block cache is wrong — it calls `read_range_remote` with `cache_blocks = +true`, which consults the whole-file blob and then the block cache before the +network. What made `release` slow was going to the network at all, not doing so +uncachedly. + +Fourth batch (uncommitted, same working tree): + +| Landed | Item | +|---|---| +| ✅ | T3 the in-memory ring (`StreamRing` → `BlockRing`, now a true LRU) sits in front of **both** block paths, not just the disk-bypassing stream. A block read off disk is promoted into it, so the other ~31 kernel reads of that block cost nothing | +| ✅ | T1 `BlockFlight`: `(uid, mtime, size, bidx)` single-flight over block fetches, so demand read, kernel read-ahead and prefetch share one download+decrypt. The leader's claim is a drop guard, so a cancelled `JoinSet` leaves no key nobody will publish; a follower whose leader vanished fetches for itself | +| ✅ | T2 `Core::prefetch_ahead` on both paths: per-file sequential detection (`Prefetch::window`), depth 2→8 doubling per sequential read, 0 after a seek, skipping blocks already in the ring or on disk (`ContentCache::has_block`), bounded by its own `PREFETCH_BUDGET` permits with `try_acquire_owned` so it never queues ahead of a demand read | +| ✅ | The block-cache disk write moved off the read's critical path (`spawn_blocking` inside `read_block`) | + +Corrected while implementing: T3's premise is half stale — `ContentCache::read_range` +already `pread`s only the requested window, and T4/T5 removed the per-read SQLite +write and the fsync. What remained, and is what the ring fixes, is `cached_block` +reading and allocating the whole 4 MiB for each 128 KiB the kernel asks for. +T1's fix is client-side rather than in the SDK's `RevisionReader` as the audit +suggested: the SDK ships from a separate repo through crates.io, and the flight +map needs the client's `(mtime, size)` validity pair to be safe across revisions. + +Fifth batch (uncommitted on top of `e48c4bb`): + +| Landed | Item | +|---|---| +| ✅ | DB3 `nodes.path` is persisted (migration V25, backfilled by the same walk V21 used). `path_of` reads a column; `walk_path_of` stays as the fallback for a row written before it existed | +| ✅ | DB3 a folder upsert whose name, parent, path and trashed flag are all unchanged — i.e. every re-listing — no longer touches its subtree at all. `re_listing_a_folder_does_not_scale_with_its_subtree` measures it | +| ✅ | DB3 when the subtree *has* moved, one recursive walk carries both the new path and whether anything above is trashed, so a descendant costs two trivial statements instead of a `path_of` walk plus an ancestor walk of its own (`reindex_subtree_tx`) | +| ✅ | DB4 `sync_folder_list` and the pin lookup are hoisted out of the per-candidate loop (`Core::search_roots`, `Core::pinned_set`); `SearchRoots::resolve` maps a hit to its mountpoint by string arithmetic over the path the index already carries, with no query at all | +| ✅ | DB2 a pool of read-only connections (`ReadPool`, `Db::read`), and every `SELECT`-only method routed at it. `a_read_does_not_wait_for_the_write_connection` pins the property | +| ✅ | DB1 `State` no longer writes to SQLite. Mutations queue a `DbWrite` in an outbox that `StateGuard` applies **after** releasing the inode lock — so interning a 5000-child listing no longer commits a transaction with the whole mount frozen | + +Corrected while implementing: DB4's "filter by score *before* decorating" was +already true — `relevance_score(…)?` short-circuits ahead of the `SearchHit` +construction. What actually cost the keystroke was `sync_folder_list` plus a +`path_relative_to` walk per (hit, sync folder) pair, and `is_pinned`'s ancestor +CTE per hit. DB3's suggested `path` column is here, but its companion suggestion +to skip the reindex on an unchanged name/parent needed the trashed flag in the +comparison too: a folder being trashed has to take its subtree out of the index, +and the old code only did that by accident, through reindexing unconditionally. + +DB1 is done for writes, not reads: `State::has_children` still reads the DB +under the inode lock. That read now goes to the pool, so it no longer contends +with a writer, and moving it would mean changing what `rmdir` is allowed to +assume. + +Sixth batch (on top of `433e206`): + +| Landed | Item | +|---|---| +| ✅ | §5 parallel drain — `DRAIN_WORKERS = 3` threads over one queue, sharing it through `pending_op.claimed_at` (migration V26) and `Db::claim_next_due_op`. Select-and-claim is one transaction, and the query excludes any op whose uid another worker holds, so ordering still holds per node without holding it globally. Worker 0 keeps the idle chores. Stale claims are cleared in `Db::open`, which the single-writer `flock` makes sound | +| ✅ | §5 adaptive debounce — `Core::revision_debounce` widens the grace period toward the node's last measured upload wall-time, clamped to `[2s, 60s]`; only a *completed* upload counts as a measurement, so a fault cannot widen it permanently | +| ✅ | §5 cancel on supersede — `Core::cancel_upload` sets a per-node flag that the upload's `CountingReader` refuses the SDK's next block on. Signalled by `enqueue_staged_write`, `queue_trash` and `discard_queued_ops`, i.e. every path that either replaces those bytes or unlinks them. A cancelled upload returns `Ok` rather than a retry: its row and its blob already belong to the write that cancelled it | + +Corrected while implementing: the audit's "a claim column and N=3–4 workers" needed +one thing it does not mention — with several workers, a supersede or a trash can +now delete a row that is *in flight*, which single-threading used to make +impossible. That is what makes the cancellation flag part of the parallel-drain +change rather than a separate throughput nicety. + +Seventh batch — T8, in `proton-sdk-rs` (uncommitted there; reaches this client +only through a crates.io publish, so nothing here changes until that lands): + +| Landed | Item | +|---|---| +| ✅ | T8 token lock — `Inner::tokens` is a `std::sync::RwLock>` read per request, with a separate `Inner::refresh` async mutex held across the refresh call. An ordinary request no longer queues behind an in-flight `auth/v4/refresh`; `an_ordinary_request_does_not_wait_for_an_in_flight_refresh` pins it against a deliberately slow loopback refresh | +| ✅ | T8 crypto off the reactor — the three remaining inline spots are on the blocking pool: `create_public_link`'s bcrypt + SRP verifier, `decrypt_invitation_name`'s share-key S2K, and the public-link handshake's `generate_proofs` + `unlock_share_key` (`public_link.rs::blocking`) | +| ✅ | T8 `list_blocks` pipelining — pages after the first are fetched concurrently in a window ramping 2→6, page offsets derived positionally (which the existing contiguity check already requires). A 4 GiB file is ~6 round trips instead of 21 | +| ✅ | T8 one 4 MiB copy per block removed — `ContentKey::decrypt_block` returns `Bytes` via `LiteralData::into_bytes` instead of `data().to_vec()`; `decrypt_block_blocking`, `digest_and_decrypt_block_blocking` and `RevisionReader::block_plaintext` carry `Bytes` through | +| n/a | T8's last bullet is client-side only: the SDK already exposes `RevisionReader::block_sizes()`, so consulting it instead of the hardcoded `BLOCK_SIZE = 1 << 22` is a change in this repo, not that one | + +Note: `decrypt_block` returning `Bytes` is a breaking change to the SDK's public +crypto surface, so the publish that carries T8 needs the version bump to match. + +--- + +## 0. The one-sentence version + +The subsystem designs are sound and unusually well documented; the defects are +almost all **placement** — correct work happening on the wrong thread, under the +wrong lock, or at the wrong granularity. Three exceptions destroy user bytes. + +Two structural facts explain most of the severity: + +1. **fuser's session loop is non-concurrent** and `Config::n_threads` is unset → + `unwrap_or(1)` (`fuser-0.17.0/src/session.rs:254`). One thread reads + `/dev/fuse`. Anything a handler does inline stalls *every* request on the + mount, including the 11 idle workers, which have nothing to receive. +2. **One `Mutex`** (`pdfs-core/src/db/mod.rs:73`) for the whole + daemon, and `State` (`lib.rs:305`) is one mutex over the entire inode space — + and `State`'s methods write through to the DB *while the caller holds it*. + The `State → Db` edge turns a microsecond lock into a multi-second one. + +Fixing (1) and (2) is worth more than every micro-optimisation below combined. + +--- + +## 1. Data loss — fix first + +### D1. Sweep deletes staged blobs after a network round trip +`sweep.rs:169` — `discard_queued_ops` (`lib.rs:2610`) deletes the op row *and* +unlinks the staged blob, and runs *after* the `trash_nodes` call at +`sweep.rs:150-153`. The B71 interlock `duplicate_still_removable` +(`sweep.rs:193`) runs *before* it. A `close(2)` landing in that window stages +bytes that the sweep then destroys. Only survives today because `SweepMode` +defaults to report-only. + +Fix: drop `discard_queued_ops` from `remove_conflict_copy` entirely. Unhook +locally and let the drain discover the node is trashed — `revision_conflict` +(`drain.rs:615`) already routes that to `keep_as_conflict_copy`, the correct +non-destructive outcome. + +### D2. Conflict paths evict a `pending` entry that may belong to a newer write +`drain.rs:713` (`keep_as_conflict_copy`), `drain.rs:751` (`abandon_to_staging`) +both do `self.pending.lock().remove(uid)` unconditionally. `drain_revision` +deliberately does not — `drain.rs:890` removes only when `op_in_mem.path == +blob`, precisely because a supersede may have replaced the entry mid-flight. + +Consequences of the missing guard: reads fall back to a stale remote; the next +`remote_baseline` (`lib.rs:2195`) forks a spurious `(sync-conflict)` copy; and +the incomplete-write interlock at `lib.rs:2246` is silently disarmed, letting an +incomplete write gap-fill from a revision that no longer describes the file. + +Fix: copy `drain_revision`'s `path == blob` guard into both sites. + +### D3. No ENOSPC path anywhere +`cache.rs:891` `emergency_evict` has **zero callers** (verified by grep across +the workspace). `write(2)` returns `EIO`, not `ENOSPC` (`filesystem.rs:379`). +Worse: `stage_write` failing on a full disk leaves the scratch file with no +durability sidecar, so `discard_unmarked_scratch` deletes the only copy at next +mount — the one place the "never delete on a failure path" invariant breaks. + +Fix: call `emergency_evict` on `StorageFull`, propagate `ENOSPC`, and +`mark_scratch_durable` before returning any staging error. + +### D4. `record_pending_write` failure is only a `warn!` +`state.rs:925-931`. Its own doc comment (`:918-923`) says this row is the only +record that the file is as long as the caller was told. On ENOSPC the daemon +acknowledges the write and loses the size. Fix: fail the `release` with `ENOSPC`. + +### D5. `rescue_scratch` fabricates a sidecar for a corrupt one +`cache.rs:836-861` invents `uid: "recovered~"`, `complete: true`. That uid +parses, so `recover_fsynced_writes` enqueues an op against a nonexistent node +that retries forever, and `complete: true` is a lie that stops any gap-fill. +Fix: move the blob to `recovery/` *without* a sidecar — `recovered_writes` +already retains sidecar-less blobs for a human, which is the right outcome. + +--- + +## 2. Whole-mount freezes (single dispatch thread) + +### F1. `release(2)` gap-fills over the network, inline +`filesystem.rs:687` → `lib.rs:2113` → `reads.rs:616` → `rt.block_on`. Closing a +2 GB file after a 4 KB edit downloads ~500 blocks with the dispatch loop stopped: +no `stat`, no `ls`, no `open`, no `read` anywhere on the mount. The comment at +`filesystem.rs:674-686` already concedes the placement is wrong. + +Fix: the staged blob is allowed to be incomplete (`complete: false` is +first-class, and `drain_revision` at `drain.rs:851-869` already gap-fills off the +loop). Skip `fill_gaps` in `queue_revision` for partial writes; add the per-node +staging-in-flight barrier that `read`/`open` wait on, then move `finish_release` +to `Lane::Transfer`. Also: `fill_gaps` uses `read_range_remote`, bypassing the +block cache — a cache-aware fill makes editing a recently-read file ~free. + +### F2. `open(2)` copies the whole staged blob, inline +`filesystem.rs:180` `std::fs::copy` of a possibly multi-GB pending blob, on the +dispatch thread. Fix: reflink (`FICLONE` → `copy_file_range` → `fs::copy`), and +route the write-open path through `Lane::Transfer`. + +### F3. `fsync(2)` runs `sync_all` + sidecar write inline +`filesystem.rs:602`, `:641`. Apps that fsync per record stall the mount at their +barrier rate. No ordering constraint — the kernel waits for the reply either way. +Fix: `Lane::Transfer`. + +### F4. Size-upgrade waiters occupy every worker in both lanes for 10 s +`filesystem.rs:75-88`, `:1069-1077` queue a `Lane::Meta` job that blocks on a +condvar up to `SizeUpgrade::WAIT = 10s` (`lib.rs:4292`). This defeats the premise +of the lane split (`workers.rs:39-52`: "metadata jobs are short"). Single-flight +is per folder, so `ls -l` across N cold folders fills all 11 threads. Also +`lib.rs:1801` spawns an unbounded thread per folder. + +Fix: register the `Reply` on a completion list instead of blocking a worker; bound +the upgrade threads. + +### F5. `mounts` lock held across FUSE teardown + thread join +`devices.rs:539`, `devices.rs:870`. Edition-2024 if-let chains keep the +`MutexGuard` alive through the body, so the lock spans `abort_fuse_connection` + +`umount_and_join`. If the secondary session is inside F1's gap-fill, the join +waits it out while the shutdown path blocks on the same lock → systemd SIGKILLs +mid-unmount, stranding the endpoint that `abort_fuse_connection` exists to avoid. +Fix: `let taken = self.mounts.lock().remove(&id);` then drop before teardown. + +### F6. Notifier called while holding the State lock +`lib.rs:706-710` `for_each_mount`. `for_each_state` (`:689`) and +`flush_access_changes` (`:715`) both document and honour the opposite rule. +`inval_inode` → `invalidate_inode_pages2` can block on pages that this daemon's +own workers must fill — and those workers are blocked on the State lock the +notifier holds. `background.rs:226-234` walks every cached directory this way in +one hold. Fix: collect inodes under the lock, notify after dropping it. + +--- + +## 3. Throughput + +### T1. No single-flight on block fetches — ~2× bandwidth and CPU, steady state +`reads.rs:475-526`. Nothing registers "(uid, mtime, size, bidx) in flight". With +`FUSE_ASYNC_READ` on, the demand read and the kernel's readahead land in the same +4 MiB block for 31 of every 32 reads, and both fetch and decrypt it in full. + +The codebase already has the pattern (`ReaderSlot::Pending`, `reads.rs:164-277`) +and the SDK has `SingleFlight` used for node/parent-key loads. Best fixed in the +SDK's `RevisionReader` so every consumer benefits. + +### T2. No prefetch for anything under 256 MiB +`reads.rs:541` calls `stream_readahead` only when `!cache_blocks`. Everything +else is a strictly serial block chain: fetch, serve 32 kernel reads, stall. +Derived ceiling on a 100 ms RTT link ≈ 24 MB/s against ~125 MB/s available. The +SDK's 10-way block concurrency is never exercised. + +Fix: per-(ino, fh) sequential detection, prefetch depth ramping 2→8, reset on a +non-sequential offset. Give prefetch its own permit budget or `try_acquire` so it +never delays a demand read (SDK's semaphore is flat, `client.rs:126`). + +### T3. Warm cached reads re-read and re-allocate 4 MiB per 128 KiB +`cache.rs:570-585`: per kernel read, 2 opens + a JSON parse + a SQLite write txn ++ a 4 MiB pread + a 4 MiB alloc. 128 MiB of traffic to deliver 4 MiB. ~60 MB/s +ceiling on a fully cached file. + +Fix: serve the disk-cache path through an in-memory block LRU (the `StreamRing` +shape, currently used only when `cache_blocks == false`); fold `Meta` into the +block filename to kill the sidecar read; `pread` only the requested window. + +### T4. A SQLite write on the read hot path +`cache.rs:581` `cache_accessed` = `UPDATE cache_entries SET last_accessed` under +the global connection mutex, 32× per 4 MiB read, contending with every `lookup` +and `readdir`. Reintroduces one layer down exactly the contention the `Lane::Meta` +reservation exists to prevent. Also `filesystem.rs:297` `is_pinned` per read. + +Fix: batch LRU touches in memory, flush periodically; cache the pin bit. + +### T5. `fsync` per cached block on the cold path +`cache.rs:605` `sync_all()` inside `store_block`, inline on the thread the kernel +read is waiting on. The cache is disposable and `cached_block` doesn't even +validate length. Fix: drop the fsync, validate by length like `valid_blob` does. + +### T6. Kernel/mount caps left at fuser defaults +No `Filesystem::init` impl at all. `mount.rs:257-273` sets only +`FSName`/`Subtype`/`DefaultPermissions`. + +| Knob | Now | Should be | +|---|---|---| +| `n_threads` / `clone_fd` | 1 / false | 4 / true (`FUSE_DEV_IOC_CLONE`, Linux 4.5+) | +| `max_background` | 16 | 64 | +| `max_readahead` | 128 KiB | 4 MiB (match `BLOCK_SIZE`) | +| `FUSE_PARALLEL_DIROPS` | off | on — kernel serialises concurrent lookups per dir | +| `FUSE_DO_READDIRPLUS` + `_AUTO` | off, no `readdirplus` impl | on — `ls -l` is readdir + N lookups today | +| `FUSE_ATOMIC_O_TRUNC`, `CACHE_SYMLINKS` | off | on | +| `FOPEN_KEEP_CACHE` | never set (`filesystem.rs:145`, `:214`) | set — the event poller already does the `inval_inode` this requires | +| `attr.blksize` | `512` (`filesystem.rs:1149`) | `1 << 20` — coreutils size their buffers from it | +| `statfs` | not implemented | implement — `df` and free-space preflights see garbage | + +### T7. HTTP/2 is compiled out of the SDK +`proton-sdk-rs/Cargo.toml:27` uses `default-features = false` and omits +`http2`, which is in reqwest 0.13's default set. Verified: `h2` appears **zero** +times in `Cargo.lock`. Every concurrent block download pays its own TCP+TLS +handshake instead of multiplexing. **One-line fix, add `"http2"`.** + +### T8. Other SDK hot spots +- Token mutex held across the refresh network call (`http.rs:503-524`); every + ordinary request takes the same lock just to *read* the token. Worst case ~2 min + of total API stall. Fix: `ArcSwap` for reads, separate refresh mutex. +- Three inline S2K/PGP decrypts on the reactor (`client.rs:1376-1378`, `:1281`, + `:6092`) — tens of ms each, while the rest of the crate correctly uses + `crypto::blocking`. +- `list_blocks` pages 50 at a time, sequentially (`transport.rs:152`): a 4 GiB + file is 21 serial round trips before the first byte. +- ~3 full 4 MiB copies per block delivered (`content.rs:350-366`, `revision.rs:361`). +- Client hardcodes `BLOCK_SIZE = 1 << 22` instead of consulting the SDK's + `block_sizes()`; any revision with a different geometry doubles fetch count. + +--- + +## 4. Database and IPC + +### DB1. Break the `State → Db` edge +`state.rs:614`, `:688`, `:772`, `:854`, `:928`, `:281` all write to SQLite from +inside `State` methods, under the whole-mount inode lock. `lib.rs:1665-1695` +commits a 5000-row listing that way. `intern_batch`/`intern_from_db` already +prove the split works — extend it to the rest, and write through after `drop(st)`. + +### DB2. Single connection throws away WAL's concurrent readers +Add a small pool of read-only connections with the same PRAGMAs and route every +`SELECT`-only method at them. The module's own justification (`mod.rs:6-7`, +"FUSE callbacks already serialize behind the State lock") stopped being true when +reads moved onto an 11-thread pool. + +### DB3. A folder upsert re-indexes its whole subtree +`nodes.rs:532-608`. Renaming a 10k-descendant folder ≈ 50k statements, ~20k +recursive CTEs, holding both the State and DB locks. Fixes, in order: persist a +`path` column so `path_of` stops being a CTE; skip the reindex when name and +parent are unchanged (every re-listing pays it today); batch the FTS refresh; +hoist `node_is_indexable_tx` out of the per-descendant loop. + +### DB4. Search is ~6000 statements per keystroke +`lib.rs:3064` takes up to 1000 candidates, then per candidate runs `path_of` +(CTE), `is_pinned` (CTE), and `mounted_search_path` → `sync_folder_list()` + +`path_relative_to` per sync folder (CTE). Fix: hoist `sync_folder_list` out of +the loop, batch pins via the existing `pinned_uids`, filter by score *before* +decorating, and persist `nodes.path`. + +### DB5. `is_pinned`'s CTE has no depth cap and no cycle guard +`pins.rs:81-88`. Every other walk in the codebase caps (`utils.rs:56`, +`nodes.rs:312`, `nodes.rs:643`, `migrations.rs:580`). A `parent_uid` cycle — +which `nodes.rs:567-570` treats as data the API can hand you — makes `UNION ALL` +non-terminating *while holding the DB mutex*. Unrecoverable mount hang, one-line +fix. + +### DB6. IPC can wedge the daemon two ways +- No `set_write_timeout` on the daemon side (`pdfs-fuse/src/control.rs:108` sets + only the read timeout; the *client* side gets both, `pdfs-core/src/control.rs:1390`). + 64 clients that never read their reply exhaust `MAX_CONTROL_HANDLERS` + permanently. +- Client-supplied `limit` goes unclamped into `SQL LIMIT ?` (`control.rs:349`, + and `ListActivity`/`PhotoTimeline`/`AlbumPhotos`), with no cap on response size. + One request materialises the whole account under the mutex. + +### DB7. Long handlers that don't ack-then-background +`VACUUM` (`maintenance.rs:183`), deep `integrity_check`, and `local_finish_scan` +(`db/local.rs:79-95`, a whole-index FTS trigram rebuild) all run under the +connection mutex — freezing the mount for tens of seconds. Every *other* long +handler already uses the ack-then-background pattern (`control.rs:452`, `:638`, +`:707`). + +### DB8. Missing indexes (one migration) +```sql +CREATE INDEX idx_sync_entry_remote_uid ON sync_entry(remote_uid); +CREATE INDEX idx_pending_op_due ON pending_op(next_attempt_at, id); +CREATE INDEX idx_local_files_scan_gen ON local_files(scan_gen); +CREATE INDEX idx_nodes_parent_live ON nodes(parent_uid) WHERE trashed = 0; +``` +`idx_pending_op_due` matters more than it looks: B70's tempfile parking puts long +runs of parked ops at the head of the queue, and `next_due_op`'s claim that it +"exits on the first row" (`ops.rs:462-465`) stops holding. +`activity_time` (`migrations.rs:302`) indexes `time DESC` while both queries order +by `id` — dead weight on every insert. + +### DB9. PRAGMAs +Set and correct: `journal_mode=WAL`, `synchronous=NORMAL`, `foreign_keys`, +`journal_size_limit`, `busy_timeout`. Missing: `cache_size` (still the 2 MiB +default, against a `node_json`-heavy schema), `mmap_size`, `temp_store=MEMORY` +(every `ORDER BY … COLLATE NOCASE` and every recursive CTE spills to a file +today), `wal_autocheckpoint` (the committing thread — often the dispatch loop — +runs the checkpoint synchronously). + +### DB10. Robustness gaps +No `SQLITE_CORRUPT`/`SQLITE_NOTADB` handling anywhere; a corrupt `cache.db` means +the daemon won't start. It is a pure cache — rename it aside and rebuild, loudly. +No single-instance lock: a hand-run `pdfs mount` alongside the systemd unit gives +two writers. Add `flock(LOCK_EX|LOCK_NB)` in `Db::open`. + +--- + +## 5. Sequential drain, and other throughput ceilings + +- **The drain is one thread, one op** (`drain.rs:130-191`, `next_due_op` is + `ORDER BY id LIMIT 1`) while the bulk uploader is 4-way (`upload.rs:7`). A + 10 GiB upload blocks every queued rename and small write behind it. Fix: a claim + column and N=3–4 workers; ordering only needs to hold per uid. +- **A permanently failing op** never wedges the queue (the backoff is correct and + never dropping the op is right) but steals 30 s of the only drain thread every + 5 minutes forever, invisibly. Surface `attempts`/`last_error` in `Status`. +- **`recover_fsynced_writes` does a `read_dir` on every idle drain iteration** + (`drain.rs:139`) for a directory that is empty ~always. +- **No adaptive debounce.** A file saved slower than 2 s but faster than its + upload never catches up. Scale the debounce toward the last measured upload + wall-time per uid, and cancel an in-flight upload on supersede. +- **Nothing bounds `staging/`.** Parked transient creates (`ops.rs:85`) are parked + forever; three abandoned 4 GiB browser downloads leave 12 GiB with no expiry and + no way for the user to see it. Report staging bytes + parked count + oldest age. +- **`own_sealed_revs` (`lib.rs:383`) is in-memory and unbounded** — a restart + re-opens the B70 conflict hole. One column persists it. +- **Orphaned staging blobs are unrecoverable by tooling.** Three paths stage the + only copy and return `EIO` with no durable reference (`lib.rs:2250`, `:2276`, + `:2297`); `hydrate_pending` walks the op table, `recover_fsynced_writes` walks + `recovery/`, nobody walks `staging/`. Add a startup reconcile. + +--- + +## 6. Confirmed sound — don't spend time here + +Reader/key caching is single-flighted and LRU-bounded per file (`reads.rs:164-277`); +no per-block key derivation. Block decryption is off the reactor +(`revision.rs:38`). `readdir`/`lookup` are genuinely not N+1 — one uid +enumeration plus one batched `enumerate_nodes_light`, with S2K deferred to a +chunked single-flight. TTL=30 s is fine and could go higher given active +invalidation. **No lock-order inversion anywhere**; no `std::sync::Mutex`, no +`.lock().unwrap()` outside tests. Worker queues are unbounded *by design* and the +reasoning is right — backpressure onto the dispatch loop is the thing being +avoided. `write(2)` takes no lock and does no fsync. `fallocate`'s `PUNCH_HOLE` +branch correctly marks the range authored so gap-fill won't refill it. Migrations +are single-transaction with a hard newer-schema refusal. `enqueue_op`'s supersede +is transactional and cannot orphan the previous blob. `fsync` durability +semantics are correct and correctly documented. + +--- + +## 7. Suggested order + +1. **One-line safety** — depth-cap `is_pinned` (DB5), `set_write_timeout` (DB6), + clamp IPC `limit`s (DB6), add `"http2"` (T7). +2. **Stop losing bytes** — D1, D2, D3, D4, D5. +3. **Off the dispatch loop** — `n_threads=4`+`clone_fd` (T6), F1, F2, F3, F5, F6. +4. **Break `State → Db`** (DB1) and stop the indexer monopolising the connection + (DB7). +5. **Cheap kernel/DB tuning batch** — rest of T6, T5, T4, DB8, DB9. +6. **Real throughput** — T1 (block single-flight, ideally in the SDK), T3 + (in-memory block cache), T2 (sequential prefetch). +7. **Structural** — persist `nodes.path` (DB3/DB4), parallel drain, adaptive + debounce, staging reconcile + reporting.