diff --git a/crates/pdfs-core/src/db/ops.rs b/crates/pdfs-core/src/db/ops.rs index bbddfd1..a04be71 100644 --- a/crates/pdfs-core/src/db/ops.rs +++ b/crates/pdfs-core/src/db/ops.rs @@ -1483,12 +1483,26 @@ impl Db { } /// [`retry_op_now`](Self::retry_op_now) for every op that has failed at - /// least once. Returns how many rows changed. + /// least once, for the user's "Retry all". Returns how many rows changed. pub fn retry_failed_ops_now(&self, now: i64) -> Result { + self.retry_ops_now(now, "") + } + + /// [`retry_failed_ops_now`](Self::retry_failed_ops_now) for the ops Drive + /// did not refuse, for a reconnect. A link coming back changes nothing + /// about a full quota or a missing permission, so those keep their backoff + /// instead of all being sent again at once. + pub fn retry_unrefused_ops_now(&self, now: i64) -> Result { + self.retry_ops_now(now, "AND issue IS NULL") + } + + fn retry_ops_now(&self, now: i64, filter: &str) -> Result { let conn = self.conn.lock(); let changed = conn.execute( - "UPDATE pending_op SET next_attempt_at = ?1 - WHERE attempts > 0 AND next_attempt_at > ?1 AND next_attempt_at < ?2", + &format!( + "UPDATE pending_op SET next_attempt_at = ?1 + WHERE attempts > 0 AND next_attempt_at > ?1 AND next_attempt_at < ?2 {filter}" + ), params![now, PARK_UNTIL], )?; Ok(changed) diff --git a/crates/pdfs-core/src/db/tests.rs b/crates/pdfs-core/src/db/tests.rs index 8843589..21a7f59 100644 --- a/crates/pdfs-core/src/db/tests.rs +++ b/crates/pdfs-core/src/db/tests.rs @@ -7531,6 +7531,32 @@ fn a_refused_change_names_its_node_by_local_id() { ); } +/// 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. +#[test] +fn a_reconnect_leaves_refused_ops_to_their_backoff() { + let db = Db::open_in_memory().unwrap(); + let offline = queued_op(&db, "vol~a", 2, 90_000); + let refused = queued_op(&db, "vol~b", 2, 90_000); + db.set_op_issue(refused, Some("quota")).unwrap(); + + assert_eq!(db.retry_unrefused_ops_now(5_000).unwrap(), 1); + let due = |id: i64| { + db.pending_ops() + .unwrap() + .into_iter() + .find(|op| op.id == id) + .unwrap() + .next_attempt_at + }; + assert_eq!(due(offline), 5_000); + assert_eq!(due(refused), 90_000); + + assert_eq!(db.retry_failed_ops_now(5_000).unwrap(), 1); + assert_eq!(due(refused), 5_000); +} + #[test] fn retry_all_touches_only_ops_that_have_failed() { let db = Db::open_in_memory().unwrap(); diff --git a/crates/pdfs-fuse/src/drain.rs b/crates/pdfs-fuse/src/drain.rs index 3668217..e530787 100644 --- a/crates/pdfs-fuse/src/drain.rs +++ b/crates/pdfs-fuse/src/drain.rs @@ -389,13 +389,7 @@ impl Core { // A name a change of ours is about to free is a wait, not a // failure: the trash or rename holding it lands in moments. let held = waits_for_name(e.as_ref()); - let backoff = if held { - DRAIN_BACKOFF_MIN - } else { - DRAIN_BACKOFF_MIN - .saturating_mul(1u32 << attempts.min(6)) - .min(DRAIN_BACKOFF_MAX) - }; + let backoff = drain_backoff(attempts, held, op.id); warn!(uid = %op.uid, attempts, error = %err_str, "pending upload failed; will retry"); if let Err(e) = self.db.record_attempt_failure( op.id, @@ -2787,6 +2781,25 @@ fn waits_for_name(e: &(dyn std::error::Error + 'static)) -> bool { std::iter::successors(Some(e), |e| e.source()).any(|e| e.is::()) } +/// How long an op waits after its `attempts`-th failure: doubling from +/// [`DRAIN_BACKOFF_MIN`] up to [`DRAIN_BACKOFF_MAX`], plus up to a fifth more, +/// different for each op (`seed`), so that ops that failed together are not +/// all retried in the same second. One that waits for a name (`held`) waits +/// the minimum. +/// +/// The doubling stopped at 2^6, 128 s, so the maximum was never reached. +fn drain_backoff(attempts: i64, held: bool, seed: i64) -> Duration { + if held { + return DRAIN_BACKOFF_MIN; + } + let base = DRAIN_BACKOFF_MIN + .saturating_mul(1u32 << attempts.clamp(0, 8)) + .min(DRAIN_BACKOFF_MAX); + // A cheap, stable spread: the same op and attempt always wait the same. + let spread = (seed.wrapping_mul(0x9E37_79B9).wrapping_add(attempts) as u64 % 21) as u32; + base + base * spread / 100 +} + /// How many attempts an op waits for a rename or a create on the wire to free /// the name it wants. Two files swapping names each wait for the other, so the /// wait has to end; a conflict name ends it. @@ -3386,6 +3399,22 @@ mod tests { } } + #[test] + fn a_failing_op_backs_off_to_the_maximum_with_a_spread() { + let at = |attempts| drain_backoff(attempts, false, 7); + assert!(at(1) >= DRAIN_BACKOFF_MIN * 2); + assert!(at(2) > at(1)); + // Reached, which the doubling capped at 2^6 never did. + assert!(at(8) >= DRAIN_BACKOFF_MAX); + assert!(at(30) <= DRAIN_BACKOFF_MAX + DRAIN_BACKOFF_MAX / 5); + // Ops that failed together do not all come back together. + let waits: std::collections::HashSet<_> = + (1..=20).map(|id| drain_backoff(8, false, id)).collect(); + assert!(waits.len() > 5, "{waits:?}"); + // A wait for a name stays short however often it repeats. + assert_eq!(drain_backoff(30, true, 7), DRAIN_BACKOFF_MIN); + } + #[test] fn only_a_fresh_empty_twin_of_the_same_kind_is_adopted() { // The op was queued at t=1000 s, after a create that got no answer. diff --git a/crates/pdfs-fuse/src/link.rs b/crates/pdfs-fuse/src/link.rs index b23a352..3430235 100644 --- a/crates/pdfs-fuse/src/link.rs +++ b/crates/pdfs-fuse/src/link.rs @@ -280,8 +280,9 @@ impl Core { self.relist_offline_listings(); // Anything written while offline is queued and waiting on exactly this, // including ops that failed on the way down and are sitting out a - // backoff sized for a server that keeps refusing. - if let Err(e) = self.db.retry_failed_ops_now(crate::now_millis()) { + // backoff sized for a server that keeps refusing. Ops Drive refused + // keep theirs: the link was not why. + if let Err(e) = self.db.retry_unrefused_ops_now(crate::now_millis()) { warn!(error = %e, "moving backed-off ops up after a reconnect failed"); } self.wake_drain(); diff --git a/docs/BUGS.md b/docs/BUGS.md index 93b971d..282a941 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -100,6 +100,7 @@ a live account since. | Entry | Title | |---|---| +| [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 | | [B234](#b234--any-folder-can-become-a-sync-root-the-home-folder-included) | Any folder can become a sync root, the home folder included | @@ -339,6 +340,33 @@ a live account since. Newest first. +### B237 — Every reconnect resends refused uploads, all in the same second + +**Status:** Fixed, not verified live +**Found:** 2026-10-16, architecture audit (`FINDINGS.md` 2.3.3). +**Where:** `drain_backoff` in `crates/pdfs-fuse/src/drain.rs`; the reconnect in +`crates/pdfs-fuse/src/link.rs`; `Db::retry_unrefused_ops_now` in +`crates/pdfs-core/src/db/ops.rs` + +**Repro:** Fill the Drive quota and queue a few hundred files, then let the link drop and come +back a few times. + +**Cause.** Three things together: +- Every reconnect made every failed op due at once, the ones Drive refused for quota or access + included. Each reconnect then sent the whole refused queue again. +- All ops that failed together were retried together: the backoff had no spread. +- The doubling stopped at 2^6, so the wait never grew past 128 s, and the 300 s maximum was never + reached. + +**Fix.** A reconnect makes due only the ops without a recorded refusal. "Retry all" still moves +every op. The backoff doubles up to the 300 s maximum, plus up to a fifth more, which differs +from op to op. + +**Test:** `db::tests::a_reconnect_leaves_refused_ops_to_their_backoff`; +`drain::tests::a_failing_op_backs_off_to_the_maximum_with_a_spread`. + +--- + ### B236 — Imported, re-dated and app-uploaded photos have no thumbnail **Status:** Fixed, not verified live