fix(fuse): keep pinned files on disk through remote changes

A remote change to a pinned file called `ContentCache::evict`, which does
not check pins, and only the pin request itself ever downloaded anything.
An edit from another device, or even a rename, left the file listed as kept
offline with no copy, so the next read downloaded it or failed without a
network. A file added to a pinned folder elsewhere was never downloaded,
and a folder pin whose downloads failed while offline never retried them.

A remote change no longer evicts a pinned file's copy. The copy is checked
against the revision's (mtime, size), so a newer revision does not read
it, and a rename keeps the mtime (B123), so the copy stays valid. A new
pdfs-pin-keeper thread checks pins again and downloads files that are
missing or stale:

- the pins a batch of remote events changed, after the batch's listings
  are dropped, including a pinned folder that gets a new file;
- every pin at startup, when the link returns, and after the event feed
  loses continuity;
- a file whose version is restored.

Failed downloads retry with a backoff of 30 s to 30 min. Pinning and the
keeper settle a provisional size first: a copy stored under the
ciphertext size never matched the plaintext it holds.

Fixes B282.
This commit is contained in:
nvrl committed 2026-11-14 00:20:06 +01:00
1 parent 3f45e89391
commit 14a663cee9
10 files changed
+460 -15

No files matched your search

+35 -1
View File
@@ -157,6 +157,17 @@ fn apply_event(core: &Core, event: &DriveEvent, dirty: &mut DirtyParents) -> pdf
if ours {
debug!(uid = %node_uid, "ignoring remote event for a node with a queued write");
}
// A file kept offline keeps its copy through a change, and the
// keeper replaces it once this batch's listings are read again.
// The copy is checked against the revision's `(mtime, size)`, so
// no read takes it for a newer revision. Evicted, it was gone until
// something read the file, which then downloaded it (B282). A
// node new in, or moved into, a kept folder is fetched by checking
// that folder.
let kept = match !*is_trashed && !ours {
true => kept_by_pin(core, node_uid, parent_node_uid.as_ref()),
false => None,
};
let mut had_node = false;
core.for_each_mount(|st, notify| {
if *is_trashed {
@@ -172,9 +183,12 @@ fn apply_event(core: &Core, event: &DriveEvent, dirty: &mut DirtyParents) -> pdf
}
});
// Once, not once per mount: the content cache is per-daemon.
if had_node {
if had_node && kept.is_none() {
core.cache.evict(node_uid);
}
if let Some(kept) = kept {
dirty.kept.insert(kept);
}
// A create, rename or move-in shows up as a change to the parent
// listing too, which is recorded for the end of the batch rather
// than acted on here — see [`DirtyParents`].
@@ -205,6 +219,8 @@ fn apply_event(core: &Core, event: &DriveEvent, dirty: &mut DirtyParents) -> pdf
| DriveEvent::ScopeAccessLost { .. }
| DriveEvent::SharedWithMeUpdated { .. } => {
warn!("event access continuity lost; downgrading shared trees and resyncing lazily");
// Changes went by unseen, kept files among them (B282).
core.recheck_all_pins();
apply_result = apply_result.and(core.db.clear_state(SHARED_WITH_ME_SYNCED_MS));
core.for_each_mount(|st, notify| {
let dirs: Vec<u64> = st.children.keys().copied().collect();
@@ -296,6 +312,21 @@ struct DirtyParents {
/// Checked per mount — a mount that already lists every one of those
/// children under that parent keeps its listing.
ours: HashMap<NodeUid, HashSet<NodeUid>>,
/// Pinned files and folders the batch changed, for the pin keeper to
/// check once the listings above are dropped (`keeper`).
kept: HashSet<NodeUid>,
}
/// What the pin keeper has to check after a remote change to `node_uid`,
/// whose parent is now `parent_uid`: the node itself when a pin keeps it, or
/// else the parent, when a pinned folder holds it and the node is new there.
fn kept_by_pin(core: &Core, node_uid: &NodeUid, parent_uid: Option<&NodeUid>) -> Option<NodeUid> {
if core.cache.is_pinned(node_uid) {
return Some(node_uid.clone());
}
parent_uid
.filter(|parent| core.cache.is_pinned(parent))
.cloned()
}
/// Whether this mount's cached listing of `parent_uid` already accounts for
@@ -476,6 +507,9 @@ pub(super) async fn run_event_sync(
cursor = Some(applied);
}
flush_dirty_parents(&core, &dirty);
for uid in &dirty.kept {
core.recheck_pinned(uid);
}
// Another device changed the tree: whatever a front-end lists may have
// moved, including what is shared and what is in the trash.
core.events
+271
View File
@@ -0,0 +1,271 @@
//! Keeping pinned files on this computer after they were pinned.
//!
//! A pin downloads a file once. Nothing downloaded it again afterwards: a
//! change from another device deleted the cached copy, a file added to a
//! pinned folder elsewhere was never fetched, and a download that failed while
//! the link was down stayed failed. Each of those left a file listed as kept
//! offline that the next read downloaded, and that could not be read at all
//! without a network (`docs/BUGS.md` B282).
//!
//! The keeper is one thread that checks pins again and downloads what is
//! missing or stale: every pin when the daemon starts and when the link comes
//! back, and the pins a batch of remote events touched once the batch is
//! applied. A remote change no longer deletes a pinned file's copy. The copy
//! is stored under the revision's `(mtime, size)`, so a read of a newer
//! revision does not take it, and the keeper replaces it once the new one is
//! downloaded.
use std::collections::HashSet;
use std::time::{Duration, Instant};
use proton_drive_rs::NodeKind;
use proton_drive_rs::proton_sdk::ids::NodeUid;
use tracing::{debug, info, warn};
use super::{Core, is_local_uid, node_size, parse_uid};
/// How often the keeper looks for work.
const KEEPER_TICK: Duration = Duration::from_secs(1);
/// The first wait before a check that could not download everything is tried
/// again. It doubles on every failure, up to [`RETRY_MAX`].
const RETRY_FIRST: Duration = Duration::from_secs(30);
const RETRY_MAX: Duration = Duration::from_secs(30 * 60);
/// What the keeper owes a check.
#[derive(Debug, Default)]
pub(crate) struct PinWork {
/// Every pin.
all: bool,
/// The pinned files and folders named here. A folder is checked whole.
nodes: HashSet<NodeUid>,
}
impl PinWork {
fn is_empty(&self) -> bool {
!self.all && self.nodes.is_empty()
}
fn merge(&mut self, other: PinWork) {
self.all |= other.all;
self.nodes.extend(other.nodes);
}
}
/// A file a pin keeps, as the tree knows it now.
pub(crate) struct KeptFile {
pub(crate) uid: NodeUid,
pub(crate) name: String,
pub(crate) mtime: i64,
pub(crate) size: u64,
}
impl Core {
/// Have the keeper check every pin.
pub(crate) fn recheck_all_pins(&self) {
self.pin_work.lock().all = true;
}
/// Have the keeper check the pinned file or folder `uid`.
pub(crate) fn recheck_pinned(&self, uid: &NodeUid) {
self.pin_work.lock().nodes.insert(uid.clone());
}
/// The keeper's loop, until the daemon stops. Starts with a check of every
/// pin, which catches up on whatever changed while the daemon was down.
///
/// Work that failed waits out a backoff of its own. New work does not wait
/// for it, so a file that keeps failing does not hold up the next change.
pub(crate) fn run_pin_keeper(&self) {
self.recheck_all_pins();
let mut backlog = PinWork::default();
let mut backlog_due = Instant::now();
let mut retry = RETRY_FIRST;
while self.shutdown.sleep(KEEPER_TICK) {
if !self.is_online() {
continue;
}
let mut work = std::mem::take(&mut *self.pin_work.lock());
let retrying = !backlog.is_empty() && Instant::now() >= backlog_due;
if retrying {
work.merge(std::mem::take(&mut backlog));
}
if work.is_empty() {
continue;
}
let failed = self.keep_pins(&work);
if failed.is_empty() {
if retrying {
retry = RETRY_FIRST;
}
continue;
}
backlog.merge(failed);
backlog_due = Instant::now() + retry;
retry = (retry * 2).min(RETRY_MAX);
}
}
/// Download every file `work` covers whose cached copy is missing or
/// stale. Returns what could not be checked or downloaded.
fn keep_pins(&self, work: &PinWork) -> PinWork {
let mut failed = PinWork::default();
let roots: Vec<NodeUid> = if work.all {
self.cache
.list_pins()
.iter()
.filter_map(|pin| parse_uid(&pin.uid))
.chain(work.nodes.iter().cloned())
.collect()
} else {
work.nodes.iter().cloned().collect()
};
let mut seen: HashSet<u64> = HashSet::new();
let mut files: Vec<u64> = Vec::new();
for uid in roots {
if self.shutdown.is_stopping() {
return failed;
}
if !self.cache.is_pinned(&uid) {
continue;
}
let found = {
let st = self.state();
st.by_uid.get(&uid).and_then(|&ino| {
let e = st.entries.get(&ino)?;
Some((ino, e.parent, e.node.is_folder()))
})
};
let Some((ino, parent, is_folder)) = found else {
debug!(%uid, "pin keeper: pinned node is not in the tree");
continue;
};
if is_folder {
match self.pin_subtree_files(ino) {
Ok(inos) => files.extend(inos.into_iter().filter(|i| seen.insert(*i))),
Err(e) => {
warn!(%uid, error = %e, "pin keeper: listing a kept folder failed");
failed.nodes.insert(uid);
}
}
continue;
}
// An event about this file dropped its parent's listing. Listing
// it again brings the file's entry up to the revision Drive has
// now, which is what the cached copy is checked against.
if !work.all
&& parent != ino
&& let Err(e) = self.ensure_children(parent)
{
warn!(%uid, error = ?e, "pin keeper: listing a kept file's folder failed");
failed.nodes.insert(uid);
continue;
}
if seen.insert(ino) {
files.push(ino);
}
}
let stale: Vec<KeptFile> = files
.into_iter()
.filter_map(|ino| self.kept_file(ino))
.filter(|f| !self.cache.is_cached(&f.uid, f.mtime, f.size))
.collect();
if stale.is_empty() {
return failed;
}
let job = self.transfers.begin_job("Updating files kept offline");
job.set_total(stale.len() as u64);
let (mut fetched, mut failures) = (0usize, 0usize);
for file in stale {
if self.shutdown.is_stopping() {
break;
}
// Unpinned while the check ran.
if !self.cache.is_pinned(&file.uid) {
job.step();
continue;
}
job.detail(file.name.clone());
match self.fetch_pinned(&file.uid, &file.name, file.mtime, file.size) {
Ok(()) => fetched += 1,
Err(e) => {
warn!(uid = %file.uid, error = %e, "pin keeper: download failed");
failed.nodes.insert(file.uid);
failures += 1;
}
}
job.step();
}
info!(
fetched,
failed = failures,
"pin keeper: brought files kept offline up to date"
);
failed
}
/// The file at `ino`, as a pin keeps it. `None` for a folder, a file this
/// daemon still owes an upload for, or a file whose real size cannot be
/// read.
///
/// A queued write is ahead of Drive. Downloading Drive's revision would
/// cache what the write replaces, and the drain keeps the uploaded bytes
/// of a pinned file once they land.
///
/// A listing carries the ciphertext size until its size upgrade lands
/// (B12). Stored under that size, the copy never matched the plaintext it
/// holds, so every read downloaded the file again and every check
/// downloaded it once more.
pub(crate) fn kept_file(&self, ino: u64) -> Option<KeptFile> {
let (uid, name, mtime, size, provisional) = {
let st = self.state();
let e = st.entries.get(&ino)?;
let NodeKind::File { claimed_size, .. } = &e.node.kind else {
return None;
};
(
e.uid.clone(),
e.node.name.clone(),
e.node.modification_time,
node_size(&e.node),
claimed_size.is_none(),
)
};
if is_local_uid(&uid) || self.pending.lock().contains_key(&uid) {
return None;
}
let size = match provisional {
true => self.resolve_provisional_size(ino, &uid)?,
false => size,
};
Some(KeptFile {
uid,
name,
mtime,
size,
})
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn work_that_failed_keeps_every_node_and_the_full_check() {
let mut backlog = PinWork::default();
assert!(backlog.is_empty());
let uid = parse_uid("vol~a").unwrap();
backlog.merge(PinWork {
all: false,
nodes: HashSet::from([uid.clone()]),
});
backlog.merge(PinWork {
all: true,
nodes: HashSet::from([uid.clone()]),
});
assert!(backlog.all);
assert_eq!(backlog.nodes.len(), 1);
assert!(!backlog.is_empty());
}
}
+24 -14
View File
@@ -91,6 +91,7 @@ mod drive;
use drive::{DriveApi, RevisionRead};
mod events;
mod filesystem;
mod keeper;
mod link;
pub use filesystem::ProtonFs;
mod mount;
@@ -589,6 +590,8 @@ struct Core {
/// Nudges the drain workers: set true and notify to have them re-examine
/// the queue instead of waiting out their backoff.
drain_wake: Arc<(Mutex<bool>, Condvar)>,
/// The pins the keeper owes a check (`keeper`).
pin_work: Arc<Mutex<keeper::PinWork>>,
/// The daemon's stop signal, shared with every background loop this `Core`
/// was cloned into so teardown can end and join them (`docs/BUGS.md` B44).
shutdown: Arc<shutdown::Shutdown>,
@@ -4309,6 +4312,12 @@ impl Core {
.map_err(|e| CoreError::internal(format!("spawn pin: {e}")))?;
return Ok(format!("{name}: downloading its files in the background"));
}
// Stored under a provisional size, the copy would never match the
// plaintext it holds (`docs/BUGS.md` B282).
let size = match self.kept_file(ino) {
Some(file) if file.uid == uid => file.size,
_ => size,
};
self.fetch_content(&uid, &name, mtime, size)?;
self.cache
.add_pin(&uid, recorded, false)
@@ -4329,14 +4338,20 @@ impl Core {
Ok(files) => {
job.set_total(files.len() as u64);
let mut failed = 0usize;
for (uid, file, mtime, size) in files {
for ino in files {
if self.shutdown.is_stopping() || !self.cache.is_pinned(folder) {
return;
}
job.detail(file.clone());
if let Err(e) = self.fetch_pinned(&uid, &file, mtime, size) {
warn!(%uid, error = %e, "pin folder: download failed");
failed += 1;
// A file with a queued write, or no readable size, is
// left to the drain and the keeper (`keeper::kept_file`).
if let Some(file) = self.kept_file(ino) {
job.detail(file.name.clone());
if let Err(e) =
self.fetch_pinned(&file.uid, &file.name, file.mtime, file.size)
{
warn!(uid = %file.uid, error = %e, "pin folder: download failed");
failed += 1;
}
}
job.step();
}
@@ -4370,9 +4385,9 @@ impl Core {
Ok(())
}
/// Every file in the subtree rooted at folder `ino`.
fn pin_subtree_files(&self, ino: u64) -> CoreResult<Vec<(NodeUid, String, i64, u64)>> {
let mut files: Vec<(NodeUid, String, i64, u64)> = Vec::new();
/// The inode of every file in the subtree rooted at folder `ino`.
fn pin_subtree_files(&self, ino: u64) -> CoreResult<Vec<u64>> {
let mut files: Vec<u64> = Vec::new();
let mut stack = vec![ino];
while let Some(dir) = stack.pop() {
if self.shutdown.is_stopping() {
@@ -4387,12 +4402,7 @@ impl Core {
if e.node.is_folder() {
stack.push(k);
} else {
files.push((
e.uid.clone(),
e.node.name.clone(),
e.node.modification_time,
node_size(&e.node),
));
files.push(k);
}
}
}
+3
View File
@@ -286,6 +286,9 @@ impl Core {
warn!(error = %e, "moving backed-off ops up after a reconnect failed");
}
self.wake_drain();
// Downloads for pins failed while the link was down, and the events
// that would have named the files are behind us (`docs/BUGS.md` B282).
self.recheck_all_pins();
}
/// Wait up to `budget` for the link to come back. Returns whether it is up.
+12
View File
@@ -511,6 +511,7 @@ pub(crate) fn mount_with(
landed: Arc::new(Mutex::new(HashMap::new())),
unadopted: Arc::new(Mutex::new(HashMap::new())),
drain_wake: Arc::new((Mutex::new(false), Condvar::new())),
pin_work: Arc::new(Mutex::new(Default::default())),
shutdown: Arc::new(crate::shutdown::Shutdown::default()),
upload_times: Arc::new(Mutex::new(HashMap::new())),
upload_cancel: Arc::new(Mutex::new(HashMap::new())),
@@ -621,6 +622,17 @@ pub(crate) fn mount_with(
keep_or_abandon(&core, control_socket, &mut workers, spawned)?;
}
// Keep pinned files on this computer through changes made elsewhere, and
// fetch the ones a failed or missed download left out (`docs/BUGS.md`
// B282). Its own thread: a check lists folders and downloads files.
{
let keeper_core = core.clone();
let spawned = std::thread::Builder::new()
.name("pdfs-pin-keeper".into())
.spawn(move || keeper_core.run_pin_keeper());
keep_or_abandon(&core, control_socket, &mut workers, spawned)?;
}
// Keep the launcher prompt's "This computer" index fresh. Its own thread:
// the walk is I/O-heavy and must never sit in front of a FUSE callback.
if matches!(host, Host::Daemon) {
+5
View File
@@ -111,6 +111,11 @@ impl Core {
// stale from here on, and the server may not have applied the swap yet.
self.cache.evict(uid);
self.evict_reader(uid);
// A file kept offline gets the restored content back on disk, and
// again when the event of the swap arrives (`docs/BUGS.md` B282).
if self.cache.is_pinned(uid) {
self.recheck_pinned(uid);
}
self.log_activity(
ActivityKind::Restore,
name,
+62
View File
@@ -3497,6 +3497,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_kept_offline_stays_on_disk_through_changes_made_elsewhere() {
// A change from another device deleted a pinned file's copy and
// nothing downloaded it again, and a file added to a pinned folder
// elsewhere was never downloaded: both were listed as kept offline
// and read only with a network (B282).
let drive = FakeDrive::new();
let dir = scratch("pin-keeper");
let daemon = Daemon::start(&dir, drive.client(1, Faults::lan())).unwrap();
std::fs::create_dir(daemon.mountpoint.join("Docs")).unwrap();
std::fs::write(daemon.mountpoint.join("Docs/a.txt"), b"first").unwrap();
std::fs::write(daemon.mountpoint.join("solo.txt"), b"alone").unwrap();
assert!(wait_until(Duration::from_secs(30), || {
daemon.pending().is_ok_and(|items| items.is_empty())
&& drive.lookup("Docs/a.txt").is_some()
&& drive.lookup("solo.txt").is_some()
}));
daemon.request(&Request::PurgeCache).unwrap();
let entry = |folder: &str, name: &str| match daemon.request(&Request::ListDir {
path: folder.into(),
}) {
Ok(Response::Entries { entries }) => entries.into_iter().find(|e| e.name == name),
_ => None,
};
for path in ["Docs", "solo.txt"] {
let pinned = daemon.request(&Request::Pin { path: path.into() }).unwrap();
assert!(matches!(pinned, Response::Ok { .. }), "{pinned:?}");
}
assert!(wait_until(Duration::from_secs(30), || {
entry("Docs", "a.txt").is_some_and(|e| e.cached)
&& entry("", "solo.txt").is_some_and(|e| e.cached)
}));
// Another device edits both files and adds one to the kept folder.
drive.device().write("Docs/a.txt", b"second, and longer");
drive.device().write("solo.txt", b"alone, edited");
drive.device().write("Docs/new.txt", b"made elsewhere");
assert!(
wait_until(Duration::from_secs(60), || {
entry("Docs", "a.txt").is_some_and(|e| e.cached && e.size == 18)
&& entry("", "solo.txt").is_some_and(|e| e.cached && e.size == 13)
&& entry("Docs", "new.txt").is_some_and(|e| e.cached)
}),
"the keeper downloaded the new content: {:?} {:?} {:?}",
entry("Docs", "a.txt"),
entry("", "solo.txt"),
entry("Docs", "new.txt"),
);
// With the link gone, only the kept copies can answer.
daemon.client.set_online(false);
let read = |rel: &str| std::fs::read(daemon.mountpoint.join(rel)).unwrap();
assert_eq!(read("Docs/a.txt"), b"second, and longer");
assert_eq!(read("solo.txt"), b"alone, edited");
assert_eq!(read("Docs/new.txt"), b"made elsewhere");
daemon.client.set_online(true);
assert!(matches!(daemon.stop(), Ok(MountOutcome::Shutdown)));
let _ = std::fs::remove_dir_all(&dir);
}
/// The synced folder `id` as the daemon lists it.
fn listed_folder(daemon: &Daemon, id: i64) -> Option<control::SyncFolderInfo> {
match daemon.request(&Request::ListSyncFolders) {