Take the server's shards and dates when the scan completes, not after the sweep
Benchmarks / CPU and I/O (per commit) (push) Successful in 8m28s
Benchmarks / Frame budget (on demand) (push) Skipped
Build and test / Desktop (Linux) (push) Failing after 36s
Build and test / Layer separation (push) Successful in 28s
Traceability / Requirement traces (push) Failing after 39s
🐳 Android image / Build and push (push) Successful in 2s
Build and test / android-image (push) Successful in 2s
🐳 Windows image / Build and push (push) Successful in 1s
Build and test / windows-image (push) Successful in 1s
Build and test / Android (aarch64) (push) Successful in 43m11s
Build and test / Windows (x86_64, cross) (push) Failing after 41m4s
Benchmarks / CPU and I/O (per commit) (push) Successful in 8m28s
Benchmarks / Frame budget (on demand) (push) Skipped
Build and test / Desktop (Linux) (push) Failing after 36s
Build and test / Layer separation (push) Successful in 28s
Traceability / Requirement traces (push) Failing after 39s
🐳 Android image / Build and push (push) Successful in 2s
Build and test / android-image (push) Successful in 2s
🐳 Windows image / Build and push (push) Successful in 1s
Build and test / windows-image (push) Successful in 1s
Build and test / Android (aarch64) (push) Successful in 43m11s
Build and test / Windows (x86_64, cross) (push) Failing after 41m4s
The derived sync fired only after the metadata sweep, so a fresh device re-derived every thumbnail it scrolled past, re-detected faces and re-read every header for hours before adopting the shards and snapshot that held all of it. It now fires as soon as the scan completes — the first moment the rows the merges key on exist — and the sweep starts behind it. In steady state that pass is one listing. The catalog merge gains a fourth half: capture metadata (captured_at, offset, camera, lens, ISO) for images still at metadata_state < 2, matched by oc:fileid from a remote row at 2. A date is a fact about the file's bytes, not local state, and the snapshot already carried it. The sweep's per-chunk query then finds nothing left, and the timeline is whole on a fresh device without a header fetch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -119,6 +119,8 @@ pub struct MergeReport {
|
|||||||
pub keywords_fused: usize,
|
pub keywords_fused: usize,
|
||||||
/// Keyword assignments taken from the remote.
|
/// Keyword assignments taken from the remote.
|
||||||
pub keywords_assigned: usize,
|
pub keywords_assigned: usize,
|
||||||
|
/// Images whose capture metadata was taken from the remote.
|
||||||
|
pub metadata_adopted: usize,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl MergeReport {
|
impl MergeReport {
|
||||||
@@ -133,6 +135,7 @@ impl MergeReport {
|
|||||||
|| self.keywords_deleted > 0
|
|| self.keywords_deleted > 0
|
||||||
|| self.keywords_fused > 0
|
|| self.keywords_fused > 0
|
||||||
|| self.keywords_assigned > 0
|
|| self.keywords_assigned > 0
|
||||||
|
|| self.metadata_adopted > 0
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Whether the local catalog holds anything the remote did not, and so
|
/// Whether the local catalog holds anything the remote did not, and so
|
||||||
@@ -193,10 +196,67 @@ pub fn merge_all(conn: &Connection) -> Result<MergeReport, CatalogError> {
|
|||||||
merge_collections_within(&tx, &mut report)?;
|
merge_collections_within(&tx, &mut report)?;
|
||||||
merge_keywords_within(&tx, &mut report)?;
|
merge_keywords_within(&tx, &mut report)?;
|
||||||
merge_people_within(&tx, &mut report)?;
|
merge_people_within(&tx, &mut report)?;
|
||||||
|
merge_metadata_within(&tx, &mut report)?;
|
||||||
tx.commit()?;
|
tx.commit()?;
|
||||||
Ok(report)
|
Ok(report)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Adopt capture metadata from an attached catalog, on its own.
|
||||||
|
pub fn merge_metadata(conn: &Connection) -> Result<MergeReport, CatalogError> {
|
||||||
|
let tx = conn.unchecked_transaction()?;
|
||||||
|
let mut report = MergeReport::default();
|
||||||
|
merge_metadata_within(&tx, &mut report)?;
|
||||||
|
tx.commit()?;
|
||||||
|
Ok(report)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Capture metadata a peer's sweep already read, for images this device has
|
||||||
|
/// not dated yet.
|
||||||
|
///
|
||||||
|
/// The `images` table is local state and the merge leaves it alone — except
|
||||||
|
/// for these columns, which are not: a capture time, an offset, a camera, a
|
||||||
|
/// lens and an ISO are facts about the file's bytes, identical on every
|
||||||
|
/// device, and read by fetching a header per image across the whole library
|
||||||
|
/// (`dr_ui::library::spawn_sweep`). A fresh device inherits its peers'
|
||||||
|
/// thumbnails and faces from the shards and then spent hours re-reading
|
||||||
|
/// every header for the timeline; the snapshot it had just merged held
|
||||||
|
/// every one of those dates.
|
||||||
|
///
|
||||||
|
/// Matched by `oc:fileid`, as collection membership is. Only rows still at
|
||||||
|
/// `metadata_state < 2` take anything, and only from a remote row at 2: a
|
||||||
|
/// date this device read for itself is never overwritten, and a peer that
|
||||||
|
/// has not read one has nothing to give. The sweep's own query
|
||||||
|
/// (`metadata_state < 2`) then finds nothing left to do for them.
|
||||||
|
const METADATA_BY_FILE_ID: &str = "
|
||||||
|
UPDATE main.images
|
||||||
|
SET captured_at = r.captured_at,
|
||||||
|
captured_offset = coalesce(main.images.captured_offset, r.captured_offset),
|
||||||
|
camera = coalesce(main.images.camera, r.camera),
|
||||||
|
lens = coalesce(main.images.lens, r.lens),
|
||||||
|
iso = coalesce(main.images.iso, r.iso),
|
||||||
|
metadata_state = 2
|
||||||
|
FROM (SELECT lr.image_id, ri.captured_at, ri.captured_offset,
|
||||||
|
ri.camera, ri.lens, ri.iso
|
||||||
|
FROM remote_cat.images ri
|
||||||
|
JOIN remote_cat.remote rr ON rr.image_id = ri.id
|
||||||
|
JOIN main.remote lr ON lr.file_id = rr.file_id
|
||||||
|
WHERE ri.metadata_state >= 2 AND ri.captured_at IS NOT NULL) AS r
|
||||||
|
WHERE main.images.id = r.image_id
|
||||||
|
AND main.images.metadata_state < 2";
|
||||||
|
|
||||||
|
fn merge_metadata_within(tx: &Connection, report: &mut MergeReport) -> Result<(), CatalogError> {
|
||||||
|
// A snapshot from before these columns, or from a library with no server
|
||||||
|
// behind it, has nothing to join on.
|
||||||
|
if !remote_has(tx, "remote")?
|
||||||
|
|| !remote_has_column(tx, "images", "metadata_state")?
|
||||||
|
|| !remote_has_column(tx, "images", "captured_offset")?
|
||||||
|
{
|
||||||
|
return Ok(());
|
||||||
|
}
|
||||||
|
report.metadata_adopted = tx.execute(METADATA_BY_FILE_ID, [])?;
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
/// Merge people and identity judgements from an attached catalog.
|
/// Merge people and identity judgements from an attached catalog.
|
||||||
///
|
///
|
||||||
/// The people half of [`merge_all`], on its own, for the same reason the other
|
/// The people half of [`merge_all`], on its own, for the same reason the other
|
||||||
@@ -1168,6 +1228,57 @@ mod tests {
|
|||||||
|
|
||||||
// ---- integration over two real catalogs ------------------------------
|
// ---- integration over two real catalogs ------------------------------
|
||||||
|
|
||||||
|
/// A fresh device takes the capture dates a peer's sweep read, matched by
|
||||||
|
/// `oc:fileid`, and never overwrites a date it read for itself.
|
||||||
|
#[test]
|
||||||
|
fn capture_metadata_arrives_for_undated_images_only() {
|
||||||
|
let c = two_catalogs();
|
||||||
|
// Three photographs on both devices: 1 undated here and dated there;
|
||||||
|
// 2 dated on both, differently; 3 undated on both.
|
||||||
|
for id in 1..=3 {
|
||||||
|
add_image_without_hash(&c, "main", id);
|
||||||
|
add_image_without_hash(&c, "remote_cat", id + 10);
|
||||||
|
add_remote_id(&c, "main", id, 100 + id);
|
||||||
|
add_remote_id(&c, "remote_cat", id + 10, 100 + id);
|
||||||
|
}
|
||||||
|
c.execute(
|
||||||
|
"UPDATE remote_cat.images
|
||||||
|
SET captured_at = 1000, captured_offset = 60, camera = 'X', metadata_state = 2
|
||||||
|
WHERE id = 11",
|
||||||
|
[],
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
c.execute(
|
||||||
|
"UPDATE remote_cat.images SET captured_at = 2000, metadata_state = 2 WHERE id = 12",
|
||||||
|
[],
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
c.execute(
|
||||||
|
"UPDATE main.images SET captured_at = 2222, metadata_state = 2 WHERE id = 2",
|
||||||
|
[],
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let report = merge_metadata(&c).unwrap();
|
||||||
|
assert_eq!(report.metadata_adopted, 1);
|
||||||
|
|
||||||
|
let row = |id: i64| -> (Option<i64>, Option<i64>, Option<String>, i64) {
|
||||||
|
c.query_row(
|
||||||
|
"SELECT captured_at, captured_offset, camera, metadata_state
|
||||||
|
FROM main.images WHERE id = ?1",
|
||||||
|
[id],
|
||||||
|
|r| Ok((r.get(0)?, r.get(1)?, r.get(2)?, r.get(3)?)),
|
||||||
|
)
|
||||||
|
.unwrap()
|
||||||
|
};
|
||||||
|
assert_eq!(row(1), (Some(1000), Some(60), Some("X".into()), 2));
|
||||||
|
assert_eq!(row(2), (Some(2222), None, None, 2));
|
||||||
|
assert_eq!(row(3), (None, None, None, 0));
|
||||||
|
|
||||||
|
// Idempotent: a second pass finds nothing left to take.
|
||||||
|
assert_eq!(merge_metadata(&c).unwrap().metadata_adopted, 0);
|
||||||
|
}
|
||||||
|
|
||||||
fn two_catalogs() -> Connection {
|
fn two_catalogs() -> Connection {
|
||||||
attached_remote(schema::for_attached("remote_cat"))
|
attached_remote(schema::for_attached("remote_cat"))
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -530,6 +530,15 @@ Three invariants, each tested:
|
|||||||
| Re-storing an existing id updates in place, never migrates | Migrating would rewrite a sealed shard |
|
| 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 |
|
| 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 |
|
||||||
|
|
||||||
|
**When the exchange runs.** Corrected 2026-09-20. It fired only after the metadata sweep — hours
|
||||||
|
on a large library — so a fresh device re-derived every thumbnail it looked at, re-detected faces
|
||||||
|
and re-read every header before adopting the shards and snapshot that held all of it. It now also
|
||||||
|
fires the moment the scan completes, which is the first moment the rows the merges key on exist,
|
||||||
|
and the sweep starts behind it. In steady state that pass is one listing. The catalog merge also
|
||||||
|
takes **capture metadata** (`captured_at`, offset, camera, lens, ISO) for images still at
|
||||||
|
`metadata_state < 2`, matched by `oc:fileid` — a date is a fact about the file's bytes, not local
|
||||||
|
state, and the snapshot already carried it; the sweep's per-chunk query then finds nothing left.
|
||||||
|
|
||||||
**The transfer**, in `dr-ui`'s `derived_sync`, exchanges shards with `.darkroom-derived/` under the
|
**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
|
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.
|
is one listing plus whichever shard is still open.
|
||||||
|
|||||||
@@ -67,6 +67,9 @@ pub struct SyncReport {
|
|||||||
/// devices already had gains *no* collection, and reporting only the
|
/// devices already had gains *no* collection, and reporting only the
|
||||||
/// former left the sidebar showing no count beside a full collection.
|
/// former left the sidebar showing no count beside a full collection.
|
||||||
pub members_gained: usize,
|
pub members_gained: usize,
|
||||||
|
/// Images dated from the remote's snapshot rather than by this device's
|
||||||
|
/// own sweep — what makes the timeline whole on a fresh device.
|
||||||
|
pub dates_gained: usize,
|
||||||
|
|
||||||
// Face data is counted apart from thumbnails for the same reason keywords
|
// Face data is counted apart from thumbnails for the same reason keywords
|
||||||
// are counted apart from collections: "adopted 4,812 faces" is a sentence
|
// are counted apart from collections: "adopted 4,812 faces" is a sentence
|
||||||
@@ -815,6 +818,7 @@ fn merge_downloaded(
|
|||||||
report.catalog_merged = true;
|
report.catalog_merged = true;
|
||||||
report.collections_gained += merge.inserted + merge.updated;
|
report.collections_gained += merge.inserted + merge.updated;
|
||||||
report.members_gained += merge.members_added;
|
report.members_gained += merge.members_added;
|
||||||
|
report.dates_gained += merge.metadata_adopted;
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1378,6 +1378,15 @@ fn drain_scan(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
load_window(&w, ctl);
|
load_window(&w, ctl);
|
||||||
|
// Take the server's shards and catalog *now*,
|
||||||
|
// before the sweep: the rows they key on exist
|
||||||
|
// from this moment, and on a fresh device
|
||||||
|
// every thumbnail, face and collection a peer
|
||||||
|
// has already made is on the server. Waiting
|
||||||
|
// for the sweep — hours on a large library —
|
||||||
|
// meant re-deriving all of it here first. In
|
||||||
|
// steady state this is one listing.
|
||||||
|
start_derived_sync(&w, ctl);
|
||||||
// Everything the grid did not touch: the rest
|
// Everything the grid did not touch: the rest
|
||||||
// of the library gets a thumbnail and a date,
|
// of the library gets a thumbnail and a date,
|
||||||
// so the timeline describes all of it rather
|
// so the timeline describes all of it rather
|
||||||
@@ -4201,8 +4210,11 @@ fn spawn_scheduled_backup(ctl: &Rc<LibraryController>) {
|
|||||||
|
|
||||||
/// Push shards and the catalog to the server, and take what it has.
|
/// Push shards and the catalog to the server, and take what it has.
|
||||||
///
|
///
|
||||||
/// Fired after the sweep completes, when there is a finished index worth
|
/// Fired when the scan completes, so a fresh device inherits its peers'
|
||||||
/// sharing, and from the Sync button for an explicit exchange.
|
/// work before deriving any of its own; after the sweep completes, when
|
||||||
|
/// there is a finished index worth sharing; and from the Sync button for an
|
||||||
|
/// explicit exchange. A pass still running when the next trigger fires is
|
||||||
|
/// left to finish — the guard below.
|
||||||
fn start_derived_sync(window: &AppWindow, ctl: &Rc<LibraryController>) {
|
fn start_derived_sync(window: &AppWindow, ctl: &Rc<LibraryController>) {
|
||||||
// An escape hatch for running the app against a *copied* library without
|
// An escape hatch for running the app against a *copied* library without
|
||||||
// touching the account's real server.
|
// touching the account's real server.
|
||||||
@@ -4326,7 +4338,7 @@ fn start_derived_sync(window: &AppWindow, ctl: &Rc<LibraryController>) {
|
|||||||
crate::derived_sync::SyncMessage::Finished(report) => {
|
crate::derived_sync::SyncMessage::Finished(report) => {
|
||||||
log::info!(
|
log::info!(
|
||||||
"sync: {} shard(s) up, {} down ({} thumbnails), \
|
"sync: {} shard(s) up, {} down ({} thumbnails), \
|
||||||
catalog {}{}",
|
catalog {}{}{}",
|
||||||
report.shards_uploaded,
|
report.shards_uploaded,
|
||||||
report.shards_downloaded,
|
report.shards_downloaded,
|
||||||
report.thumbnails_adopted,
|
report.thumbnails_adopted,
|
||||||
@@ -4343,6 +4355,11 @@ fn start_derived_sync(window: &AppWindow, ctl: &Rc<LibraryController>) {
|
|||||||
format!(", {} collection(s) gained", report.collections_gained)
|
format!(", {} collection(s) gained", report.collections_gained)
|
||||||
} else {
|
} else {
|
||||||
String::new()
|
String::new()
|
||||||
|
},
|
||||||
|
if report.dates_gained > 0 {
|
||||||
|
format!(", {} date(s) gained", report.dates_gained)
|
||||||
|
} else {
|
||||||
|
String::new()
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
w.set_library_syncing(false);
|
w.set_library_syncing(false);
|
||||||
@@ -4359,13 +4376,22 @@ fn start_derived_sync(window: &AppWindow, ctl: &Rc<LibraryController>) {
|
|||||||
w.set_library_status(format!("synced · {summary}").into());
|
w.set_library_status(format!("synced · {summary}").into());
|
||||||
}
|
}
|
||||||
// Adopted thumbnails and merged collections both change
|
// Adopted thumbnails and merged collections both change
|
||||||
// what the grid should show.
|
// what the grid should show; adopted dates change its
|
||||||
|
// order, and the timeline beside it.
|
||||||
if report.thumbnails_adopted > 0
|
if report.thumbnails_adopted > 0
|
||||||
|| report.collections_gained > 0
|
|| report.collections_gained > 0
|
||||||
|| report.members_gained > 0
|
|| report.members_gained > 0
|
||||||
|
|| report.dates_gained > 0
|
||||||
{
|
{
|
||||||
load_window(&w, &ctl_cb);
|
load_window(&w, &ctl_cb);
|
||||||
}
|
}
|
||||||
|
if report.dates_gained > 0 {
|
||||||
|
let borrow = ctl_cb.catalog();
|
||||||
|
let borrow = borrow.borrow();
|
||||||
|
if let Some(cat) = borrow.as_ref() {
|
||||||
|
refresh_timeline(&w, cat, &ctl_cb);
|
||||||
|
}
|
||||||
|
}
|
||||||
// TRACES: FR-CAT-7
|
// TRACES: FR-CAT-7
|
||||||
// And the sidebar, which the grid reload does not
|
// And the sidebar, which the grid reload does not
|
||||||
// touch. Membership counts as a change: a sync that
|
// touch. Membership counts as a change: a sync that
|
||||||
|
|||||||
Reference in New Issue
Block a user