Four 0.87.6 follow-ups from the adversarial review: 1. **Migration 0031 pre-dedup.** Demote all-but-lowest-id duplicate `analyze_page` rows in `(pending|running)` to `dead` before creating the unique index, with a curator-recoverable last_error marker. Without this, `sqlx::migrate!` would refuse to boot on any dirty production DB. 2. **`enqueue_for_page(force=true)` collision.** The partial unique index used to silently swallow force requests when a `force=false` job was already pending. Repo function now upgrades the pending row's `force` flag in place (or falls through to re-INSERT if the sibling drained mid-call), and reports an `EnqueueForPageOutcome` for accurate auditing. 3. **`record_duration` gate test.** New test seeds a `done` row with known duration, force-re-analyzes with failing dispatcher + max_attempts=3 (non-terminal), asserts duration_ms wasn't overwritten. 4. **`bookmark.rs` PK comment correction.** Use `b.id DESC` instead of the wrong `manga_id`; PK is actually `id`, and migration 0004 allows both chapter-level and manga-level bookmarks on the same manga. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
492 lines
16 KiB
Rust
492 lines
16 KiB
Rust
//! Integration tests for the analysis worker daemon: it leases only
|
|
//! `analyze_page` jobs, acks done on success, skips already-done pages
|
|
//! (unless forced), and on terminal failure writes a `failed` row.
|
|
|
|
use std::sync::Arc;
|
|
use std::time::Duration;
|
|
|
|
use mangalord::analysis::daemon::{
|
|
self,
|
|
test_support::{CountingDispatcher, SlowDispatcher},
|
|
AnalysisDaemonConfig,
|
|
};
|
|
use mangalord::domain::page_analysis::{
|
|
AnalysisStatus, SafetyFlag, VisionAnalysis,
|
|
};
|
|
use mangalord::repo::page_analysis;
|
|
use sqlx::PgPool;
|
|
use tokio_util::sync::CancellationToken;
|
|
use uuid::Uuid;
|
|
|
|
async fn seed_page(pool: &PgPool) -> Uuid {
|
|
let manga_id: Uuid =
|
|
sqlx::query_scalar("INSERT INTO mangas (title) VALUES ('M') RETURNING id")
|
|
.fetch_one(pool)
|
|
.await
|
|
.unwrap();
|
|
let chapter_id: Uuid = sqlx::query_scalar(
|
|
"INSERT INTO chapters (manga_id, number) VALUES ($1, 1) RETURNING id",
|
|
)
|
|
.bind(manga_id)
|
|
.fetch_one(pool)
|
|
.await
|
|
.unwrap();
|
|
sqlx::query_scalar(
|
|
"INSERT INTO pages (chapter_id, page_number, storage_key, content_type) \
|
|
VALUES ($1, 1, 'k/1.png', 'image/png') RETURNING id",
|
|
)
|
|
.bind(chapter_id)
|
|
.fetch_one(pool)
|
|
.await
|
|
.unwrap()
|
|
}
|
|
|
|
async fn job_state(pool: &PgPool, page_id: Uuid) -> Option<String> {
|
|
sqlx::query_scalar(
|
|
"SELECT state FROM crawler_jobs \
|
|
WHERE payload->>'kind' = 'analyze_page' AND payload->>'page_id' = $1",
|
|
)
|
|
.bind(page_id.to_string())
|
|
.fetch_optional(pool)
|
|
.await
|
|
.unwrap()
|
|
}
|
|
|
|
/// Poll until the page's analyze_page job reaches `want`, or fail after 5s.
|
|
async fn wait_for_state(pool: &PgPool, page_id: Uuid, want: &str) {
|
|
let deadline = Duration::from_secs(5);
|
|
let result = tokio::time::timeout(deadline, async {
|
|
loop {
|
|
if job_state(pool, page_id).await.as_deref() == Some(want) {
|
|
return;
|
|
}
|
|
tokio::time::sleep(Duration::from_millis(50)).await;
|
|
}
|
|
})
|
|
.await;
|
|
assert!(result.is_ok(), "job for {page_id} never reached state {want}");
|
|
}
|
|
|
|
fn spawn_with(
|
|
pool: &PgPool,
|
|
dispatcher: Arc<CountingDispatcher>,
|
|
) -> (daemon::AnalysisDaemonHandle, CancellationToken) {
|
|
let cancel = CancellationToken::new();
|
|
let handle = daemon::spawn(
|
|
pool.clone(),
|
|
cancel.clone(),
|
|
AnalysisDaemonConfig {
|
|
dispatcher,
|
|
workers: 1,
|
|
job_timeout: Duration::from_secs(5),
|
|
events: std::sync::Arc::new(
|
|
mangalord::analysis::events::AnalysisEvents::new(),
|
|
),
|
|
readiness: None,
|
|
},
|
|
);
|
|
(handle, cancel)
|
|
}
|
|
|
|
#[sqlx::test(migrations = "./migrations")]
|
|
async fn worker_dispatches_and_acks_done(pool: PgPool) {
|
|
let page_id = seed_page(&pool).await;
|
|
page_analysis::enqueue_for_page(&pool, page_id, false)
|
|
.await
|
|
.unwrap();
|
|
|
|
let dispatcher = CountingDispatcher::ok();
|
|
let (handle, _c) = spawn_with(&pool, dispatcher.clone());
|
|
wait_for_state(&pool, page_id, "done").await;
|
|
handle.shutdown().await;
|
|
|
|
assert_eq!(dispatcher.call_count(), 1);
|
|
}
|
|
|
|
#[sqlx::test(migrations = "./migrations")]
|
|
async fn worker_skips_already_done_page_unless_forced(pool: PgPool) {
|
|
let page_id = seed_page(&pool).await;
|
|
// Pre-mark the page analyzed.
|
|
page_analysis::persist_analysis(
|
|
&pool,
|
|
page_id,
|
|
&VisionAnalysis {
|
|
ocr_results: vec![],
|
|
tagging_results: vec![],
|
|
scene_description: String::new(),
|
|
safety_flag: SafetyFlag::default(),
|
|
},
|
|
"m",
|
|
)
|
|
.await
|
|
.unwrap();
|
|
page_analysis::enqueue_for_page(&pool, page_id, false)
|
|
.await
|
|
.unwrap();
|
|
|
|
let dispatcher = CountingDispatcher::ok();
|
|
let (handle, _c) = spawn_with(&pool, dispatcher.clone());
|
|
wait_for_state(&pool, page_id, "done").await;
|
|
handle.shutdown().await;
|
|
|
|
assert_eq!(
|
|
dispatcher.call_count(),
|
|
0,
|
|
"a non-forced job for a done page must skip dispatch"
|
|
);
|
|
}
|
|
|
|
#[sqlx::test(migrations = "./migrations")]
|
|
async fn worker_reanalyzes_done_page_when_forced(pool: PgPool) {
|
|
let page_id = seed_page(&pool).await;
|
|
page_analysis::persist_analysis(
|
|
&pool,
|
|
page_id,
|
|
&VisionAnalysis {
|
|
ocr_results: vec![],
|
|
tagging_results: vec![],
|
|
scene_description: String::new(),
|
|
safety_flag: SafetyFlag::default(),
|
|
},
|
|
"m",
|
|
)
|
|
.await
|
|
.unwrap();
|
|
page_analysis::enqueue_for_page(&pool, page_id, true)
|
|
.await
|
|
.unwrap();
|
|
|
|
let dispatcher = CountingDispatcher::ok();
|
|
let (handle, _c) = spawn_with(&pool, dispatcher.clone());
|
|
wait_for_state(&pool, page_id, "done").await;
|
|
handle.shutdown().await;
|
|
|
|
assert_eq!(dispatcher.call_count(), 1, "force must re-dispatch");
|
|
|
|
// The worker stamps how long the dispatch took onto the existing row
|
|
// (recorded after ack; shutdown has awaited the in-flight job).
|
|
let dur: Option<i64> =
|
|
sqlx::query_scalar("SELECT duration_ms FROM page_analysis WHERE page_id = $1")
|
|
.bind(page_id)
|
|
.fetch_one(&pool)
|
|
.await
|
|
.unwrap();
|
|
assert!(dur.is_some(), "worker should record analysis duration_ms");
|
|
}
|
|
|
|
#[sqlx::test(migrations = "./migrations")]
|
|
async fn worker_marks_failed_row_on_terminal_failure(pool: PgPool) {
|
|
let page_id = seed_page(&pool).await;
|
|
page_analysis::enqueue_for_page(&pool, page_id, false)
|
|
.await
|
|
.unwrap();
|
|
// Make the first failure terminal (no backoff wait in the test).
|
|
sqlx::query(
|
|
"UPDATE crawler_jobs SET max_attempts = 1 \
|
|
WHERE payload->>'page_id' = $1",
|
|
)
|
|
.bind(page_id.to_string())
|
|
.execute(&pool)
|
|
.await
|
|
.unwrap();
|
|
|
|
let dispatcher = CountingDispatcher::failing();
|
|
let (handle, _c) = spawn_with(&pool, dispatcher.clone());
|
|
wait_for_state(&pool, page_id, "dead").await;
|
|
handle.shutdown().await;
|
|
|
|
let row = page_analysis::load(&pool, page_id).await.unwrap().unwrap();
|
|
assert_eq!(row.status, AnalysisStatus::Failed);
|
|
assert!(row.error.is_some());
|
|
}
|
|
|
|
/// Non-terminal failure of a force-re-analyze must NOT overwrite the
|
|
/// prior `done` row's `duration_ms`. Before the 0.87.6 gate change,
|
|
/// `record_duration` ran unconditionally — a transient failure-retry
|
|
/// of a force-re-analyze would clobber the legitimately-measured
|
|
/// previous duration with the duration of an attempt that wrote
|
|
/// nothing. Test pins the gate.
|
|
#[sqlx::test(migrations = "./migrations")]
|
|
async fn worker_non_terminal_force_failure_does_not_overwrite_done_duration(pool: PgPool) {
|
|
let page_id = seed_page(&pool).await;
|
|
// 1) Pre-seed a done analysis row with a meaningful duration so we
|
|
// have something to be "overwritten".
|
|
page_analysis::persist_analysis(
|
|
&pool,
|
|
page_id,
|
|
&VisionAnalysis {
|
|
ocr_results: vec![],
|
|
tagging_results: vec![],
|
|
scene_description: String::new(),
|
|
safety_flag: SafetyFlag::default(),
|
|
},
|
|
"m",
|
|
)
|
|
.await
|
|
.unwrap();
|
|
sqlx::query("UPDATE page_analysis SET duration_ms = $1 WHERE page_id = $2")
|
|
.bind(12345_i64)
|
|
.bind(page_id)
|
|
.execute(&pool)
|
|
.await
|
|
.unwrap();
|
|
|
|
// 2) Force re-analyze (force=true) — but with a failing dispatcher
|
|
// AND max_attempts=3 so the first failure is NON-terminal.
|
|
page_analysis::enqueue_for_page(&pool, page_id, true)
|
|
.await
|
|
.unwrap();
|
|
sqlx::query("UPDATE crawler_jobs SET max_attempts = 3 WHERE payload->>'page_id' = $1")
|
|
.bind(page_id.to_string())
|
|
.execute(&pool)
|
|
.await
|
|
.unwrap();
|
|
|
|
let dispatcher = CountingDispatcher::failing();
|
|
let (handle, cancel) = spawn_with(&pool, dispatcher.clone());
|
|
|
|
// 3) Wait until the worker has dispatched once — the job goes back
|
|
// to `pending` because the retry is non-terminal (attempts < max).
|
|
let deadline = std::time::Instant::now() + Duration::from_secs(5);
|
|
while dispatcher.call_count() == 0 && std::time::Instant::now() < deadline {
|
|
tokio::time::sleep(Duration::from_millis(50)).await;
|
|
}
|
|
assert!(dispatcher.call_count() >= 1, "dispatcher must have run");
|
|
|
|
// Wait briefly for `record_duration` to either run or get gated.
|
|
// Then cancel before the next backoff fires (it'd burn 60s otherwise).
|
|
tokio::time::sleep(Duration::from_millis(200)).await;
|
|
cancel.cancel();
|
|
handle.shutdown().await;
|
|
|
|
// 4) The prior `done` row's `duration_ms` must NOT have been
|
|
// overwritten by the failed retry's duration.
|
|
let dur: Option<i64> =
|
|
sqlx::query_scalar("SELECT duration_ms FROM page_analysis WHERE page_id = $1")
|
|
.bind(page_id)
|
|
.fetch_one(&pool)
|
|
.await
|
|
.unwrap();
|
|
assert_eq!(
|
|
dur,
|
|
Some(12345),
|
|
"non-terminal failure must not overwrite the prior done row's duration"
|
|
);
|
|
}
|
|
|
|
#[sqlx::test(migrations = "./migrations")]
|
|
async fn worker_isolates_dispatcher_panics(pool: PgPool) {
|
|
let page_id = seed_page(&pool).await;
|
|
page_analysis::enqueue_for_page(&pool, page_id, false)
|
|
.await
|
|
.unwrap();
|
|
sqlx::query(
|
|
"UPDATE crawler_jobs SET max_attempts = 1 WHERE payload->>'page_id' = $1",
|
|
)
|
|
.bind(page_id.to_string())
|
|
.execute(&pool)
|
|
.await
|
|
.unwrap();
|
|
|
|
let dispatcher = CountingDispatcher::panicking();
|
|
let (handle, _c) = spawn_with(&pool, dispatcher.clone());
|
|
wait_for_state(&pool, page_id, "dead").await;
|
|
handle.shutdown().await;
|
|
|
|
// The worker survived the panic and recorded a failed row.
|
|
let row = page_analysis::load(&pool, page_id).await.unwrap().unwrap();
|
|
assert_eq!(row.status, AnalysisStatus::Failed);
|
|
}
|
|
|
|
#[sqlx::test(migrations = "./migrations")]
|
|
async fn worker_publishes_started_and_completed_events(pool: PgPool) {
|
|
let page_id = seed_page(&pool).await;
|
|
page_analysis::enqueue_for_page(&pool, page_id, false)
|
|
.await
|
|
.unwrap();
|
|
|
|
let events = Arc::new(mangalord::analysis::events::AnalysisEvents::new());
|
|
let mut rx = events.subscribe();
|
|
let cancel = CancellationToken::new();
|
|
let handle = daemon::spawn(
|
|
pool.clone(),
|
|
cancel,
|
|
AnalysisDaemonConfig {
|
|
dispatcher: CountingDispatcher::ok(),
|
|
workers: 1,
|
|
job_timeout: Duration::from_secs(5),
|
|
events: events.clone(),
|
|
readiness: None,
|
|
},
|
|
);
|
|
|
|
let mut kinds = Vec::new();
|
|
let _ = tokio::time::timeout(Duration::from_secs(5), async {
|
|
while let Ok(ev) = rx.recv().await {
|
|
let v = serde_json::to_value(&ev).unwrap();
|
|
let kind = v["kind"].as_str().unwrap().to_string();
|
|
// Each event must carry the page breadcrumb.
|
|
assert_eq!(v["page_id"].as_str().unwrap(), page_id.to_string());
|
|
let done = kind == "completed";
|
|
kinds.push(kind);
|
|
if done {
|
|
break;
|
|
}
|
|
}
|
|
})
|
|
.await;
|
|
handle.shutdown().await;
|
|
|
|
assert!(kinds.contains(&"started".to_string()), "expected a started event");
|
|
assert!(
|
|
kinds.contains(&"completed".to_string()),
|
|
"expected a completed event"
|
|
);
|
|
}
|
|
|
|
#[sqlx::test(migrations = "./migrations")]
|
|
async fn worker_publishes_failed_event_on_dispatch_error(pool: PgPool) {
|
|
let page_id = seed_page(&pool).await;
|
|
page_analysis::enqueue_for_page(&pool, page_id, false)
|
|
.await
|
|
.unwrap();
|
|
sqlx::query("UPDATE crawler_jobs SET max_attempts = 1 WHERE payload->>'page_id' = $1")
|
|
.bind(page_id.to_string())
|
|
.execute(&pool)
|
|
.await
|
|
.unwrap();
|
|
|
|
let events = Arc::new(mangalord::analysis::events::AnalysisEvents::new());
|
|
let mut rx = events.subscribe();
|
|
let cancel = CancellationToken::new();
|
|
let handle = daemon::spawn(
|
|
pool.clone(),
|
|
cancel,
|
|
AnalysisDaemonConfig {
|
|
dispatcher: CountingDispatcher::failing(),
|
|
workers: 1,
|
|
job_timeout: Duration::from_secs(5),
|
|
events: events.clone(),
|
|
readiness: None,
|
|
},
|
|
);
|
|
|
|
let mut saw_failed = false;
|
|
let _ = tokio::time::timeout(Duration::from_secs(5), async {
|
|
while let Ok(ev) = rx.recv().await {
|
|
let v = serde_json::to_value(&ev).unwrap();
|
|
if v["kind"] == "failed" {
|
|
saw_failed = true;
|
|
break;
|
|
}
|
|
}
|
|
})
|
|
.await;
|
|
handle.shutdown().await;
|
|
assert!(saw_failed, "expected a failed event");
|
|
}
|
|
|
|
#[sqlx::test(migrations = "./migrations")]
|
|
async fn worker_ignores_non_analyze_jobs(pool: PgPool) {
|
|
// A crawl job must be left untouched by the analysis worker.
|
|
use mangalord::crawler::jobs::{self, JobPayload};
|
|
let crawl = jobs::enqueue(
|
|
&pool,
|
|
&JobPayload::SyncManga {
|
|
source_id: "s".into(),
|
|
source_manga_key: "k".into(),
|
|
url: "https://target.example/manga/k".into(),
|
|
title: "K".into(),
|
|
},
|
|
)
|
|
.await
|
|
.unwrap();
|
|
let crawl_id = match crawl {
|
|
jobs::EnqueueResult::Inserted(id) => id,
|
|
jobs::EnqueueResult::Skipped => panic!("expected insert"),
|
|
};
|
|
|
|
let page_id = seed_page(&pool).await;
|
|
page_analysis::enqueue_for_page(&pool, page_id, false)
|
|
.await
|
|
.unwrap();
|
|
|
|
let dispatcher = CountingDispatcher::ok();
|
|
let (handle, _c) = spawn_with(&pool, dispatcher.clone());
|
|
wait_for_state(&pool, page_id, "done").await;
|
|
handle.shutdown().await;
|
|
|
|
let crawl_state: String =
|
|
sqlx::query_scalar("SELECT state FROM crawler_jobs WHERE id = $1")
|
|
.bind(crawl_id)
|
|
.fetch_one(&pool)
|
|
.await
|
|
.unwrap();
|
|
assert_eq!(crawl_state, "pending", "crawl job must be left for the crawler");
|
|
}
|
|
|
|
/// Shutdown must race in-flight dispatches against cancellation, not wait
|
|
/// for them to finish. The previous behaviour blocked `handle.shutdown()`
|
|
/// for up to `job_timeout` (default 600s) on one in-flight vision call,
|
|
/// which also stranded SIGTERM and any `Supervisors::reload_analysis`
|
|
/// caller under the supervisor lock.
|
|
#[sqlx::test(migrations = "./migrations")]
|
|
async fn worker_shutdown_does_not_block_on_in_flight_dispatch(pool: PgPool) {
|
|
let page_id = seed_page(&pool).await;
|
|
page_analysis::enqueue_for_page(&pool, page_id, false)
|
|
.await
|
|
.unwrap();
|
|
|
|
// 30s dispatch + 30s job_timeout. Shutdown must observe the cancel
|
|
// signal and return well before either deadline; if it waits for
|
|
// dispatch to finish the test would take 30s.
|
|
let dispatcher = SlowDispatcher::new(Duration::from_secs(30));
|
|
let cancel = CancellationToken::new();
|
|
let handle = daemon::spawn(
|
|
pool.clone(),
|
|
cancel.clone(),
|
|
AnalysisDaemonConfig {
|
|
dispatcher: dispatcher.clone(),
|
|
workers: 1,
|
|
job_timeout: Duration::from_secs(30),
|
|
events: std::sync::Arc::new(
|
|
mangalord::analysis::events::AnalysisEvents::new(),
|
|
),
|
|
readiness: None,
|
|
},
|
|
);
|
|
|
|
// Wait for the worker to lease the job and enter the dispatch.
|
|
wait_for_state(&pool, page_id, "running").await;
|
|
assert_eq!(
|
|
dispatcher.call_count(),
|
|
1,
|
|
"dispatcher should be in flight when we cancel"
|
|
);
|
|
|
|
// Now cancel. Shutdown must return within a few seconds — comfortably
|
|
// under both the slow-dispatch and the job_timeout deadlines.
|
|
let started = std::time::Instant::now();
|
|
tokio::time::timeout(Duration::from_secs(5), handle.shutdown())
|
|
.await
|
|
.expect("shutdown must observe cancel and return promptly");
|
|
assert!(
|
|
started.elapsed() < Duration::from_secs(5),
|
|
"shutdown took too long; cancel didn't propagate into dispatch"
|
|
);
|
|
|
|
// The lease was released, not failed — attempts must not have grown.
|
|
let attempts: i32 = sqlx::query_scalar(
|
|
"SELECT attempts FROM crawler_jobs \
|
|
WHERE payload->>'kind' = 'analyze_page' AND payload->>'page_id' = $1",
|
|
)
|
|
.bind(page_id.to_string())
|
|
.fetch_one(&pool)
|
|
.await
|
|
.unwrap();
|
|
assert_eq!(
|
|
attempts, 0,
|
|
"cancellation must not burn a job attempt (release, not ack_failed)"
|
|
);
|
|
}
|