From 651fb27522e350b2c8f90e44cd6c763d7a4a8369 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Tue, 23 Jun 2026 08:01:53 +0200 Subject: [PATCH] fix(analysis): emit Cancelled event + assert lock-release after cancel (0.87.13) Two 0.87.4 follow-ups: 1. Cron tick: existing shutdown-on-cancel test now also asserts the advisory lock is re-acquirable after cancel-during-tick. Without this, a refactor moving `pg_advisory_unlock` under the cancel arm would ship green and break multi-replica safety. 2. Analysis worker: cancel-mid-dispatch left the dashboard banner stuck on the cancelled page until a later page overwrote it. Add a `Cancelled` AnalysisEvent variant, publish on the cancel-release branch, wire the analysis and admin overview dashboards to clear on it. Per-page chip rolls back from `analyzing` to `queued`. Co-Authored-By: Claude Opus 4.7 (1M context) --- backend/Cargo.lock | 2 +- backend/Cargo.toml | 2 +- backend/src/analysis/daemon.rs | 18 ++++++++++++++- backend/src/analysis/events.rs | 14 +++++++++++ backend/tests/crawler_daemon.rs | 23 +++++++++++++++++++ frontend/package.json | 2 +- frontend/src/lib/api/admin.ts | 7 +++++- frontend/src/routes/admin/+page.svelte | 10 ++++++++ .../src/routes/admin/analysis/+page.svelte | 15 ++++++++++++ 9 files changed, 88 insertions(+), 5 deletions(-) diff --git a/backend/Cargo.lock b/backend/Cargo.lock index 0c74754..ee26703 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -1517,7 +1517,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4" [[package]] name = "mangalord" -version = "0.87.12" +version = "0.87.13" dependencies = [ "anyhow", "argon2", diff --git a/backend/Cargo.toml b/backend/Cargo.toml index f1c6d46..6954dc6 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mangalord" -version = "0.87.12" +version = "0.87.13" edition = "2021" default-run = "mangalord" diff --git a/backend/src/analysis/daemon.rs b/backend/src/analysis/daemon.rs index 03cc16e..9a5a2af 100644 --- a/backend/src/analysis/daemon.rs +++ b/backend/src/analysis/daemon.rs @@ -276,7 +276,23 @@ impl WorkerContext { ); let _ = jobs::release(&self.pool, lease.id).await; // Don't publish Failed (the page didn't actually fail) and - // don't write a duration row — neither outcome is true. + // don't write a duration row — neither outcome is true. DO + // publish Cancelled so the dashboard's "now analyzing" banner + // can clear the page we previously sent Started for. Without + // this the banner stuck on the cancelled page until a later + // page kicked off and overwrote it. + if let Some((manga_id, manga_title, chapter_id, chapter_number, page_number)) = + breadcrumb + { + self.events.publish(AnalysisEvent::Cancelled { + page_id, + manga_id, + manga_title, + chapter_id, + chapter_number, + page_number, + }); + } return; }; diff --git a/backend/src/analysis/events.rs b/backend/src/analysis/events.rs index 3414fdb..354c75d 100644 --- a/backend/src/analysis/events.rs +++ b/backend/src/analysis/events.rs @@ -54,6 +54,20 @@ pub enum AnalysisEvent { chapter_number: i32, page_number: i32, }, + /// The worker cancelled an in-flight dispatch (daemon shutdown or + /// settings reload toggling analysis off). The lease was released, + /// not failed — but the dashboard banner already saw `Started` and + /// only clears on `Completed`/`Failed`/`Cancelled`. Without this + /// variant the banner stuck on the cancelled page forever (until a + /// later page kicked off and overwrote it). + Cancelled { + page_id: Uuid, + manga_id: Uuid, + manga_title: String, + chapter_id: Uuid, + chapter_number: i32, + page_number: i32, + }, } /// Broadcaster shared on `AppState`. Cheap to clone via `Arc`. Publishing diff --git a/backend/tests/crawler_daemon.rs b/backend/tests/crawler_daemon.rs index e5e4c31..f865715 100644 --- a/backend/tests/crawler_daemon.rs +++ b/backend/tests/crawler_daemon.rs @@ -894,4 +894,27 @@ async fn cron_shutdown_does_not_block_on_in_flight_metadata_pass(pool: PgPool) { started.elapsed() < Duration::from_secs(5), "shutdown took too long; cancel didn't propagate into the cron tick body" ); + + // The advisory lock must be released so another replica's cron can + // pick up the slot. Without this assertion, a refactor that moved + // the `pg_advisory_unlock` under the cancel arm (instead of running + // unconditionally below the select!) would ship green — only to + // wedge multi-replica deployments where the second replica's cron + // would block on `pg_try_advisory_lock` forever. `pg_try_advisory_lock` + // returns true iff the lock is free, so we use it as the probe. + let acquired: bool = sqlx::query_scalar("SELECT pg_try_advisory_lock($1)") + .bind(CRON_LOCK_KEY) + .fetch_one(&pool) + .await + .unwrap(); + assert!( + acquired, + "advisory lock must be released after cancel-during-tick \ + (otherwise multi-replica safety breaks)" + ); + // Be a good citizen for whatever sqlx test runs next. + let _ = sqlx::query("SELECT pg_advisory_unlock($1)") + .bind(CRON_LOCK_KEY) + .execute(&pool) + .await; } diff --git a/frontend/package.json b/frontend/package.json index f2ae3bc..e8445c2 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mangalord-frontend", - "version": "0.87.12", + "version": "0.87.13", "private": true, "type": "module", "scripts": { diff --git a/frontend/src/lib/api/admin.ts b/frontend/src/lib/api/admin.ts index 071d68d..5e3b0e3 100644 --- a/frontend/src/lib/api/admin.ts +++ b/frontend/src/lib/api/admin.ts @@ -857,7 +857,12 @@ export type AnalysisEvent = chapter_id: string | null; } | { - kind: 'started' | 'completed' | 'failed'; + // `cancelled` fires when the worker drops an in-flight dispatch + // on daemon shutdown / settings-toggle-off. The lease was + // released, not failed — the page returns to pending. The + // dashboard banner clears on this so it doesn't stick on the + // cancelled page until another `started` overwrites it. + kind: 'started' | 'completed' | 'failed' | 'cancelled'; page_id: string; manga_id: string; manga_title: string; diff --git a/frontend/src/routes/admin/+page.svelte b/frontend/src/routes/admin/+page.svelte index 698ff81..833fb9c 100644 --- a/frontend/src/routes/admin/+page.svelte +++ b/frontend/src/routes/admin/+page.svelte @@ -121,6 +121,16 @@ function applyAnalysisEvent(ev: AnalysisEvent) { if (ev.kind === 'enqueued') return; + // Cancelled (daemon shutdown / settings toggle-off mid-dispatch) + // is a non-terminal release — the lease went back to pending. + // Clear the banner instead of frame-flipping to a "done"/"failed" + // misrepresentation. + if (ev.kind === 'cancelled') { + if (nowAnalyzing && nowAnalyzing.page === ev.page_number) { + nowAnalyzing = null; + } + return; + } const phase = ev.kind === 'started' ? 'analyzing' : ev.kind === 'completed' ? 'done' : 'failed'; nowAnalyzing = { diff --git a/frontend/src/routes/admin/analysis/+page.svelte b/frontend/src/routes/admin/analysis/+page.svelte index b00ce14..1a61b8b 100644 --- a/frontend/src/routes/admin/analysis/+page.svelte +++ b/frontend/src/routes/admin/analysis/+page.svelte @@ -247,6 +247,21 @@ scheduleClear(ev.page_id); } pushTicker(`✗ Failed page ${ev.page_number}`); + } else if (ev.kind === 'cancelled') { + // Daemon shutdown / analysis toggled off mid-dispatch. The + // lease was released, not failed — the page is still pending + // server-side. We just need to clear the "Analyzing pN" banner + // and roll the per-page live status back to queued so the + // chip doesn't stay spinner-frozen forever. + const cur = liveStatus[ev.page_id]; + if (cur === 'analyzing') { + liveStatus = { ...liveStatus, [ev.page_id]: 'queued' }; + } + if (nowAnalyzing?.page_id === ev.page_id) { + nowAnalyzing = null; + if (clearTimer) clearTimeout(clearTimer); + } + pushTicker(`↺ Cancelled page ${ev.page_number}`); } }