diff --git a/core/dr-thumbs/src/lib.rs b/core/dr-thumbs/src/lib.rs index 9f099e9..dd5af21 100644 --- a/core/dr-thumbs/src/lib.rs +++ b/core/dr-thumbs/src/lib.rs @@ -124,6 +124,7 @@ pub struct Thumbnail { pub struct ThumbStore { dir: PathBuf, index: Connection, + client: String, } impl ThumbStore { @@ -139,13 +140,47 @@ impl ThumbStore { // entries under a bare `file_id` key; bring it forward rather than // discarding every thumbnail already fetched. migrate_size_column(&index, "entries")?; + let client = mint_client_id(&index)?; Ok(Self { dir: dir.to_path_buf(), index, + client, }) } + /// This store's identity among the clients sharing a library. + /// + /// Shard ids are per-store — every client fills its own numbering from 0 — + /// so a name that travels needs this to say *whose* shard 3 it is. + pub fn client_id(&self) -> &str { + &self.client + } + + /// The size a remote shard had when it was last merged, if it ever was. + /// + /// A size rather than a bare flag: a peer's sealed shard never changes + /// again, but its open one grows, and re-merging the grown copy is how the + /// thumbnails it gained since arrive. + pub fn adopted(&self, name: &str) -> Option { + self.index + .query_row("SELECT size FROM adopted WHERE name = ?1", [name], |r| { + r.get::<_, i64>(0) + }) + .ok() + .map(|bytes| bytes as u64) + } + + /// Record that a remote shard of this name and size has been merged. + pub fn record_adopted(&self, name: &str, size: u64) -> Result<(), ThumbError> { + self.index.execute( + "INSERT INTO adopted(name, size) VALUES (?1, ?2) + ON CONFLICT(name) DO UPDATE SET size = excluded.size", + rusqlite::params![name, size as i64], + )?; + Ok(()) + } + /// Fetch a thumbnail by file id. pub fn get(&self, file_id: u64, size: ThumbSize) -> Result, ThumbError> { let shard: Option = self @@ -566,8 +601,43 @@ CREATE TABLE IF NOT EXISTS shards ( -- already hold it would see it change. sealed INTEGER NOT NULL DEFAULT 0 ); + +-- Facts about this store rather than about any thumbnail. The index never +-- leaves the device, so what lives here is safe to be device-specific. +CREATE TABLE IF NOT EXISTS meta ( + key TEXT PRIMARY KEY, + value TEXT NOT NULL +); + +-- Remote shards already merged, under the name and size they had on the +-- server. Nothing in a merged thumbnail records where it came from, so without +-- this ledger a client either re-downloads every peer's shard on every sync or +-- guesses from its own numbering — and its own numbering says nothing about +-- anyone else's. +CREATE TABLE IF NOT EXISTS adopted ( + name TEXT PRIMARY KEY, + size INTEGER NOT NULL +); "#; +/// Read this store's client id, minting one on first open. +/// +/// It lives in the index because that is where the shard numbering it +/// qualifies lives: a store deleted and rebuilt starts again at shard 0, and +/// must not claim the remote names its predecessor wrote. SQLite's own +/// randomness keeps it dependency-free, and six bytes separate far more +/// devices than one account ever has. +fn mint_client_id(conn: &Connection) -> Result { + conn.execute( + "INSERT INTO meta(key, value) VALUES('client_id', lower(hex(randomblob(6)))) + ON CONFLICT(key) DO NOTHING", + [], + )?; + Ok(conn.query_row("SELECT value FROM meta WHERE key = 'client_id'", [], |r| { + r.get(0) + })?) +} + const SHARD_SCHEMA: &str = r#" CREATE TABLE IF NOT EXISTS thumbs ( file_id INTEGER NOT NULL, @@ -985,6 +1055,43 @@ mod tests { assert!(p.to_string_lossy().ends_with("shard-0007.sqlite")); } + #[test] + fn every_store_gets_its_own_stable_client_id() { + let (mine, dir) = store(); + let id = mine.client_id().to_string(); + assert!(!id.is_empty()); + drop(mine); + + // Stable across reopen, or a client would orphan its own uploads and + // re-download them as if they were a peer's. + assert_eq!(ThumbStore::open(&dir).unwrap().client_id(), id); + + // Distinct per store, which is the property the remote naming rests + // on: two devices both filling shard 0 must not name one file. + let (theirs, _d) = store(); + assert_ne!(theirs.client_id(), id); + } + + #[test] + fn the_adoption_ledger_remembers_a_merged_shard_by_size() { + let (mine, dir) = store(); + assert_eq!(mine.adopted("shard-abc-0000.sqlite"), None); + + mine.record_adopted("shard-abc-0000.sqlite", 1234).unwrap(); + assert_eq!(mine.adopted("shard-abc-0000.sqlite"), Some(1234)); + drop(mine); + + // Survives reopen: the point is to not re-download across sessions. + let reopened = ThumbStore::open(&dir).unwrap(); + assert_eq!(reopened.adopted("shard-abc-0000.sqlite"), Some(1234)); + + // A peer's open shard grows, and the new size is what says so. + reopened + .record_adopted("shard-abc-0000.sqlite", 5678) + .unwrap(); + assert_eq!(reopened.adopted("shard-abc-0000.sqlite"), Some(5678)); + } + #[test] fn reopening_finds_what_was_stored() { let (mut s, dir) = store(); diff --git a/docs/catalog.md b/docs/catalog.md index 5c5deb5..08793fc 100644 --- a/docs/catalog.md +++ b/docs/catalog.md @@ -497,7 +497,7 @@ sync to Nextcloud**, so a second device gets a full grid without re-fetching a b ```text thumbs/ - index.sqlite fileid → shard, plus size accounting + index.sqlite fileid → shard, size accounting, client id, adoption ledger shard-0000.sqlite ≤ 25 MB, sealed shard-0001.sqlite ≤ 25 MB, active ``` @@ -530,9 +530,32 @@ Three invariants, each tested: | Re-storing an existing id updates in place, never migrates | Migrating would rewrite a sealed shard | | Merging another client's shard is insert-only and idempotent | Both copies derive from the same bytes by the same code, so neither is better; preferring ours avoids dirtying a shard others have synced | -**Not yet built:** the transfer itself. `ThumbStore::shards()` reports which shards are sealed — -the input a sync pass needs — and `merge_shard` adopts a downloaded one, but nothing uploads or -downloads them yet. Until that lands the store is a local cache that happens to have the right shape. +**The transfer**, in `dr-ui`'s `derived_sync`, exchanges shards with `.darkroom-derived/` under the +library root. `ThumbStore::shards()` reports which are sealed, so an up-to-date client's whole pass +is one listing plus whichever shard is still open. + +**Why a remote name carries a client id.** Corrected 2026-08-16. Shard ids are *per store* — every +client fills its own numbering from 0 — so the flat `shard-NNNN.sqlite` namespace the transfer first +used had two clients writing one name. Two failures followed from it, and both were live: the second +client's upload **overwrote** content the first still believed was published, and no client could +distinguish a peer's shard 3 from its own, so the only safe reading of "I already hold 3" was to skip +it. Between them, two populated clients exchanged almost nothing — only shards numbered above the +other's highest. A fresh device worked, which is why it went unnoticed: with no local shards there is +nothing to collide with. + +The name is now `shard--NNNN.sqlite`, where `` is minted per store in `index.sqlite` +beside the numbering it qualifies — a store deleted and rebuilt restarts at shard 0 and must not +claim its predecessor's names. Since a client's own ids no longer say anything about what it has +taken from others, `index.sqlite` also keeps an **adoption ledger** of merged remote names and the +size each had. Size, not a flag: a peer's sealed shard never returns, but its open one grows, and +re-merging the grown copy is how the thumbnails it gained arrive. + +Flat names left on servers by earlier builds are still read — they report no owner, so each client +adopts them once — and nothing is written under that form again. A flat name whose id and byte size +match a local shard is that client's own earlier upload by the same identity argument used for +sealed shards, so the rename does not cost every client a re-download of its whole store. Older +builds ignore the new names, so they stop receiving shards until updated; nothing is lost, since +their own uploads are still adopted. Two size classes remain planned — grid (256px) and filmstrip/loupe (1024px). Only the grid class is implemented. The cache is LRU-capped per NFR-RES-4, and thumbnails evict before proxies and long diff --git a/ui/dr-ui/src/derived_sync.rs b/ui/dr-ui/src/derived_sync.rs index fdd14db..a13586d 100644 --- a/ui/dr-ui/src/derived_sync.rs +++ b/ui/dr-ui/src/derived_sync.rs @@ -158,6 +158,18 @@ fn derived_path(root: &str) -> RemotePath { /// Upload what the server lacks, download what we lack. Sealed shards are /// immutable, so a name match is a content match and nothing needs comparing /// beyond existence — which is what keeps a steady-state sync to one listing. +/// +/// # Why the name carries a client +/// +/// Shard ids are per-store: every client fills its own numbering from 0, so +/// "shard 3" names different thumbnails on each device. A flat `shard-0003` +/// remote namespace therefore has two clients writing one name — the second +/// upload overwrites content the first still believes is published — and +/// leaves a client no way to tell a peer's shard 3 from its own, so the only +/// safe reading of "I already have 3" is to skip it and never adopt anything. +/// Qualifying the name with [`ThumbStore::client_id`] gives each store its own +/// namespace, and [`ThumbStore::adopted`] then tracks what has been merged by +/// remote name instead of by our own ids. async fn sync_shards( backend: &NextcloudBackend, base: &RemotePath, @@ -168,12 +180,14 @@ async fn sync_shards( let store = match ThumbStore::open(thumbs_dir) { Ok(s) => s, Err(e) => { - // No local store is not a failure: a fresh device has nothing to - // upload and everything to gain from downloading. + // A store that will not open can be neither read nor merged into, + // so there is no half of this worth attempting. It is not a sync + // failure: the next pass retries once the store is openable. log::debug!("thumbnail store unavailable: {e}"); return Ok(()); } }; + let client = store.client_id().to_string(); let remote: std::collections::HashMap = backend .list(base, None) @@ -197,10 +211,11 @@ async fn sync_shards( let Ok(bytes) = std::fs::read(&path) else { continue; }; - let name = shard_name(shard.id); + let name = shard_name(&client, shard.id); // A sealed shard the server already has is byte-identical by - // construction, so its presence is proof enough. The open shard is + // construction, so its presence is proof enough — and with the client + // in the name, no one else can have written it. The open shard is // re-uploaded whenever its size differs, which is the only way it // changes. let skip = match remote.get(&name) { @@ -222,12 +237,23 @@ async fn sync_shards( } // ---- download -------------------------------------------------------- - let have: std::collections::HashSet = local.iter().map(|s| s.id).collect(); let mut store = store; - for name in remote.keys() { - let Some(id) = shard_id(name) else { continue }; - if have.contains(&id) { + for (name, size) in &remote { + let Some((owner, id)) = parse_shard(name) else { + continue; + }; + if owner == client { + continue; + } + // Merged already, at the size it still has. A sealed shard never + // reaches here twice; a peer's open one does each time it grows, which + // is what carries its later thumbnails across. + if store.adopted(name) == Some(*size) { + continue; + } + if owner.is_empty() && legacy_upload_of_ours(&store, id, *size) { + let _ = store.record_adopted(name, *size); continue; } @@ -251,6 +277,9 @@ async fn sync_shards( Ok(n) => { report.shards_downloaded += 1; report.thumbnails_adopted += n; + // Recorded only on success, so a failed merge is retried next + // pass rather than written off. + let _ = store.record_adopted(name, *size); } Err(e) => log::warn!("merging {name}: {e}"), } @@ -260,6 +289,23 @@ async fn sync_shards( Ok(()) } +/// Whether a flat-named remote shard is this client's own earlier upload. +/// +/// Before the name carried a client every client wrote `shard-NNNN.sqlite`, so +/// the folder still holds files with nothing in the name to say whose they +/// are. A local shard of the same id and the same size is ours by +/// construction — the same identity argument the upload path makes for +/// skipping a sealed shard the server already has — and skipping those is what +/// keeps the rename from costing every client a re-download of its whole +/// store. Being wrong costs a peer's shard going unmerged and its thumbnails +/// being derived locally instead; it loses nothing, and two independently +/// filled 25 MB databases landing on the same byte count is not a real case. +fn legacy_upload_of_ours(store: &ThumbStore, id: u32, remote_size: u64) -> bool { + std::fs::metadata(store.shard_path(id)) + .map(|m| m.len() == remote_size) + .unwrap_or(false) +} + /// Exchange the catalog, for its collections. /// /// Only collections merge — see [`dr_catalog::sync`]. The rest of a catalog @@ -318,19 +364,26 @@ async fn sync_catalog( Ok(()) } -fn shard_name(id: u32) -> String { - format!("shard-{id:04}.sqlite") +fn shard_name(client: &str, id: u32) -> String { + format!("shard-{client}-{id:04}.sqlite") } -/// The shard id in a filename, or `None` if it is not a shard. +/// The client that wrote a remote shard and its id in that client's numbering, +/// or `None` if the name is not a shard. +/// +/// Also accepts the flat `shard-NNNN.sqlite` written before names carried a +/// client, reporting an empty owner: those belong to nobody identifiable, so +/// they read as foreign and are adopted once like any peer's. Nothing is ever +/// uploaded under that form again. /// /// Guards the download loop against adopting the catalog, a stray file, or /// anything else the folder happens to contain. -fn shard_id(name: &str) -> Option { - name.strip_prefix("shard-")? - .strip_suffix(".sqlite")? - .parse() - .ok() +fn parse_shard(name: &str) -> Option<(&str, u32)> { + let stem = name.strip_prefix("shard-")?.strip_suffix(".sqlite")?; + match stem.rsplit_once('-') { + Some((client, id)) => Some((client, id.parse().ok()?)), + None => Some(("", stem.parse().ok()?)), + } } #[cfg(test)] @@ -351,20 +404,42 @@ mod tests { #[test] fn shard_names_round_trip() { - assert_eq!(shard_name(0), "shard-0000.sqlite"); - assert_eq!(shard_name(42), "shard-0042.sqlite"); - assert_eq!(shard_id("shard-0042.sqlite"), Some(42)); - assert_eq!(shard_id(&shard_name(7)), Some(7)); + assert_eq!(shard_name("a1b2c3d4e5f6", 0), "shard-a1b2c3d4e5f6-0000.sqlite"); + assert_eq!(shard_name("a1b2c3d4e5f6", 42), "shard-a1b2c3d4e5f6-0042.sqlite"); + assert_eq!( + parse_shard(&shard_name("a1b2c3d4e5f6", 7)), + Some(("a1b2c3d4e5f6", 7)) + ); + } + + #[test] + fn two_clients_shard_three_are_different_files() { + // The whole point: one client's numbering must not name another's + // shard, or the second upload overwrites the first's content and + // neither can tell the other's shards from its own. + assert_ne!(shard_name("aaaa", 3), shard_name("bbbb", 3)); + assert_eq!(parse_shard(&shard_name("aaaa", 3)).unwrap().0, "aaaa"); + assert_eq!(parse_shard(&shard_name("bbbb", 3)).unwrap().0, "bbbb"); + } + + #[test] + fn flat_names_read_as_belonging_to_nobody() { + // Written before the name carried a client. They must still parse, so + // a library synced by an older build is not stranded, and they must + // not match any live client id, so they are never mistaken for ours. + assert_eq!(parse_shard("shard-0042.sqlite"), Some(("", 42))); + assert_ne!(parse_shard("shard-0042.sqlite").unwrap().0, "a1b2c3d4e5f6"); } #[test] fn non_shard_files_are_not_adopted() { // The folder also holds the catalog; downloading it as a shard would // hand a catalog to the thumbnail merger. - assert_eq!(shard_id("catalog.sqlite"), None); - assert_eq!(shard_id("shard-0000.sqlite-wal"), None); - assert_eq!(shard_id("notes.txt"), None); - assert_eq!(shard_id("shard-abc.sqlite"), None); + assert_eq!(parse_shard("catalog.sqlite"), None); + assert_eq!(parse_shard("shard-0000.sqlite-wal"), None); + assert_eq!(parse_shard("notes.txt"), None); + assert_eq!(parse_shard("shard-abc.sqlite"), None); + assert_eq!(parse_shard("shard-a1b2c3-notanid.sqlite"), None); } #[test]