From e32f72bfca14e4ab2a87672c19103a32e21b2db4 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Sun, 4 Oct 2026 21:23:10 +0200 Subject: [PATCH] fix(core,fuse): a deleted file's trash is not held back by its create (B171) A file deleted after its create was sent keeps the op as a trash. The op kept the create's attempts and backoff, and the drain recorded the failure of a create attempt still on the wire on the row that was by then the trash. The trash waited out the create's backoff. The trash starts due, as a create that becomes a revision does, and the drain records a failed attempt only if the op is still the kind it ran as (`Db::record_attempt_failure`). --- crates/pdfs-core/src/db/ops.rs | 30 +++++++++++++++-- crates/pdfs-core/src/db/tests.rs | 55 ++++++++++++++++++++++++++++++++ crates/pdfs-fuse/src/drain.rs | 3 +- docs/BUGS.md | 25 +++++++++++++++ docs/CHANGELOG.md | 2 ++ docs/MILESTONE-3.0.0-STATUS.md | 6 ++-- 6 files changed, 116 insertions(+), 5 deletions(-) diff --git a/crates/pdfs-core/src/db/ops.rs b/crates/pdfs-core/src/db/ops.rs index abb82d9..32be8c4 100644 --- a/crates/pdfs-core/src/db/ops.rs +++ b/crates/pdfs-core/src/db/ops.rs @@ -151,6 +151,7 @@ fn drop_doomed_ops(tx: &rusqlite::Transaction<'_>, uid: &str) -> Result Result<()> { + self.record_failure(id, None, error, next_attempt_at) + } + + /// [`Db::record_op_failure`] for an attempt at op `id` as a `kind` op. An + /// op that has become another kind while the attempt ran is left as it + /// is: a create deleted on the wire is a trash now, and the create's + /// failure would hold the trash back for the create's backoff + /// (`docs/BUGS.md` B171). + pub fn record_attempt_failure( + &self, + id: i64, + kind: &str, + error: &str, + next_attempt_at: i64, + ) -> Result<()> { + self.record_failure(id, Some(kind), error, next_attempt_at) + } + + fn record_failure( + &self, + id: i64, + kind: Option<&str>, + error: &str, + next_attempt_at: i64, + ) -> Result<()> { let conn = self.conn.lock(); if drop_replaced_rename(&conn, id)? { return Ok(()); @@ -1284,8 +1310,8 @@ impl Db { "UPDATE pending_op SET attempts = attempts + 1, last_error = ?2, next_attempt_at = ?3, access_deferred_since = 0 - WHERE id = ?1", - params![id, error, next_attempt_at], + WHERE id = ?1 AND (?4 IS NULL OR kind = ?4)", + params![id, error, next_attempt_at, kind], )?; Ok(()) } diff --git a/crates/pdfs-core/src/db/tests.rs b/crates/pdfs-core/src/db/tests.rs index a1eb850..33e2554 100644 --- a/crates/pdfs-core/src/db/tests.rs +++ b/crates/pdfs-core/src/db/tests.rs @@ -3726,6 +3726,61 @@ fn a_create_withdrawn_after_it_failed_queues_a_trash_that_keeps_its_blob() { assert!(db.pending_ops().unwrap().is_empty()); } +/// A create deleted after it was sent becomes a trash in its place. The +/// create's backoff, and the failure of an attempt still on the wire, are the +/// create's: the trash is due at once (`docs/BUGS.md` B171). +#[test] +fn a_create_deleted_after_it_was_sent_leaves_a_trash_that_is_due_at_once() { + let db = Db::open_in_memory().unwrap(); + let parent = uid("parent").to_string(); + let create = |local: &str| { + db.enqueue_op(&PendingOp { + id: 0, + kind: OP_CREATE.to_string(), + uid: local.to_string(), + parent_uid: Some(parent.clone()), + name: Some(format!("{local}.txt")), + blob_path: Some(format!("/staging/{local}")), + meta_json: Some("{}".to_string()), + created_at: 1, + attempts: 0, + last_error: None, + next_attempt_at: 0, + }) + .unwrap() + .0 + }; + let failed = create("local~failed"); + db.record_attempt_failure(failed, OP_CREATE, "no answer", 50_000) + .unwrap(); + let sending = create("local~sending"); + assert_eq!( + db.claim_next_due_op(10).unwrap().map(|op| op.id), + Some(sending) + ); + + db.delete_ops_for_uid("local~failed").unwrap(); + db.delete_ops_for_uid("local~sending").unwrap(); + db.record_attempt_failure(sending, OP_CREATE, "No such file or directory", 50_000) + .unwrap(); + + let ops = db.pending_ops().unwrap(); + assert_eq!(ops.len(), 2); + for op in &ops { + assert_eq!(op.kind, OP_TRASH); + assert_eq!((op.attempts, op.last_error.as_deref()), (0, None), "{op:?}"); + assert_eq!(op.next_attempt_at, 0, "{op:?}"); + } + db.record_attempt_failure(failed, OP_TRASH, "offline", 50_000) + .unwrap(); + let trash = db + .pending_ops() + .unwrap() + .into_iter() + .find(|op| op.id == failed); + assert_eq!(trash.map(|op| op.attempts), Some(1)); +} + #[test] fn a_landed_placeholder_never_overwrites_the_server_copy() { let db = Db::open_in_memory().unwrap(); diff --git a/crates/pdfs-fuse/src/drain.rs b/crates/pdfs-fuse/src/drain.rs index 0329d87..573173b 100644 --- a/crates/pdfs-fuse/src/drain.rs +++ b/crates/pdfs-fuse/src/drain.rs @@ -394,8 +394,9 @@ impl Core { .min(DRAIN_BACKOFF_MAX) }; warn!(uid = %op.uid, attempts, error = %err_str, "pending upload failed; will retry"); - if let Err(e) = self.db.record_op_failure( + if let Err(e) = self.db.record_attempt_failure( op.id, + &op.kind, &err_str, now_millis() + backoff.as_millis() as i64, ) { diff --git a/docs/BUGS.md b/docs/BUGS.md index 54c4f72..1eb057d 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -12,6 +12,31 @@ Conventions: --- +## B171 — A file deleted while its upload fails stays on Drive for the upload's backoff + +**Status:** Fixed (unverified). +**Found:** 2026-10-04, by the simulation suite (`one_client_on_a_good_link`, seed 9). The queue +did not drain within 4 s of the settle: it still held a trash of `b.txt` with one attempt and +the error "No such file or directory (os error 2)", due seconds later. + +**Where:** `drop_doomed_ops` and `Db::record_op_failure` in `crates/pdfs-core/src/db/ops.rs`, and +the drain loop in `crates/pdfs-fuse/src/drain.rs`. + +**Cause.** A file deleted after its create was sent keeps its op as a trash, in case the create +made the node and only its answer was lost (B151). The op kept the create's attempts, error and +backoff. The drain also recorded the failure of a create attempt still on the wire on the row, +which by then was the trash. Here the attempt failed because a write had replaced the blob it +was reading, and the trash waited out the create's backoff. + +**Fix.** The trash starts with no attempts and is due at once, as a create that becomes a +revision already does. The drain records a failed attempt only if the op is still the kind it +ran as (`Db::record_attempt_failure`). + +**Test:** `a_create_deleted_after_it_was_sent_leaves_a_trash_that_is_due_at_once` in +`crates/pdfs-core/src/db/tests.rs`. + +--- + ## B170 — A write opened as an upload lands fails the next write with `EIO` **Status:** Fixed (unverified). diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 3cbe2ba..2981429 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -70,6 +70,8 @@ by its local id, in the queue, the stored tree and pins alike. progress finish, and the rest waits for the next start (B169). - **Writing to a file again just as its upload finishes no longer fails.** A write right after it failed with "Input/output error" (B170). +- **A file deleted while its upload was retrying is removed from Drive right away.** It no + longer waits for the upload's next retry (B171). - **A file renamed just as its upload finished keeps its new name.** The mount no longer showed the old name, and a second rename no longer failed with "No such file or directory" (B152). - **A change that waits for a name to be freed no longer waits seconds after it is.** A rename or diff --git a/docs/MILESTONE-3.0.0-STATUS.md b/docs/MILESTONE-3.0.0-STATUS.md index 292e5d4..abf4457 100644 --- a/docs/MILESTONE-3.0.0-STATUS.md +++ b/docs/MILESTONE-3.0.0-STATUS.md @@ -9,7 +9,7 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase - **Phase 2**: local ids, inode = local id, one tree for every mount (2.10.0). - **Phase 3, first part**: mkdir, create, rename, unlink and rmdir on the mount are recorded locally and sent from the queue; `"local_first": false` brings back the old path (5e0f30c). -- Fixes from the simulation and account runs since then: B127 and B141 to B170. +- Fixes from the simulation and account runs since then: B127 and B141 to B171. - **Sync issues, first cut** (§6.2): refusals sorted into issues, shown at once in the app, tray and CLI; Export button and `pdfs sync export`; `pdfs sync issues`. @@ -112,7 +112,9 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase sent again, to the root (B168, fixed), and a stop that let a mirror pass go on until the runtime ended under it (B169, fixed). CI on 9bbc994 failed `one_client_on_a_good_link`: a write opened as an upload landed took the revision - it replaced as its base, and the next write failed with `EIO` (B170, fixed). + it replaced as its base, and the next write failed with `EIO` (B170, fixed). The + suite's seed 9 found a deleted file's trash waiting out its create's backoff (B171, + fixed). 5. **Release**: 2.10.0 is tagged; the 2.9.0 tag still waits for the user's go-ahead. ## After 3.0.0