fix(fuse): a file renamed while its upload is read back keeps its new name (B172)

This commit is contained in:
nvrl committed 2026-10-04 21:34:20 +02:00
1 parent e32f72bfca
commit d850265225
5 files changed
+135 -22

No files matched your search

+43 -19
View File
@@ -2294,6 +2294,14 @@ impl Core {
blob: &Path,
adopt: impl FnOnce(i64, u64),
) -> Option<Node> {
// Asked before the read-back: the op went before it, so a rename queued
// behind the upload can land while Drive answers, and the answer may
// still have the old name (`docs/BUGS.md` B172).
let placed = self.placement(uid);
let renaming = self
.db
.has_pending_op(&uid.to_string(), OP_RENAME)
.unwrap_or(false);
let fetched = self.fetch_node_remote(uid);
if fetched.is_ok() {
self.sealed_unread.lock().remove(uid);
@@ -2376,37 +2384,42 @@ impl Core {
self.release_pending(uid, blob);
#[cfg(test)]
self.drive.landing(uid);
self.place_landed(uid, &node);
self.place_landed(uid, &node, placed, renaming);
self.landed.lock().remove(uid);
Some(node)
}
/// Put a node whose upload just landed into the tree, unless a newer write
/// is queued for it.
fn place_landed(&self, uid: &NodeUid, node: &Node) {
/// is queued for it. `before` is where the tree had the node and
/// `renaming` whether a rename of it was queued, both from before the
/// read-back.
fn place_landed(
&self,
uid: &NodeUid,
node: &Node,
before: Option<(String, Option<NodeUid>)>,
renaming: bool,
) {
// Ordered so that a write queued *during* the fetch above is still
// caught: it took its baseline from the node's optimistic stamp, and
// this overwrites it with the revision the server actually holds.
if self.rebaseline_pending(uid, node) {
return;
}
// A rename queued behind the upload has not reached Drive yet, and
// until it lands the tree keeps the name and folder the user gave the
// file, as `adopt_real_uid` does. Taking Drive's put the file back
// under its old name. A rename queued after the check below has
// already moved the entry when the tree is updated, so a name that
// changed in between is the user's too (docs/BUGS.md B146).
let placed = |st: &crate::state::State| {
let entry = st.by_uid.get(uid).and_then(|ino| st.entries.get(ino))?;
Some((entry.node.name.clone(), entry.node.parent_uid.clone()))
};
let before = placed(&self.state());
let renaming = self
.db
.has_pending_op(&uid.to_string(), OP_RENAME)
.unwrap_or(false);
// A rename queued behind the upload has not reached Drive yet, or
// landed after Drive read the node back, and the tree keeps the name
// and folder the user gave the file, as `adopt_real_uid` does. Taking
// Drive's put the file back under its old name. A rename queued since
// the read-back began has already moved the entry when the tree is
// updated, so a name that changed in between is the user's too
// (docs/BUGS.md B146, B172).
let renaming = renaming
|| self
.db
.has_pending_op(&uid.to_string(), OP_RENAME)
.unwrap_or(false);
self.for_each_state(|st| {
let moved = placed(st) != before;
let moved = placed_in(st, uid) != before;
let Some(entry) = st.by_uid.get(uid).and_then(|ino| st.entries.get(ino)) else {
return;
};
@@ -2420,6 +2433,11 @@ impl Core {
});
}
/// Where the tree has `uid`: its name and parent.
fn placement(&self, uid: &NodeUid) -> Option<(String, Option<NodeUid>)> {
placed_in(&self.state(), uid)
}
/// Rebase every write handle open on `uid` onto `node`, a revision this
/// daemon has just put there itself (`WriteHandle::rebase_onto`).
fn rebase_open_writes(&self, uid: &NodeUid, node: &Node) {
@@ -2675,6 +2693,12 @@ fn keeps_landed_upload(pinned: bool, len: u64, budget: u64) -> bool {
pinned || budget == 0 || len <= budget / LANDED_KEEP_SHARE
}
/// Where `st` has `uid`: its name and parent.
fn placed_in(st: &crate::state::State, uid: &NodeUid) -> Option<(String, Option<NodeUid>)> {
let entry = st.by_uid.get(uid).and_then(|ino| st.entries.get(ino))?;
Some((entry.node.name.clone(), entry.node.parent_uid.clone()))
}
#[cfg(test)]
mod tests {
use super::*;
+62
View File
@@ -463,6 +463,68 @@ mod tests {
let _ = std::fs::remove_dir_all(&dir);
}
#[test]
#[ignore = "mounts FUSE: run with `cargo test -p pdfs-fuse sim:: -- --ignored`"]
fn a_file_renamed_while_its_upload_is_read_back_keeps_its_new_name() {
// The rename landed while the drain read the uploaded file back. The
// answer still had the old name, and the drain found no rename queued,
// so the mount showed the file under its old name and the new one
// answered ENOENT (B172).
let drive = FakeDrive::new();
let dir = scratch("renamed-while-read-back");
let daemon = Daemon::start(&dir, drive.client(1, Faults::lan())).unwrap();
let mnt = daemon.mountpoint.clone();
let file = |bytes: &[u8]| Some(Entry::File(Arc::new(bytes.to_vec())));
std::fs::write(mnt.join("a.txt"), b"first").unwrap();
assert!(wait_until(Duration::from_secs(30), || {
daemon.pending().is_ok_and(|items| items.is_empty())
&& drive.tree().get("a.txt").cloned() == file(b"first")
}));
let uid = drive.lookup("a.txt").unwrap();
let mut held = daemon.client.hold_answer_to_next_read(&uid);
std::fs::write(mnt.join("a.txt"), b"second").unwrap();
assert!(wait_until(Duration::from_secs(30), || held.reached()));
std::fs::rename(mnt.join("a.txt"), mnt.join("f.txt")).unwrap();
// Each read is answered in turn until the upload's op is retired; the
// answer to the read-back after it is held until the rename has landed.
let reading_back = loop {
if daemon
.pending()
.is_ok_and(|items| items.iter().all(|op| op.kind != "revision"))
{
break held;
}
let next = daemon.client.hold_answer_to_next_read(&uid);
drop(held);
assert!(wait_until(Duration::from_secs(30), || next.reached()));
held = next;
};
assert!(
wait_until(Duration::from_secs(30), || drive.lookup("f.txt").is_some()),
"{:?} {:?}",
daemon.pending(),
drive.tree()
);
drop(reading_back);
assert!(
wait_until(Duration::from_secs(30), || {
daemon.pending().is_ok_and(|items| items.is_empty())
&& drive.tree().get("f.txt").cloned() == file(b"second")
}),
"{:?} {:?}",
daemon.pending(),
drive.tree()
);
// Long enough for the drain to have put the read-back into the tree.
std::thread::sleep(Duration::from_millis(200));
assert_eq!(std::fs::read(mnt.join("f.txt")).unwrap(), b"second");
assert!(!mnt.join("a.txt").exists());
assert!(matches!(daemon.stop(), Ok(MountOutcome::Shutdown)));
let _ = std::fs::remove_dir_all(&dir);
}
#[test]
#[ignore = "mounts FUSE: run with `cargo test -p pdfs-fuse sim:: -- --ignored`"]
fn a_file_moved_while_its_create_was_on_the_wire_keeps_its_new_name() {
+25
View File
@@ -12,6 +12,31 @@ Conventions:
---
## B172 — A file renamed while its upload is read back shows its old name
**Status:** Fixed (unverified).
**Found:** 2026-10-04, by `one_client_on_a_good_link` (seed 8) on 2502af6, run three at a time
in a loop: two of nine runs failed. In one, a write to `f.txt` answered `ENOENT`; in the other,
a rename of `f.txt`. The mount showed the file as `a.txt` again.
**Where:** `refresh_after_upload` and `place_landed` in `crates/pdfs-fuse/src/drain.rs`.
**Cause.** `a.txt` was renamed to `f.txt` while its create was on the wire, with a write behind
it. The create landed, and its op became a revision with a rename queued after it. The upload
retires its op before it reads the node back, so the rename went out while Drive answered the
read-back, and that answer still had the old name. The drain asked whether a rename was queued
only after the answer came, found none, and put `a.txt` back into the tree. The create's
read-back already asks first (B161); the upload's did not.
**Fix.** The upload's read-back notes whether a rename is queued, and where the tree has the
file, before it asks Drive. The tree keeps the name and folder the user gave the file if a
rename was queued then or is now, or if the file moved meanwhile.
**Test:** `a_file_renamed_while_its_upload_is_read_back_keeps_its_new_name` in
`crates/pdfs-fuse/src/sim/daemon.rs`.
---
## B171 — A file deleted while its upload fails stays on Drive for the upload's backoff
**Status:** Fixed (unverified).
+2 -1
View File
@@ -73,7 +73,8 @@ by its local id, in the queue, the stored tree and pins alike.
- **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).
the old name, and a second rename no longer failed with "No such file or directory" (B152,
B172).
- **A change that waits for a name to be freed no longer waits seconds after it is.** A rename or
upload over a file being deleted goes out as soon as the delete lands (B153).
- **A file deleted right after it was made no longer stays on Drive when the link lost Drive's
+3 -2
View File
@@ -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 B171.
- Fixes from the simulation and account runs since then: B127 and B141 to B172.
- **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`.
@@ -114,7 +114,8 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase
`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). The
suite's seed 9 found a deleted file's trash waiting out its create's backoff (B171,
fixed).
fixed). Run three at a time on 2502af6, seed 8 failed two of nine runs: a file renamed
while its upload was read back showed its old name again (B172, 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