mirror of
https://github.com/narrrl/proton-drive-linux.git
synced 2026-11-03 16:41:26 +01:00
Merge pull request #63 from narrrl/fix/sim-flakes-and-open-race
fix(fuse): retry a write open whose queued revision landed under the copy
This commit is contained in:
3 files changed
+91
-8
No files matched your search
@@ -1434,6 +1434,14 @@ impl ProtonFs {
|
||||
// (`ContentCache::clone_or_copy`).
|
||||
if let Err(e) = ContentCache::clone_or_copy(&pending.path, &path) {
|
||||
let _ = std::fs::remove_file(&path);
|
||||
// The drain lets go of the pending entry before it
|
||||
// discards the blob, so a blob that went away under the
|
||||
// copy is a revision that landed or was replaced: the
|
||||
// same race the re-check below catches, only earlier.
|
||||
if self.core.pending_blob(&uid).as_ref() != Some(&pending.path) {
|
||||
warn!(%uid, attempt, "queued revision went away while opening for write; retrying");
|
||||
continue;
|
||||
}
|
||||
error!(%uid, source = %pending.path.display(), error = %e,
|
||||
"copy queued revision into write scratch failed");
|
||||
break 'install Err(Errno::EIO);
|
||||
|
||||
@@ -592,14 +592,22 @@ mod tests {
|
||||
daemon.pending(),
|
||||
drive.tree()
|
||||
);
|
||||
// The bytes kept for the handle went once the trash landed with it closed.
|
||||
let cached: Vec<_> = std::fs::read_dir(dir.join("cache"))
|
||||
.unwrap()
|
||||
.filter_map(|e| e.ok())
|
||||
.filter(|e| e.file_type().is_ok_and(|t| t.is_file()))
|
||||
.map(|e| e.file_name())
|
||||
.collect();
|
||||
assert!(cached.is_empty(), "{cached:?}");
|
||||
// The bytes kept for the handle went once the trash landed with it
|
||||
// closed. The queue empties as the trash's row goes, a moment before
|
||||
// the eviction, so this waits too.
|
||||
let cached = || -> Vec<_> {
|
||||
std::fs::read_dir(dir.join("cache"))
|
||||
.unwrap()
|
||||
.filter_map(|e| e.ok())
|
||||
.filter(|e| e.file_type().is_ok_and(|t| t.is_file()))
|
||||
.map(|e| e.file_name())
|
||||
.collect()
|
||||
};
|
||||
assert!(
|
||||
wait_until(Duration::from_secs(30), || cached().is_empty()),
|
||||
"{:?}",
|
||||
cached()
|
||||
);
|
||||
|
||||
assert!(matches!(daemon.stop(), Ok(MountOutcome::Shutdown)));
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
@@ -1998,6 +2006,21 @@ mod tests {
|
||||
daemon.write_new(&path, b"first").unwrap();
|
||||
assert!(wait_until(Duration::from_secs(30), || held.reached()));
|
||||
std::fs::write(&path, b"second").unwrap();
|
||||
// The kernel sends the release after `close` returns. Attaching the
|
||||
// bytes resets the create's attempts, so a release that came after the
|
||||
// lost reply had it retried at once, and the retry landed before
|
||||
// anything here saw it fail. The bytes it replaced go only once they
|
||||
// are attached.
|
||||
let staged = dir.join("cache").join("staging");
|
||||
assert!(wait_until(Duration::from_secs(30), || {
|
||||
let blobs: Vec<Vec<u8>> = std::fs::read_dir(&staged)
|
||||
.into_iter()
|
||||
.flatten()
|
||||
.filter_map(|e| e.ok())
|
||||
.filter_map(|e| std::fs::read(e.path()).ok())
|
||||
.collect();
|
||||
blobs.iter().any(|b| b == b"second") && !blobs.iter().any(|b| b == b"first")
|
||||
}));
|
||||
drop(held);
|
||||
assert!(wait_until(Duration::from_secs(30), || {
|
||||
daemon
|
||||
|
||||
@@ -68,6 +68,7 @@ original wording.
|
||||
|
||||
| Entry | Title | What is left |
|
||||
|---|---|---|
|
||||
| [B275](#b275--on-a-slow-link-a-files-last-write-can-land-only-in-a-conflict-copy) | On a slow link, a file's last write can land only in a conflict copy | Cause not found; seen once in CI |
|
||||
| [B273](#b273--a-restore-proposes-another-computers-paths-when-the-two-share-a-hostname) | A restore proposes another computer's paths when the two share a hostname | Not fixed |
|
||||
| [B270](#b270--one-file-left-in-a-mirror-folder-clears-the-wipe-guard-for-all-the-others) | One file left in a mirror folder clears the wipe guard for all the others | Not fixed |
|
||||
| [B267](#b267--a-key-failure-at-a-start-from-a-warm-cache-shows-as-offline-for-good) | A key failure at a start from a warm cache shows as offline for good | Not fixed |
|
||||
@@ -109,6 +110,7 @@ a live account since.
|
||||
|
||||
| Entry | Title |
|
||||
|---|---|
|
||||
| [B274](#b274--opening-a-file-for-writing-fails-with-eio-while-its-queued-revision-lands) | Opening a file for writing fails with EIO while its queued revision lands |
|
||||
| [B271](#b271--one-failed-listing-makes-a-large-first-sync-start-over-from-nothing) | One failed listing makes a large first sync start over from nothing |
|
||||
| [B266](#b266--pin-unpin-search-and-node-loads-report-a-database-error-as-offline) | Pin, unpin, search and node loads report a database error as "offline" |
|
||||
| [B264](#b264--a-mount-that-fails-after-starting-its-workers-leaves-them-running) | A mount that fails after starting its workers leaves them running |
|
||||
@@ -375,6 +377,56 @@ a live account since.
|
||||
|
||||
Newest first.
|
||||
|
||||
### B275 — On a slow link, a file's last write can land only in a conflict copy
|
||||
|
||||
**Status:** Open
|
||||
**Found:** 2026-10-31, CI on #60: `one_client_on_a_slow_link`, profile `one-client-wifi` seed 2,
|
||||
failed with `Drive does not hold what the clients wrote, 120s after the links came up`. The
|
||||
same seed passed 4 times locally, and #60 changes no daemon logic, so the failure depends on
|
||||
timing.
|
||||
**Where:** not located; the drain's conflict check in `crates/pdfs-fuse/src/drain.rs` is the
|
||||
first suspect
|
||||
|
||||
**Repro:** Only the CI run above. The last steps created `x/z/y/a.txt` (37 bytes), wrote 42
|
||||
bytes into it at offset 20, and brought every link up. Drive then held the file at its first
|
||||
37 bytes, and the full content was in `x/z/y/a (sync-conflict …).txt`. No other client wrote
|
||||
to it.
|
||||
|
||||
**Cause.** Not known. Nothing was lost, since the conflict copy holds the bytes, but the file
|
||||
conflicted with its own create. B183 and B184 found the same symptom after a lost create
|
||||
reply.
|
||||
|
||||
**Fix.** Not done. Needs a reproduction: run the seed in a loop under load and keep the state
|
||||
directory of a failing run.
|
||||
|
||||
---
|
||||
|
||||
### B274 — Opening a file for writing fails with EIO while its queued revision lands
|
||||
|
||||
**Status:** Fixed, not verified live
|
||||
**Found:** 2026-10-31, CI: the simulation
|
||||
`a_file_whose_upload_was_not_read_back_takes_two_writes_at_once` failed once with
|
||||
`Input/output error` on a write, after `copy queued revision into write scratch failed … No
|
||||
such file or directory`. It did not fail in 54 local runs.
|
||||
**Where:** the write open in `crates/pdfs-fuse/src/filesystem.rs`
|
||||
|
||||
**Repro:** Write a file, and open it for writing again at the moment its queued revision
|
||||
finishes uploading. The open fails with EIO, and the program sees its save fail.
|
||||
|
||||
**Cause.** The open samples the queued revision, then copies its blob into the new handle's
|
||||
scratch file. The drain lets go of the pending entry and then deletes the blob when the upload
|
||||
lands. A blob deleted between the sample and the copy failed the open, although the re-check
|
||||
just after the copy already retries a revision that changed during it.
|
||||
|
||||
**Fix.** A failed copy checks whether the pending entry still names that blob. When it does
|
||||
not, the revision landed or was replaced, and the open goes around again with the base the
|
||||
file has now, as the re-check does. Since the drain drops the entry before the blob, a blob
|
||||
that is gone and still named is a real failure and still returns EIO.
|
||||
|
||||
**Test:** none deterministic; the simulation above covers the window.
|
||||
|
||||
---
|
||||
|
||||
### B273 — A restore proposes another computer's paths when the two share a hostname
|
||||
|
||||
**Status:** Open
|
||||
|
||||
Reference in new issue
Block a user