diff --git a/core/dr-catalog/src/jobs.rs b/core/dr-catalog/src/jobs.rs index 5b7f3cb..db2a613 100644 --- a/core/dr-catalog/src/jobs.rs +++ b/core/dr-catalog/src/jobs.rs @@ -29,7 +29,11 @@ pub enum JobKind { ScanFolder = 0, /// Promote an image from stat-only to full EXIF. ExtractMetadata = 1, - /// Build or rebuild a thumbnail. + /// Build or rebuild a thumbnail. **Retired** — see [`JobKind::RETIRED`]. + /// + /// Kept so the number stays taken: a catalog written by 0.16.0 or earlier + /// holds rows of kind 2, and reusing it would hand them to whatever took + /// its place. Thumbnail = 2, /// A sidecar on disk is newer than what the catalog read. ReadSidecar = 3, @@ -69,6 +73,18 @@ impl JobKind { JobKind::DetectFaces, ]; + /// Kinds that are no longer queued by anything, whose rows are deleted on + /// sight by [`drop_retired`]. + /// + /// `Thumbnail` is here because thumbnails are owed by the store, not by + /// the queue. The grid's worker and the thumbnail sweep both find their + /// work by asking `ThumbStore` what it lacks, and the store is shared + /// between devices, so it is the only thing that can say another device + /// already made one. Up to 0.16.0 every scan enqueued a job per + /// photograph anyway and no handler ever claimed one: the reference + /// catalog held 23,582 of them (#73; catalog.md §6.1). + pub const RETIRED: [JobKind; 1] = [JobKind::Thumbnail]; + fn from_i64(v: i64) -> Option { Some(match v { 0 => JobKind::ScanFolder, @@ -399,7 +415,7 @@ pub fn recover_orphaned(conn: &Connection) -> Result { /// /// Coalescing keeps the table one row per unit of work, but nothing shrinks it /// when the work stops existing: a library that has been culled carries a -/// thumbnail job for every photograph deleted since the last time anything +/// job for every photograph deleted since the last time anything /// looked. Each one would be claimed, run, and failed five times. /// /// Only kinds whose subject really is an image ([`JobKind::subject_is_image`]) @@ -431,6 +447,32 @@ pub fn reap_orphan_subjects(conn: &Connection) -> Result { Ok(n) } +/// Delete every row of a [`JobKind::RETIRED`] kind. +/// +/// Not a migration, deliberately. A schema bump makes an older build refuse +/// the synced catalog snapshot, and a device still on 0.16.0 would lose the +/// catalog to save a megabyte. So this runs where the queue is readied — +/// [`crate::runner::recover`], at every open — and has to be cheap when there +/// is nothing to do: `kind` leads the `UNIQUE(kind, subject_id)` index, so an +/// empty answer is one index probe, not a table scan. +/// +/// Every open rather than once, because once is not enough: an older build +/// opening the same catalog enqueues them again on its next scan. +/// +/// Rows in any state go. Nothing claims these kinds, so none can be running, +/// and a failed one would be a report about work nobody was going to do. +pub fn drop_retired(conn: &Connection) -> Result { + let kinds: Vec = JobKind::RETIRED.iter().map(|k| *k as i64).collect(); + let placeholders = std::iter::repeat_n("?", kinds.len()) + .collect::>() + .join(","); + let n = conn.execute( + &format!("DELETE FROM jobs WHERE kind IN ({placeholders})"), + rusqlite::params_from_iter(kinds.iter()), + )?; + Ok(n) +} + /// How much is left, by state. /// /// One query rather than a listing, because the caller is a progress line: a diff --git a/core/dr-catalog/src/runner.rs b/core/dr-catalog/src/runner.rs index d0d171f..6738b9b 100644 --- a/core/dr-catalog/src/runner.rs +++ b/core/dr-catalog/src/runner.rs @@ -77,7 +77,7 @@ pub enum Outcome { /// Something that can actually do the work a job describes. /// -/// The catalog knows what needs doing and nothing about how — a thumbnail +/// The catalog knows what needs doing and nothing about how — face detection /// needs a decoder, a fetch needs a network stack, and neither belongs under /// `core/dr-catalog` (ARCH §4.1: calls go downward). So the queue lives here /// and the handlers are supplied from above. @@ -187,17 +187,19 @@ pub struct Recovered { pub reclaimed: usize, /// Jobs deleted because the photograph they name no longer exists. pub reaped: usize, + /// Jobs deleted because their kind is retired ([`JobKind::RETIRED`]). + pub retired: usize, } impl Recovered { pub fn did_anything(&self) -> bool { - self.reclaimed > 0 || self.reaped > 0 + self.reclaimed > 0 || self.reaped > 0 || self.retired > 0 } } /// Ready the queue for a fresh run, before any worker touches it. /// -/// Two distinct cleanups, and both are startup-only: +/// Three distinct cleanups, and all are startup-only: /// /// - **Reclaim.** A `Running` row has no owner; the process that claimed it is /// gone. On Android that is a routine morning, not a crash (FR-PLAT-AND-3). @@ -205,19 +207,27 @@ impl Recovered { /// process down with it three times running should not be retried forever, /// and the attempt counter is the only evidence of that we have. /// - **Reap.** Jobs naming an image the catalog no longer has. A library that -/// has been culled leaves thumbnail jobs for photographs that were deleted +/// has been culled leaves jobs for photographs that were deleted /// months ago, and every one of them would be claimed, run and failed. +/// - **Retire.** Rows of a kind nothing enqueues or claims any more +/// ([`jobs::drop_retired`]). Here rather than in a migration so that no +/// schema bump locks an older device out of the synced catalog, and every +/// time rather than once because an older build sharing the catalog will +/// queue them again. /// -/// Reclaim runs first so its count is the honest number of interrupted jobs, +/// Retiring runs first, so the other two never touch rows about to go. +/// Reclaim runs next so its count is the honest number of interrupted jobs, /// before reaping removes whichever of them pointed at nothing. /// /// **Call this exactly once per catalog, at startup.** It cannot distinguish a /// job a dead process was holding from one a live runner is holding right now, /// because there is no owner column — the queue is durable, not distributed. pub fn recover(conn: &Connection) -> Result { + let retired = jobs::drop_retired(conn)?; Ok(Recovered { reclaimed: jobs::recover_orphaned(conn)?, reaped: jobs::reap_orphan_subjects(conn)?, + retired, }) } @@ -729,7 +739,7 @@ mod tests { // which is the window a durable queue exists to survive: no `complete`, // no `fail`, just a row marked `Running` with nobody holding it. let c = db(); - queued(&c, JobKind::Thumbnail, 1); + queued(&c, JobKind::ContentHash, 1); // The dead process. It claimed the job and never came back. let claimed = jobs::claim_next(&c, 0).unwrap().expect("claimable"); @@ -738,7 +748,7 @@ mod tests { // A fresh runner, before it starts, finds the queue empty — the row is // `Running` and no claim will touch it. let seen = Arc::new(Mutex::new(Vec::new())); - let mut runner = Runner::new(&c).with(recording(vec![JobKind::Thumbnail], seen.clone())); + let mut runner = Runner::new(&c).with(recording(vec![JobKind::ContentHash], seen.clone())); assert_eq!( runner.drain_all(0).unwrap().ran(), 0, @@ -766,7 +776,7 @@ mod tests { // the only evidence we keep across a death. Without this a poison-pill // job would be reclaimed and re-run forever. let c = db(); - queued(&c, JobKind::Thumbnail, 1); + queued(&c, JobKind::ContentHash, 1); for _ in 0..MAX_ATTEMPTS { jobs::claim_next(&c, 0).unwrap().expect("claimable"); @@ -782,13 +792,49 @@ mod tests { assert_eq!(state, JobState::Failed as i64); } + #[test] + fn recovery_drops_retired_kinds_every_time_and_nothing_else() { + // What 0.16.0 left behind: a thumbnail job per photograph that nothing + // would ever claim, beside live work that must survive. + let c = db(); + queued(&c, JobKind::Thumbnail, 1); + queued(&c, JobKind::Thumbnail, 2); + enqueue( + &c, + JobKind::DetectFaces, + Some(1), + Priority::Background, + None, + ) + .unwrap(); + + let first = recover(&c).unwrap(); + assert_eq!(first.retired, 2); + assert!(first.did_anything()); + + let kinds: Vec = c + .prepare("SELECT kind FROM jobs") + .unwrap() + .query_map([], |r| r.get(0)) + .unwrap() + .map(Result::unwrap) + .collect(); + assert_eq!(kinds, vec![JobKind::DetectFaces as i64]); + + // An older build opening the same catalog queues them again on its + // next scan. The next open by this one clears them again. + enqueue(&c, JobKind::Thumbnail, Some(1), Priority::Background, None).unwrap(); + assert_eq!(recover(&c).unwrap().retired, 1); + assert_eq!(recover(&c).unwrap(), Recovered::default()); + } + #[test] fn recovery_drops_jobs_whose_photograph_is_gone() { // A culled library leaves thumbnail jobs for images deleted months // ago. Every one would be claimed, run and failed. let c = db(); - queued(&c, JobKind::Thumbnail, 1); - queued(&c, JobKind::Thumbnail, 2); + queued(&c, JobKind::ContentHash, 1); + queued(&c, JobKind::ContentHash, 2); c.execute("DELETE FROM images WHERE id = 2", []).unwrap(); let recovered = recover(&c).unwrap(); @@ -804,7 +850,7 @@ mod tests { #[test] fn a_quiet_startup_recovers_nothing() { let c = db(); - queued(&c, JobKind::Thumbnail, 1); + queued(&c, JobKind::ContentHash, 1); assert_eq!(recover(&c).unwrap(), Recovered::default()); assert!(!recover(&c).unwrap().did_anything()); } diff --git a/ui/dr-ui/src/library_ui/open.rs b/ui/dr-ui/src/library_ui/open.rs index fdfdde8..816f0c0 100644 --- a/ui/dr-ui/src/library_ui/open.rs +++ b/ui/dr-ui/src/library_ui/open.rs @@ -810,8 +810,9 @@ fn adopt_catalog( // the callers' early return on an already-open catalog is what makes it // once. The job queue is durable, so a run that was killed mid-job left its // row marked `Running` with nobody holding it; recovery hands those back to - // be resumed rather than lost, and drops jobs naming photographs that have - // since been deleted. + // be resumed rather than lost, drops jobs naming photographs that have + // since been deleted, and drops rows of retired kinds — the thumbnail jobs + // 0.16.0 and earlier queued per photograph, which nothing claims (#73). // // Here rather than wherever a runner starts, because there is no owner // column in `jobs`: a second recovery pass while a worker held a claim @@ -819,9 +820,11 @@ fn adopt_catalog( match dr_catalog::runner::recover(cat.connection()) { Ok(r) if r.did_anything() => log::info!( "job queue: {} interrupted job(s) resumed, \ - {} for deleted photographs dropped", + {} for deleted photographs dropped, \ + {} of a retired kind dropped", r.reclaimed, - r.reaped + r.reaped, + r.retired ), Ok(_) => {} // Not surfaced. The queue is rebuildable like everything else in the