From d70fe9da8d9186c08d3bbad495903eb9b126d059 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Fri, 28 Aug 2026 23:08:16 +0200 Subject: [PATCH] Decide whether to send a shard before reading it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The upload read every shard off disk and only then asked whether it needed sending. For a library with 94 MB of face shards already on the server, every idle sync read 94 MB to conclude it had nothing to do. The question does not need the bytes. A sealed shard the server already has is byte-identical by construction, and the client id is in the name, so nobody else could have written it — the name settles it. The open shard is compared on size, which `stat` answers. The progress line moves after the skip for the same reason: announced before it, an idle pass claimed to be sending five shards and sent none. Co-Authored-By: Claude Opus 5 (1M context) --- ui/dr-ui/src/derived_sync.rs | 43 +++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/ui/dr-ui/src/derived_sync.rs b/ui/dr-ui/src/derived_sync.rs index dec6393..7eeff5a 100644 --- a/ui/dr-ui/src/derived_sync.rs +++ b/ui/dr-ui/src/derived_sync.rs @@ -391,16 +391,41 @@ async fn sync_face_shards( .unwrap_or_default(); // ---- upload ---------------------------------------------------------- + // + // Every commit since the last pass is still in a write-ahead log, and a + // shard is uploaded by reading its file — so without this the upload would + // ship a database missing precisely the faces just exported. + if let Err(e) = store.checkpoint() { + log::warn!("face sync: checkpointing the shard store: {e}"); + } + let local = store.shards().map_err(|e| e.to_string())?; for (n, shard) in local.iter().enumerate() { + let name = shard_name(&client, shard.id); let path = store.shard_path(shard.id); + + // **Decided before the file is read.** A sealed shard the server + // already has is byte-identical by construction, and the client is in + // the name so nobody else could have written it — the name alone + // settles it. Reading first meant every idle sync pulled ninety-four + // megabytes off disk to conclude it had nothing to send. + let on_disk = std::fs::metadata(&path).map(|m| m.len()).unwrap_or(0); + let skip = match remote.get(&name) { + Some(_) if shard.sealed => true, + Some(size) => *size == on_disk, + None => false, + }; + if skip { + continue; + } + let Ok(bytes) = std::fs::read(&path) else { continue; }; - let name = shard_name(&client, shard.id); // Face shards carry crops and run to tens of megabytes each, so a - // single one is a visible wait on any connection. Announced before the - // put rather than after, because the wait is the upload. + // single one is a visible wait on any connection. Announced after the + // skip, or an idle pass claims to be sending five shards and sends + // none; and before the put, because the wait is the upload. let _ = tx.send(SyncMessage::Status(format!( "sending faces: shard {}/{} ({} MB)", n + 1, @@ -408,18 +433,6 @@ async fn sync_face_shards( bytes.len() / 1_048_576 ))); - // Sealed and present means byte-identical, and the client is in the - // name so nobody else could have written it. The open shard goes up - // again whenever its size differs, which is the only way it changes. - let skip = match remote.get(&name) { - Some(_) if shard.sealed => true, - Some(size) => *size == bytes.len() as u64, - None => false, - }; - if skip { - continue; - } - let target = RemotePath::new(format!("{}/{name}", face_base.as_str())); match backend.put(&target, bytes, None).await { Ok(_) => report.face_shards_uploaded += 1,