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) <noreply@anthropic.com>
This commit is contained in:
2
backend/Cargo.lock
generated
2
backend/Cargo.lock
generated
@@ -1517,7 +1517,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
|
|||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
name = "mangalord"
|
name = "mangalord"
|
||||||
version = "0.87.12"
|
version = "0.87.13"
|
||||||
dependencies = [
|
dependencies = [
|
||||||
"anyhow",
|
"anyhow",
|
||||||
"argon2",
|
"argon2",
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
[package]
|
[package]
|
||||||
name = "mangalord"
|
name = "mangalord"
|
||||||
version = "0.87.12"
|
version = "0.87.13"
|
||||||
edition = "2021"
|
edition = "2021"
|
||||||
default-run = "mangalord"
|
default-run = "mangalord"
|
||||||
|
|
||||||
|
|||||||
@@ -276,7 +276,23 @@ impl WorkerContext {
|
|||||||
);
|
);
|
||||||
let _ = jobs::release(&self.pool, lease.id).await;
|
let _ = jobs::release(&self.pool, lease.id).await;
|
||||||
// Don't publish Failed (the page didn't actually fail) and
|
// 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;
|
return;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|||||||
@@ -54,6 +54,20 @@ pub enum AnalysisEvent {
|
|||||||
chapter_number: i32,
|
chapter_number: i32,
|
||||||
page_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
|
/// Broadcaster shared on `AppState`. Cheap to clone via `Arc`. Publishing
|
||||||
|
|||||||
@@ -894,4 +894,27 @@ async fn cron_shutdown_does_not_block_on_in_flight_metadata_pass(pool: PgPool) {
|
|||||||
started.elapsed() < Duration::from_secs(5),
|
started.elapsed() < Duration::from_secs(5),
|
||||||
"shutdown took too long; cancel didn't propagate into the cron tick body"
|
"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;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
{
|
{
|
||||||
"name": "mangalord-frontend",
|
"name": "mangalord-frontend",
|
||||||
"version": "0.87.12",
|
"version": "0.87.13",
|
||||||
"private": true,
|
"private": true,
|
||||||
"type": "module",
|
"type": "module",
|
||||||
"scripts": {
|
"scripts": {
|
||||||
|
|||||||
@@ -857,7 +857,12 @@ export type AnalysisEvent =
|
|||||||
chapter_id: string | null;
|
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;
|
page_id: string;
|
||||||
manga_id: string;
|
manga_id: string;
|
||||||
manga_title: string;
|
manga_title: string;
|
||||||
|
|||||||
@@ -121,6 +121,16 @@
|
|||||||
|
|
||||||
function applyAnalysisEvent(ev: AnalysisEvent) {
|
function applyAnalysisEvent(ev: AnalysisEvent) {
|
||||||
if (ev.kind === 'enqueued') return;
|
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 =
|
const phase =
|
||||||
ev.kind === 'started' ? 'analyzing' : ev.kind === 'completed' ? 'done' : 'failed';
|
ev.kind === 'started' ? 'analyzing' : ev.kind === 'completed' ? 'done' : 'failed';
|
||||||
nowAnalyzing = {
|
nowAnalyzing = {
|
||||||
|
|||||||
@@ -247,6 +247,21 @@
|
|||||||
scheduleClear(ev.page_id);
|
scheduleClear(ev.page_id);
|
||||||
}
|
}
|
||||||
pushTicker(`✗ Failed page ${ev.page_number}`);
|
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}`);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user