diff --git a/crates/pdfs-fuse/src/control.rs b/crates/pdfs-fuse/src/control.rs index e7beaac..f3a4918 100644 --- a/crates/pdfs-fuse/src/control.rs +++ b/crates/pdfs-fuse/src/control.rs @@ -315,6 +315,9 @@ impl Drop for RefreshGuard { pub(crate) fn status_response(core: &Core, username: &str, mountpoint: &Path) -> CtlResponse { let pins = core.cache.list_pins(); let queued = core.db.pending_op_counts().unwrap_or_default(); + // Read after the queue: the drain counts an upload as landing before it + // retires the op, so one of the two always has it (B119). + let landing = core.landing_uploads.load(Ordering::SeqCst); let staging = core.cache.staging_usage(); CtlResponse::Status { parked_uploads: queued.parked.max(0) as u64, @@ -331,7 +334,7 @@ pub(crate) fn status_response(core: &Core, username: &str, mountpoint: &Path) -> budget: core.cache.budget(), pins, online: core.online.load(Ordering::Relaxed), - pending_uploads: queued.uploads.max(0) as u64, + pending_uploads: queued.uploads.max(0) as u64 + landing, pending_changes: queued.changes.max(0) as u64, } } diff --git a/crates/pdfs-fuse/src/drain.rs b/crates/pdfs-fuse/src/drain.rs index 037a488..4845381 100644 --- a/crates/pdfs-fuse/src/drain.rs +++ b/crates/pdfs-fuse/src/drain.rs @@ -24,7 +24,7 @@ use std::collections::HashMap; use std::fs::File; use std::path::{Path, PathBuf}; use std::sync::Arc; -use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::time::{Duration, Instant}; use parking_lot::Mutex; @@ -113,6 +113,23 @@ impl Drop for UploadCancel { } } +/// One upload counted in [`Core::landing_uploads`] from its op's retirement +/// until its bytes are cached, and uncounted when dropped, early return or not. +struct LandingUpload(Arc); + +impl LandingUpload { + fn begin(count: &Arc) -> Self { + count.fetch_add(1, Ordering::SeqCst); + Self(count.clone()) + } +} + +impl Drop for LandingUpload { + fn drop(&mut self) { + self.0.fetch_sub(1, Ordering::SeqCst); + } +} + /// The debounce arithmetic of [`Core::revision_debounce`], without the map: /// the last measured upload time for this node, bounded on both sides. /// @@ -1635,6 +1652,14 @@ impl Core { // is what the next write's debounce is scaled from. self.record_upload_time(&uid, took); + // Still pending to whoever asks until the bytes are cached below, and + // counted before the op goes so the status never reads empty in between. + // An empty queue is how a script or the app knows a written file reads + // without the network; retiring the op alone left the read-back's round + // trip in which it did not (B119). The op itself has to go first: one + // restored after a crash would re-send a revision this daemon has not + // yet recorded as its own, and come back as a conflict copy. + let _landing = LandingUpload::begin(&self.landing_uploads); // Retire the op before dropping the blob: a crash between the two leaves // an orphaned file (harmless), whereas the reverse would leave a queued // op pointing at nothing. @@ -2337,6 +2362,22 @@ mod tests { assert!(keeps_landed_upload(false, 4 * GIB, 0)); } + #[test] + fn a_landing_upload_counts_until_its_guard_goes() { + let count = Arc::new(AtomicU64::new(0)); + let first = LandingUpload::begin(&count); + let landed = || -> Result<(), ()> { + let _second = LandingUpload::begin(&count); + assert_eq!(count.load(Ordering::SeqCst), 2); + Err(()) + }; + // Uncounted on an early return too, or the queue never reads empty. + assert!(landed().is_err()); + assert_eq!(count.load(Ordering::SeqCst), 1); + drop(first); + assert_eq!(count.load(Ordering::SeqCst), 0); + } + fn baseline(mtime: i64, size: u64, rev: Option<&str>) -> Baseline { Baseline { mtime, diff --git a/crates/pdfs-fuse/src/lib.rs b/crates/pdfs-fuse/src/lib.rs index ffe6621..100aa37 100644 --- a/crates/pdfs-fuse/src/lib.rs +++ b/crates/pdfs-fuse/src/lib.rs @@ -575,6 +575,12 @@ struct Core { /// replaces. Read by the upload's own reader ([`CountingReader`]), which is /// the one thing the SDK calls often enough to notice. upload_cancel: Arc>>>, + /// Uploads whose op is retired but whose bytes the content cache has not + /// adopted yet, because the revision just sealed is still being read back. + /// The status counts them as pending, so an empty queue means every file + /// written through the mount reads without the network (`docs/BUGS.md` + /// B119). + landing_uploads: Arc, /// True while a background refresh of the photos timeline (resp. the trash) is /// already running, so a burst of page requests against a stale listing kicks /// off one refresh rather than one per request. @@ -8624,6 +8630,24 @@ mod tests { assert_before(upload, "self.db.op_exists(op.id)", "File::open(&blob)"); } + #[test] + fn a_landed_upload_stays_pending_until_its_bytes_are_cached() { + let drain = function_source(include_str!("drain.rs"), "pub(crate) fn drain_revision("); + assert_before( + drain, + "let _landing = LandingUpload::begin(&self.landing_uploads)", + "self.db.delete_op(op.id)", + ); + assert_before(drain, "self.db.delete_op(op.id)", ".store_file(&uid,"); + let status = function_source(include_str!("control.rs"), "fn status_response("); + assert_before( + status, + "core.db.pending_op_counts()", + "core.landing_uploads.load(", + ); + assert!(status.contains("queued.uploads.max(0) as u64 + landing")); + } + #[test] fn replacement_rename_authorizes_source_and_both_parents() { use std::cell::RefCell; diff --git a/crates/pdfs-fuse/src/mount.rs b/crates/pdfs-fuse/src/mount.rs index 038f503..3f02037 100644 --- a/crates/pdfs-fuse/src/mount.rs +++ b/crates/pdfs-fuse/src/mount.rs @@ -391,6 +391,7 @@ pub fn mount( shutdown: Arc::new(crate::shutdown::Shutdown::default()), upload_times: Arc::new(Mutex::new(HashMap::new())), upload_cancel: Arc::new(Mutex::new(HashMap::new())), + landing_uploads: Arc::new(AtomicU64::new(0)), timeline_refreshing: Arc::new(AtomicBool::new(false)), timeline_progress: Arc::new(Mutex::new(None)), similar_hashing: Arc::new(AtomicBool::new(false)), diff --git a/docs/BUGS.md b/docs/BUGS.md index 078df88..15a4951 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -75,7 +75,8 @@ offline catches this race, and B117, if either comes back. ## B119 — Reading a file just written through the mount needs the network -**Status:** Fixed (verified 2026-10-03: "regression B119" passed in My files on the account run). +**Status:** Fixed (unverified since the second fix below; "regression B119" passed on one account +run of 2026-10-03 and failed on the next). **Found:** 2026-10-02, by the account run on flaky Wi-Fi, which crashed at its very end. The run's last step reads back a file it wrote to compare digests. The Wi-Fi had dropped, the read waited for `open_revision` for 120 seconds and then failed with `EIO`, and the suite stopped with @@ -96,9 +97,25 @@ discarded unless it is pinned, so one large upload cannot push the rest of the c is kept when the read-back fails or reports a different size. The suite records a failed end-of-run read-back as a failed check instead of crashing. -**Test:** the drain test `a_landed_upload_stays_cached_unless_it_would_crowd_the_cache_out`, and -the acceptance case "regression B119", which writes two files, waits for the queue and expects -`pdfs ls` to list both as cached. +**Found again** on 2026-10-03, by "regression B119" on an account run over a slow link (uploads of +a few bytes took 1.4 seconds). The case failed with "block.bin not cached after the upload" at the +same second the journal logged that upload as landed. + +**Cause, second part.** The drain retired the op first, and only then read the node back and +linked the blob into the cache. For that one round trip the status reported an empty queue for a +file that was not yet cached. Anything that waits for an empty queue before reading could still +need the network. + +**Fix, second part.** `Core::landing_uploads` counts an upload from just before its op is retired +until its bytes are cached, and the status adds it to `pending_uploads`. The status reads the +queue before the count, so it always sees the upload in one of them. The op still goes first: an +op restored after a crash would re-send a revision this daemon has not yet recorded as its own, +and `revision_conflict` would keep it as a conflict copy. + +**Test:** the drain tests `a_landed_upload_stays_cached_unless_it_would_crowd_the_cache_out` and +`a_landing_upload_counts_until_its_guard_goes`, `a_landed_upload_stays_pending_until_its_bytes_are_cached` +in `lib.rs`, and the acceptance case "regression B119", which writes two files, waits for the +queue and expects `pdfs ls` to list both as cached. ## B118 — Creating a file or folder through the mount can fail with `ENOENT` although it was made