diff --git a/crates/pdfs-core/src/db/ops.rs b/crates/pdfs-core/src/db/ops.rs index 6a8c631..abb82d9 100644 --- a/crates/pdfs-core/src/db/ops.rs +++ b/crates/pdfs-core/src/db/ops.rs @@ -222,6 +222,20 @@ fn on_node(n: usize) -> String { ON_NODE.replace("?L", &format!("?{n}")) } +/// Delete op `id` if it is a rename with a newer rename of the same node queued +/// behind it, which says where the node goes now. Returns whether it did. +fn drop_replaced_rename(conn: &rusqlite::Connection, id: i64) -> Result { + let dropped = conn.execute( + "DELETE FROM pending_op + WHERE id = ?1 AND kind = ?2 + AND EXISTS (SELECT 1 FROM pending_op AS newer + WHERE (newer.uid = pending_op.uid OR newer.lid = pending_op.lid) + AND newer.kind = ?2 AND newer.id > ?1)", + params![id, OP_RENAME], + )?; + Ok(dropped > 0) +} + /// The uid a queued op's node has now: the one its row holds, which a landing /// create has already moved to its real uid, or the one the op was queued with /// when the node has no row. @@ -501,7 +515,7 @@ impl Db { /// A rename a drain worker holds is not replaced but queued behind: the /// claim keeps the newer one from being sent until it retires, so the two /// land in the order they were made (`docs/BUGS.md` B141). Should it fail, - /// [`Db::record_op_failure`] drops it. + /// [`Db::record_op_failure`] or [`Db::release_op_claim`] drops it. pub fn enqueue_op(&self, op: &PendingOp) -> Result<(i64, Option)> { let mut conn = self.conn.lock(); let tx = conn.transaction()?; @@ -1034,8 +1048,17 @@ impl Db { /// `UPDATE` matches nothing), and a handler that returns without retiring /// *and* without failing would otherwise leave the row claimed by a worker /// that has moved on โ€” invisible to every worker, forever. + /// + /// A rename still here with a newer one queued behind it goes instead, as + /// in [`Db::record_op_failure`]. The newer one may have been queued after + /// the failure was recorded: kept, the older one sat out its backoff while + /// the newer one landed, and then put the old name back (`docs/BUGS.md` + /// B158). pub fn release_op_claim(&self, id: i64) -> Result<()> { let conn = self.conn.lock(); + if drop_replaced_rename(&conn, id)? { + return Ok(()); + } conn.execute( "UPDATE pending_op SET claimed_at = 0 WHERE id = ?1", params![id], @@ -1254,15 +1277,7 @@ impl Db { /// dropped instead: retried, it could land after the newer one and undo it. pub fn record_op_failure(&self, id: i64, error: &str, next_attempt_at: i64) -> Result<()> { let conn = self.conn.lock(); - let dropped = conn.execute( - "DELETE FROM pending_op - WHERE id = ?1 AND kind = ?2 - AND EXISTS (SELECT 1 FROM pending_op AS newer - WHERE newer.uid = pending_op.uid AND newer.kind = ?2 - AND newer.id > ?1)", - params![id, OP_RENAME], - )?; - if dropped > 0 { + if drop_replaced_rename(&conn, id)? { return Ok(()); } conn.execute( diff --git a/crates/pdfs-core/src/db/tests.rs b/crates/pdfs-core/src/db/tests.rs index 88a1435..a1eb850 100644 --- a/crates/pdfs-core/src/db/tests.rs +++ b/crates/pdfs-core/src/db/tests.rs @@ -2977,7 +2977,8 @@ fn renaming_a_backing_off_create_makes_it_due() { } /// A rename queued while an older one is on the wire waits for it. The older -/// one, if it fails, is dropped rather than retried after the newer one. +/// one, if it fails, is dropped rather than retried after the newer one, also +/// when the newer one came after the failure was recorded. #[test] fn a_rename_on_the_wire_lands_before_the_one_that_replaces_it() { let db = Db::open_in_memory().unwrap(); @@ -3013,6 +3014,16 @@ fn a_rename_on_the_wire_lands_before_the_one_that_replaces_it() { let (third, _) = db.enqueue_op(&rename("x")).unwrap(); let ops = db.pending_ops().unwrap(); assert_eq!(ops.iter().map(|o| o.id).collect::>(), [third]); + + // Queued after the failure was recorded but before the claim went back: + // the older one goes then, rather than landing after the newer (B158). + assert_eq!(db.claim_next_due_op(1).unwrap().unwrap().id, third); + db.record_op_failure(third, "e.md is still held", 10_000) + .unwrap(); + let (fourth, _) = db.enqueue_op(&rename("c0")).unwrap(); + db.release_op_claim(third).unwrap(); + let ops = db.pending_ops().unwrap(); + assert_eq!(ops.iter().map(|o| o.id).collect::>(), [fourth]); } #[test] diff --git a/docs/BUGS.md b/docs/BUGS.md index fcf1d82..282c535 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -12,6 +12,31 @@ Conventions: --- +## B158 โ€” A file renamed twice in a row ends up under the first name, again + +**Status:** Fixed (unverified). +**Found:** 2026-10-04, by the simulation's Wi-Fi profile (`one_client_on_a_slow_link`, seed 3, +four of ten runs on 4676a4e). A file was renamed to `b.txt`, whose name a create on the wire +still held, and then to `e.md`. Drive renamed it to `e.md` and then back to `b.txt`. The +liveness check found `b.txt` on Drive and `e.md` in the mount. + +**Where:** `Db::record_op_failure` and `Db::release_op_claim` in +`crates/pdfs-core/src/db/ops.rs`. + +**Cause.** B141's fix drops a rename that fails while a newer one is queued behind it. The worker +records the failure first and releases its claim after that. The second rename came in between: +the first was still claimed, so the second queued behind it, but the failure had already been +recorded with nothing behind it, so the first was kept with a backoff. Once released, it was not +due and the second was, so the second landed first. More workers made the gap easier to hit. + +**Fix.** Releasing the claim of a rename drops it as well when a newer rename of the same node is +queued behind it. Both checks match the node by local id as well as by uid. + +**Test:** unit test `a_rename_on_the_wire_lands_before_the_one_that_replaces_it`, extended. +Seeds 2 and 3 of `one_client_on_a_slow_link` pass eight replays each. + +--- + ## B157 โ€” A second write to a file whose upload was not read back fails with `EIO` **Status:** Fixed (unverified). diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 82c493f..0fad2c4 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -44,6 +44,8 @@ by its local id, in the queue, the stored tree and pins alike. of itself when the link lost Drive's answer to the first try** (B156). - **Writing to a file twice in a row no longer fails with "Input/output error" when the link lost Drive's answer to reading back the file's last upload** (B157). +- **A file renamed twice in a row, while the first name was still taken, keeps the second name** + (B158). - **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 13de481..8d940cc 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 B157. +- Fixes from the simulation and account runs since then: B127 and B141 to B158. - **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`. @@ -81,7 +81,8 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase it may be B152. Replays of slow-link seed 4 failed two of six times on a file written again after its create lost its answer (B156, fixed). With sixteen drain workers (16fca10), seed 4 failed every replay on a write after an upload that was not read - back (B157, fixed). + back (B157, fixed). Seed 3 then failed four of ten runs on two renames of one file + landing out of order (B158, fixed). - [ ] Known-bug list in `sim/run.rs` is empty: B125 and B130 open; B127, B129 and B132 fixed but unconfirmed. - [ ] A clean account run. The one on a6f36d5 (2026-10-04, LAN: 427 passed, 12 failed)