From d26dc8ada4ca2a17210df9e10084f86001523bd1 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Mon, 5 Oct 2026 02:34:05 +0200 Subject: [PATCH] fix(fuse,core): retry a create at once when a write replaced its blob (B185) --- crates/pdfs-core/src/db/ops.rs | 12 ++++++-- crates/pdfs-core/src/db/tests.rs | 50 ++++++++++++++++++++++++++++---- crates/pdfs-fuse/src/drain.rs | 3 +- docs/BUGS.md | 19 ++++++++++++ docs/CHANGELOG.md | 2 ++ docs/MILESTONE-3.0.0-STATUS.md | 5 ++-- 6 files changed, 80 insertions(+), 11 deletions(-) diff --git a/crates/pdfs-core/src/db/ops.rs b/crates/pdfs-core/src/db/ops.rs index f132e42..bd29487 100644 --- a/crates/pdfs-core/src/db/ops.rs +++ b/crates/pdfs-core/src/db/ops.rs @@ -1477,22 +1477,28 @@ impl Db { /// Make a create due again if it was renamed or moved /// ([`Db::rewrite_op_target`]) while the attempt that just failed was on /// the wire, `parent_uid` and `name` being what that attempt sent. What it - /// failed on may have been the target it no longer has. Returns whether it + /// failed on may have been the target it no longer has. Likewise if a + /// write replaced its blob (`blob_path` being the one the attempt read): + /// the old one is discarded as the new one attaches, and an attempt that + /// had not opened it yet failed on a file that is gone. Returns whether it /// was. pub fn retry_if_retargeted( &self, id: i64, parent_uid: Option<&str>, name: Option<&str>, + blob_path: Option<&str>, ) -> Result { let conn = self.conn.lock(); let n = conn.execute( &format!( "UPDATE pending_op SET next_attempt_at = 0 WHERE id = ?1 AND kind IN (?4, ?5) AND next_attempt_at < ?6 - AND ({PARENT_NOW} IS NOT ?2 OR name IS NOT ?3)" + AND ({PARENT_NOW} IS NOT ?2 OR name IS NOT ?3 OR blob_path IS NOT ?7)" ), - params![id, parent_uid, name, OP_CREATE, OP_MKDIR, PARK_UNTIL], + params![ + id, parent_uid, name, OP_CREATE, OP_MKDIR, PARK_UNTIL, blob_path + ], )?; Ok(n > 0) } diff --git a/crates/pdfs-core/src/db/tests.rs b/crates/pdfs-core/src/db/tests.rs index 6c192e5..41f29ca 100644 --- a/crates/pdfs-core/src/db/tests.rs +++ b/crates/pdfs-core/src/db/tests.rs @@ -2969,7 +2969,8 @@ fn renaming_a_queued_create_rewrites_its_target() { /// A create that failed on a name held by a queued change waits out a /// backoff. Renamed away from that name, it is due again at once; parked, it -/// stays parked. +/// stays parked. One whose blob a write replaced as it failed is due again at +/// once too. #[test] fn renaming_a_backing_off_create_makes_it_due() { let db = Db::open_in_memory().unwrap(); @@ -3011,8 +3012,13 @@ fn renaming_a_backing_off_create_makes_it_due() { db.record_op_failure(sent.id, "d is still held", 10_000) .unwrap(); assert!( - db.retry_if_retargeted(sent.id, sent.parent_uid.as_deref(), sent.name.as_deref()) - .unwrap() + db.retry_if_retargeted( + sent.id, + sent.parent_uid.as_deref(), + sent.name.as_deref(), + sent.blob_path.as_deref() + ) + .unwrap() ); let ops = db.pending_ops().unwrap(); assert_eq!( @@ -3023,8 +3029,42 @@ fn renaming_a_backing_off_create_makes_it_due() { 0 ); assert!( - !db.retry_if_retargeted(sent.id, sent.parent_uid.as_deref(), Some("f")) - .unwrap() + !db.retry_if_retargeted( + sent.id, + sent.parent_uid.as_deref(), + Some("f"), + sent.blob_path.as_deref() + ) + .unwrap() + ); + + // A write replaced the blob before the attempt opened it: the old one was + // discarded, the attempt failed on a missing file, and the create is due + // again at once with the new one (`docs/BUGS.md` B185). + db.record_op_failure(sent.id, "No such file or directory", 10_000) + .unwrap(); + db.attach_blob_to_create("local~failed", "/staging/newer", "{}") + .unwrap() + .expect("the create is still queued"); + db.record_op_failure(sent.id, "No such file or directory", 10_000) + .unwrap(); + assert!( + db.retry_if_retargeted( + sent.id, + sent.parent_uid.as_deref(), + Some("f"), + sent.blob_path.as_deref() + ) + .unwrap() + ); + assert!( + !db.retry_if_retargeted( + sent.id, + sent.parent_uid.as_deref(), + Some("f"), + Some("/staging/newer") + ) + .unwrap() ); } diff --git a/crates/pdfs-fuse/src/drain.rs b/crates/pdfs-fuse/src/drain.rs index 5dac840..92cf839 100644 --- a/crates/pdfs-fuse/src/drain.rs +++ b/crates/pdfs-fuse/src/drain.rs @@ -441,9 +441,10 @@ impl Core { op.id, op.parent_uid.as_deref(), op.name.as_deref(), + op.blob_path.as_deref(), ) { Ok(true) => { - debug!(uid = %op.uid, "create was renamed while it failed; retrying") + debug!(uid = %op.uid, "create was renamed or rewritten while it failed; retrying") } Ok(false) => {} Err(e) => { diff --git a/docs/BUGS.md b/docs/BUGS.md index 1269d0f..565113d 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -12,6 +12,25 @@ Conventions: --- +## B185 — A new file written again just as it starts uploading waits out a retry + +**Status:** Fixed (unverified). +**Found:** 2026-10-05, in the confirmation run on 3073052 (`one_client_on_a_good_link` seed 6, +then `one_client_deleting_files_as_they_upload` seed 4). The queues did not drain in time: a +create had failed with "No such file or directory" and was backing off. + +**Where:** `Db::retry_if_retargeted` in `crates/pdfs-core/src/db/ops.rs`, called from the drain +in `crates/pdfs-fuse/src/drain.rs`. + +**Cause.** A write to a file whose create is queued attaches a new blob to the create and +discards the old one at once. An attempt that had claimed the create but not yet opened the old +blob failed on a file that was gone, and backed off as if Drive had refused it. The new blob was +ready, but the create waited out the backoff. + +**Fix.** A failed create whose blob was replaced while it ran is due again at once, the way one +renamed while it ran already was. Test `renaming_a_backing_off_create_makes_it_due` in +`crates/pdfs-core/src/db/tests.rs`. + ## B184 — A file renamed after its create lost its answer stays on Drive under its old name **Status:** Fixed (unverified). diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 6566dc1..f73c071 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -105,6 +105,8 @@ by its local id, in the queue, the stored tree and pins alike. - **A file renamed while the connection lost Drive's answer to its upload lands once.** It no longer stayed on Drive under its old name too, or, when deleted, under its old name alone (B184). +- **A new file written again just as it starts uploading no longer waits out a retry** before + it reaches Drive (B185). - **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, B172). diff --git a/docs/MILESTONE-3.0.0-STATUS.md b/docs/MILESTONE-3.0.0-STATUS.md index 628e67d..629c452 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: B125, B127, B130 and B141 to B184. +- Fixes from the simulation and account runs since then: B125, B127, B130 and B141 to B185. - **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`; Discard button and `pdfs sync discard`; a warning badge on the node in the file browser. @@ -110,7 +110,8 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase after restarts: a write over a create never read back (B182) and a trash that forgot what its create sent (B183). Replays with those fixed found a third: a create renamed after it lost its answer (B184). All three fixed; no known-bug matcher fired. It was - stopped before `three_clients`. + stopped before `three_clients`. The rerun on 3073052 found a create that waited out + a retry after a write replaced its blob (B185, fixed). - [ ] A clean account run. The one on a6f36d5 (2026-10-04, LAN: 427 passed, 12 failed) failed B113 and B121 on every on-demand mount, two mirror-to-mirror move cases, and the journal check. B121 was a bug (B149, fixed); B113's case still expected creates