Give a shard's remote name the client that wrote it
Build and test / Desktop (Linux) (push) Failing after 50s
Build and test / Layer separation (push) Successful in 24s
🐳 Android image / Build and push (push) Successful in 3s
Build and test / android-image (push) Successful in 3s
Traceability / Requirement traces (push) Failing after 59s
Build and test / Android (aarch64) (push) Failing after 9m39s

Shard ids are per store: every client fills its own numbering from 0, so
"shard 3" names different thumbnails on every device. The derived sync
published them into a flat shard-NNNN.sqlite namespace anyway, which left
two clients writing one name.

Both failures that follow were live. On upload, a client's open shard
overwrote a peer's file of the same id — content the peer still believed
was published and would never restore, because its own copy was sealed and
the name existed. On download, the loop skipped any remote id it already
held locally, which is the only safe reading of a name that says nothing
about who wrote it, so a client holding shards 0..5 never fetched the
peer's 0..5 at all. Between them, two populated clients exchanged almost
nothing: only shards numbered above the other's highest. A fresh device
worked, having no local shards to collide with, which is why this went
unnoticed — it is exactly the case the feature was written for.

The name is now shard-<client>-NNNN.sqlite. The client id is minted per
store in index.sqlite, beside the numbering it qualifies rather than in
settings: a store deleted and rebuilt restarts at shard 0 and must not
claim the remote names its predecessor wrote. Since our own ids now say
nothing about what we have taken from others, index.sqlite also keeps a
ledger of adopted remote names and the size each had when merged. A size
rather than a flag, because a peer's sealed shard never returns but its
open one grows, and re-merging the grown copy is how the thumbnails it
gained since arrive.

Flat names already on servers still parse, reporting no owner, so each
client adopts them once, and nothing is written under that form again. One
whose id and byte size match a local shard is that client's own earlier
upload by the same identity argument the upload path already makes for
sealed shards, so the rename does not cost every client a re-download of
its whole store. Older builds ignore the new names and stop receiving
shards until updated; their own uploads are still adopted, so nothing is
lost.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-08-16 21:15:12 +02:00
co-authored by Claude Opus 5
parent f7e8cc99b1
commit 9a51cc88d6
3 changed files with 233 additions and 28 deletions
+107
View File
@@ -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<u64> {
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<Option<Thumbnail>, ThumbError> {
let shard: Option<i64> = 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<String, ThumbError> {
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();
+27 -4
View File
@@ -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-<client>-NNNN.sqlite`, where `<client>` 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
+99 -24
View File
@@ -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<String, u64> = 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<u32> = 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<u32> {
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]