diff --git a/crates/pdfs-core/src/db/nodes.rs b/crates/pdfs-core/src/db/nodes.rs index d5bebda..7c13d1e 100644 --- a/crates/pdfs-core/src/db/nodes.rs +++ b/crates/pdfs-core/src/db/nodes.rs @@ -17,6 +17,8 @@ use super::utils::{ pub struct StoredNode { pub node: Node, pub listed: bool, + /// The row's local id ([`Db::lid_of`]). + pub lid: i64, } /// One full-text search match: the stored [`Node`] plus its mountpoint-relative @@ -289,7 +291,8 @@ impl Db { let uid_str = parent_uid.to_string(); let conn = self.read(); let count: i64 = conn.query_row( - "SELECT COUNT(*) FROM nodes WHERE parent_uid = ?1 AND trashed = 0", + "SELECT COUNT(*) FROM nodes + WHERE parent_uid = ?1 AND trashed = 0 AND node_json IS NOT NULL", params![uid_str], |row| row.get(0), )?; @@ -305,9 +308,11 @@ impl Db { pub fn node_path(&self, uid: &str) -> Result> { let conn = self.read(); let exists: Option = conn - .query_row("SELECT 1 FROM nodes WHERE uid = ?1", params![uid], |r| { - r.get(0) - }) + .query_row( + "SELECT 1 FROM nodes WHERE uid = ?1 AND node_json IS NOT NULL", + params![uid], + |r| r.get(0), + ) .optional()?; if exists.is_none() { return Ok(None); @@ -511,17 +516,17 @@ impl Db { pub fn load_all(&self) -> Result> { let conn = self.read(); let mut stmt = - conn.prepare("SELECT node_json, listed FROM nodes WHERE node_json IS NOT NULL")?; + conn.prepare("SELECT node_json, listed, lid FROM nodes WHERE node_json IS NOT NULL")?; let rows = stmt.query_map([], |row| { let json: String = row.get(0)?; let listed: i64 = row.get(1)?; - Ok((json, listed != 0)) + Ok((json, listed != 0, row.get(2)?)) })?; let mut out = Vec::new(); for row in rows { - let (json, listed) = row?; + let (json, listed, lid) = row?; let node: Node = serde_json::from_str(&json)?; - out.push(StoredNode { node, listed }); + out.push(StoredNode { node, listed, lid }); } Ok(out) } @@ -556,6 +561,45 @@ impl Db { .optional()?) } + /// The local id of each node, in order, giving a row to each that has none. + /// + /// A new row is only a stub: it has no `node_json`, so no listing or search + /// returns it until the node is upserted, which the caller still owes. That + /// keeps this cheap enough to call with the inode lock held, which is + /// where an inode number is first needed. + pub fn lids_for(&self, nodes: &[&Node]) -> Result> { + let mut conn = self.conn.lock(); + let tx = conn.transaction()?; + let mut lids = Vec::with_capacity(nodes.len()); + { + let mut known = tx.prepare_cached("SELECT lid FROM nodes WHERE uid = ?1")?; + let mut stub = tx.prepare_cached( + "INSERT INTO nodes (uid, parent_uid, name, is_dir, mtime, trashed) + VALUES (?1, ?2, ?3, ?4, ?5, ?6)", + )?; + for node in nodes { + let uid = node.uid.to_string(); + let lid = match known.query_row(params![uid], |r| r.get(0)).optional()? { + Some(lid) => lid, + None => { + stub.execute(params![ + uid, + node.parent_uid.as_ref().map(ToString::to_string), + node.name, + node.is_folder() as i64, + node.modification_time, + node.trashed as i64, + ])?; + tx.last_insert_rowid() + } + }; + lids.push(lid); + } + } + tx.commit()?; + Ok(lids) + } + /// Read the persisted incremental-sync cursor (a `DriveEventId`), if any. /// The daemon resumes from this on restart instead of reseeding to the /// server head, so changes made while unmounted are still applied (P2). @@ -816,8 +860,10 @@ fn reindex_subtree_tx( let mut set_path = tx.prepare("UPDATE nodes SET path = ?2 WHERE rowid = ?1")?; let mut unindex = tx.prepare("DELETE FROM nodes_fts WHERE rowid = ?1")?; let mut index = tx.prepare( + // Not a stub row (`Db::lids_for`): it is indexed when its node is + // written. "INSERT INTO nodes_fts (rowid, name, path) - SELECT rowid, name, ?2 FROM nodes WHERE rowid = ?1", + SELECT rowid, name, ?2 FROM nodes WHERE rowid = ?1 AND node_json IS NOT NULL", )?; for (rowid, path, blocked) in descendants { set_path.execute(params![rowid, path])?; diff --git a/crates/pdfs-core/src/db/share_access.rs b/crates/pdfs-core/src/db/share_access.rs index d8a17aa..6779856 100644 --- a/crates/pdfs-core/src/db/share_access.rs +++ b/crates/pdfs-core/src/db/share_access.rs @@ -98,7 +98,7 @@ impl Db { pub fn effective_node_access(&self, uid: &NodeUid) -> Result> { let conn = self.read(); let exists: bool = conn.query_row( - "SELECT EXISTS(SELECT 1 FROM nodes WHERE uid = ?1)", + "SELECT EXISTS(SELECT 1 FROM nodes WHERE uid = ?1 AND node_json IS NOT NULL)", [uid.to_string()], |row| row.get(0), )?; diff --git a/crates/pdfs-core/src/db/tests.rs b/crates/pdfs-core/src/db/tests.rs index 390fa78..27b02bd 100644 --- a/crates/pdfs-core/src/db/tests.rs +++ b/crates/pdfs-core/src/db/tests.rs @@ -3145,6 +3145,31 @@ fn a_node_keeps_its_lid_across_updates_and_a_new_one_never_reuses_it() { assert!(c > b, "lid {c} reuses one a deleted row had"); } +#[test] +fn a_lid_handed_out_before_the_node_is_stored_reads_as_no_node() { + let db = Db::open_in_memory().unwrap(); + db.upsert_node(&folder("root", None, "My Files")).unwrap(); + db.set_share_access(&uid("root"), crate::Access::Owner) + .unwrap(); + let stub = file("stub", "root", "stub.txt", 1); + let [lid] = db.lids_for(&[&stub]).unwrap()[..] else { + panic!("one lid per node"); + }; + + assert_eq!(db.lid_of(&uid("stub").to_string()).unwrap(), Some(lid)); + assert_eq!(db.lids_for(&[&stub]).unwrap(), vec![lid]); + assert!(db.node_by_uid(&uid("stub").to_string()).unwrap().is_none()); + assert!(db.effective_node_access(&uid("stub")).unwrap().is_none()); + assert!(db.node_path(&uid("stub").to_string()).unwrap().is_none()); + assert!(!db.has_children(&uid("root")).unwrap()); + assert!(db.search("stub", 10).unwrap().is_empty()); + assert!(db.load_all().unwrap().iter().all(|n| n.lid != lid)); + + db.upsert_node(&stub).unwrap(); + assert_eq!(db.lid_of(&uid("stub").to_string()).unwrap(), Some(lid)); + assert!(db.has_children(&uid("root")).unwrap()); +} + #[test] fn a_landed_placeholder_keeps_its_lid() { let db = Db::open_in_memory().unwrap(); diff --git a/crates/pdfs-fuse/src/devices.rs b/crates/pdfs-fuse/src/devices.rs index 2579f28..cd22140 100644 --- a/crates/pdfs-fuse/src/devices.rs +++ b/crates/pdfs-fuse/src/devices.rs @@ -625,7 +625,7 @@ impl Core { let mut fork = self.clone(); fork.primary = false; let share_access = self.state().share_access.clone(); - fork.state = Arc::new(Mutex::new(State::new(self.db.clone(), share_access, 2))); + fork.state = Arc::new(Mutex::new(State::new(self.db.clone(), share_access))); // A fresh inode space needs a fresh notification channel: this fork's // session is the only one that knows these inodes, so it must be the one // notified about them. Filled in by `spawn_ondemand_mount`. diff --git a/crates/pdfs-fuse/src/lib.rs b/crates/pdfs-fuse/src/lib.rs index 3ddf1c0..587bf5f 100644 --- a/crates/pdfs-fuse/src/lib.rs +++ b/crates/pdfs-fuse/src/lib.rs @@ -1754,21 +1754,23 @@ impl Core { let hidden = self.hidden.lock().clone(); let mut st = self.state(); - // Pass 1: assign a stable inode to every uid (root is already mapped). + // Pass 1: give every uid its inode from its local id (the root is + // already mapped). for sn in &stored { if hidden.contains(&sn.node.uid) || st.by_uid.contains_key(&sn.node.uid) { continue; } - let ino = st.next_ino; - st.next_ino += 1; - st.by_uid.insert(sn.node.uid.clone(), ino); + st.by_uid + .insert(sn.node.uid.clone(), state::lid_ino(sn.lid)); } // Pass 2: materialize entries, resolving each parent's inode by uid. // Track folders flagged complete so their listings rebuild in pass 3. let mut listed_dirs: Vec = Vec::new(); for sn in stored { - let StoredNode { mut node, listed } = sn; + let StoredNode { + mut node, listed, .. + } = sn; if hidden.contains(&node.uid) { continue; } @@ -8868,16 +8870,27 @@ mod tests { std::fs::create_dir_all(&dir_path).unwrap(); let db = pdfs_core::db::Db::open(&dir_path.join("cache.db")).unwrap(); let share_access = db.all_share_access().unwrap(); - let st = crate::state::State::new(std::sync::Arc::new(db), share_access, 1); + let st = crate::state::State::new(std::sync::Arc::new(db), share_access); (st, TestDir(dir_path)) } fn rooted_state(volume: &str, root_link: &str) -> (crate::state::State, TestDir) { let (mut state, dir) = state_test_helper(); let root = node_helper_in_volume(volume, root_link, None, "root", true); - let root_ino = state.intern(0, root); - assert_eq!(root_ino, super::ROOT_INO); - state.entries.get_mut(&root_ino).unwrap().parent = root_ino; + // At the root inode, as `ProtonFs::new` puts it. + state.by_uid.insert(root.uid.clone(), super::ROOT_INO); + state.entries.insert( + super::ROOT_INO, + crate::state::Entry { + uid: root.uid.clone(), + parent: super::ROOT_INO, + node: root, + access: Access::Owner, + lookup_count: 1, + open_count: 0, + unlinked: false, + }, + ); (state, dir) } diff --git a/crates/pdfs-fuse/src/mount.rs b/crates/pdfs-fuse/src/mount.rs index 2dde63b..3b4cc36 100644 --- a/crates/pdfs-fuse/src/mount.rs +++ b/crates/pdfs-fuse/src/mount.rs @@ -409,7 +409,7 @@ pub(crate) fn mount_with( maintenance: Arc::new(Mutex::new(Default::default())), primary_root_uid: root.uid.clone(), primary: true, - state: Arc::new(Mutex::new(State::new(db.clone(), share_access, 2))), + state: Arc::new(Mutex::new(State::new(db.clone(), share_access))), cache: Arc::new(cache), readers: Arc::new(Mutex::new(HashMap::new())), block_ring: Arc::new(Mutex::new(BlockRing::default())), diff --git a/crates/pdfs-fuse/src/state.rs b/crates/pdfs-fuse/src/state.rs index a43336a..9cc1e16 100644 --- a/crates/pdfs-fuse/src/state.rs +++ b/crates/pdfs-fuse/src/state.rs @@ -201,6 +201,18 @@ pub(crate) struct PendingRevision { /// time so this module stays free of `fuser`. pub(crate) type DirListing = Vec<(u64, bool, String)>; +/// Where [`State::next_ino`] starts: far above any local id a database hands +/// out, so a fallback inode never names a node that has a row. +pub(crate) const FALLBACK_INO: u64 = 1 << 48; + +/// The inode of the node with local id `lid` ([`Db::lids_for`]), except where +/// it is a mount's root, which is always [`crate::ROOT_INO`]. One above the lid, +/// so no other node takes the root's number. The same in every mount and after +/// a restart. +pub(crate) fn lid_ino(lid: i64) -> u64 { + lid.unsigned_abs() + crate::ROOT_INO +} + /// Mutable inode bookkeeping, guarded by a mutex because fuser drives the /// `Filesystem` trait through `&self`. pub(crate) struct State { @@ -211,6 +223,8 @@ pub(crate) struct State { /// Cached directory listings: parent inode -> child inodes. Presence of a /// key means the directory has been enumerated. pub(crate) children: HashMap>, + /// The next inode for a node the database could not give a local id, from + /// a range lids never reach ([`FALLBACK_INO`]). pub(crate) next_ino: u64, /// Shared write state keyed by inode. Concurrent writers share the scratch file. pub(crate) active_writes: HashMap, @@ -349,9 +363,6 @@ impl Drop for StateGuard<'_> { impl State { /// A fresh, empty inode space over `db`. /// - /// `next_ino` differs by caller: a real mount hands out [`crate::ROOT_INO`] - /// to its root and starts allocating above it, while a bare state under test - /// has no root and starts at 1. /// Apply the write-throughs this state owes SQLite, here and now. /// /// Production code never calls this: [`StateGuard`] releases the inode lock @@ -378,12 +389,15 @@ impl State { self.db.clone() } - pub(crate) fn new(db: Arc, share_access: HashMap, next_ino: u64) -> Self { + /// A fresh, empty inode space over `db`. A node's inode comes from its + /// local id ([`lid_ino`]); a real mount puts its root at + /// [`crate::ROOT_INO`]. + pub(crate) fn new(db: Arc, share_access: HashMap) -> Self { Self { entries: HashMap::new(), by_uid: HashMap::new(), children: HashMap::new(), - next_ino, + next_ino: FALLBACK_INO, active_writes: HashMap::new(), handles: HashMap::new(), next_fh: 1, @@ -881,7 +895,19 @@ impl State { self.intern_mem_with_access(parent, node, access) } - fn intern_mem_with_access(&mut self, parent: u64, mut node: Node, access: Access) -> u64 { + fn intern_mem_with_access(&mut self, parent: u64, node: Node, access: Access) -> u64 { + self.intern_mem_at(parent, node, access, None) + } + + /// [`State::intern_mem_with_access`], with the inode a new entry takes when + /// the caller already has it ([`State::new_inos`]). + fn intern_mem_at( + &mut self, + parent: u64, + mut node: Node, + access: Access, + ino: Option, + ) -> u64 { if let Some(&ino) = self.by_uid.get(&node.uid) { let changed = self .entries @@ -900,8 +926,13 @@ impl State { } return ino; } - let ino = self.next_ino; - self.next_ino += 1; + let ino = match ino { + Some(ino) => ino, + None => self + .new_inos(&[&node]) + .remove(&node.uid) + .unwrap_or_default(), + }; self.by_uid.insert(node.uid.clone(), ino); self.entries.insert( ino, @@ -918,6 +949,41 @@ impl State { ino } + /// The inodes for nodes that have no entry yet, from their local ids, in + /// one transaction. Should the database fail, they come from + /// [`State::next_ino`] instead, so the mount keeps working, with inodes + /// that last only this run. So do the inodes of virtual folders, which are + /// never stored. + fn new_inos(&mut self, nodes: &[&Node]) -> HashMap { + let (virtual_nodes, stored): (Vec<&Node>, Vec<&Node>) = nodes + .iter() + .partition(|node| crate::r#virtual::is_virtual_uid(&node.uid)); + let mut inos = match self.db.lids_for(&stored) { + Ok(lids) => stored + .iter() + .zip(lids) + .map(|(node, lid)| (node.uid.clone(), lid_ino(lid))) + .collect(), + Err(error) => { + warn!(%error, "no local ids for new inodes; numbering them for this run"); + self.run_inos(&stored) + } + }; + inos.extend(self.run_inos(&virtual_nodes)); + inos + } + + fn run_inos(&mut self, nodes: &[&Node]) -> HashMap { + nodes + .iter() + .map(|node| { + let ino = self.next_ino; + self.next_ino += 1; + (node.uid.clone(), ino) + }) + .collect() + } + /// Decrement lookup count for an inode and prune if lookup_count == 0 && open_count == 0 && unlinked. pub(crate) fn forget_lookup(&mut self, ino: u64, nlookup: u64) -> Option<(u64, String)> { if let Some(entry) = self.entries.get_mut(&ino) { @@ -963,9 +1029,22 @@ impl State { self.keep_revision_mtime(node); } self.outbox.push(DbWrite::Upsert(nodes.clone())); + let fresh: Vec<&Node> = nodes + .iter() + .filter(|node| !self.by_uid.contains_key(&node.uid)) + .collect(); + let mut inos = if fresh.is_empty() { + HashMap::new() + } else { + self.new_inos(&fresh) + }; nodes .into_iter() - .map(|node| self.intern_mem(parent, node)) + .map(|node| { + let access = self.access_for_node(parent, &node); + let ino = inos.remove(&node.uid); + self.intern_mem_at(parent, node, access, ino) + }) .collect() } @@ -1283,7 +1362,7 @@ mod tests { let dir = TempDir::new(); let db = Db::open(&dir.0.join("cache.db")).unwrap(); let share_access = db.all_share_access().unwrap(); - let st = State::new(Arc::new(db), share_access, 1); + let st = State::new(Arc::new(db), share_access); (st, dir) } @@ -1301,6 +1380,36 @@ mod tests { (st, dir, src, dst) } + /// A node's inode comes from its row's local id, so it is the same in + /// every mount, after the hot cache drops it, and after a restart. + #[test] + fn a_node_keeps_its_inode_across_mounts_evictions_and_restarts() { + let (mut st, _dir) = state(); + let inos = st.intern_batch( + 0, + vec![ + node("a", "root", "a.txt", false), + node("b", "root", "b.txt", false), + ], + ); + st.flush_outbox(); + let db = st.db.clone(); + assert_ne!(inos[0], inos[1]); + assert!(!inos.contains(&crate::ROOT_INO)); + + let mut other = State::new(db.clone(), HashMap::new()); + assert_eq!(other.intern(0, node("b", "root", "b.txt", false)), inos[1]); + + st.forget_mem(&uid("a")); + assert_eq!(st.intern(0, node("a", "root", "a.txt", false)), inos[0]); + + let mut restarted = State::new(db, HashMap::new()); + assert_eq!( + restarted.intern_batch(0, vec![node("a", "root", "a.txt", false)]), + [inos[0]] + ); + } + /// Audit A5. Moving a file out of a folder leaves that folder's DB row /// claiming a complete listing, so once the hot cache drops it, remote /// changes under it stop being seen. `relocate` has to clear both sides.