fix: don't burn a job attempt when the browser is unavailable
A job's attempt is incremented at lease time, then the dispatcher acquires the headless browser. When the browser was down or mid-restart, acquire() failed and the error was ack_failed — so during an outage the whole pending backlog was chewed to `dead` within minutes while workers hot-looped. Add SyncOutcome::BrowserUnavailable: the dispatcher returns it instead of propagating the acquire error, and the daemon releases the lease (refunding the attempt, like the cancel/session paths) and backs off, so an outage is treated as an infrastructure blip rather than per-job failures. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
2
backend/Cargo.lock
generated
2
backend/Cargo.lock
generated
@@ -1558,7 +1558,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
|
|||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
name = "mangalord"
|
name = "mangalord"
|
||||||
version = "0.124.16"
|
version = "0.124.17"
|
||||||
dependencies = [
|
dependencies = [
|
||||||
"anyhow",
|
"anyhow",
|
||||||
"argon2",
|
"argon2",
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
[package]
|
[package]
|
||||||
name = "mangalord"
|
name = "mangalord"
|
||||||
version = "0.124.16"
|
version = "0.124.17"
|
||||||
edition = "2021"
|
edition = "2021"
|
||||||
default-run = "mangalord"
|
default-run = "mangalord"
|
||||||
|
|
||||||
|
|||||||
@@ -990,7 +990,17 @@ impl ChapterDispatcher for RealChapterDispatcher {
|
|||||||
pages_done: 0,
|
pages_done: 0,
|
||||||
pages_total: None,
|
pages_total: None,
|
||||||
});
|
});
|
||||||
let lease = self.browser_manager.acquire().await?;
|
let lease = match self.browser_manager.acquire().await {
|
||||||
|
Ok(l) => l,
|
||||||
|
Err(e) => {
|
||||||
|
// Browser down / mid-restart: defer the job WITHOUT
|
||||||
|
// burning an attempt (the daemon releases it back to
|
||||||
|
// pending) rather than counting an infrastructure
|
||||||
|
// outage as a per-job failure.
|
||||||
|
tracing::warn!(error = ?e, "dispatch: browser unavailable — deferring job");
|
||||||
|
return Ok(SyncOutcome::BrowserUnavailable);
|
||||||
|
}
|
||||||
|
};
|
||||||
let result = content::sync_chapter_content(
|
let result = content::sync_chapter_content(
|
||||||
&lease,
|
&lease,
|
||||||
&self.db,
|
&self.db,
|
||||||
@@ -1061,7 +1071,15 @@ impl ChapterDispatcher for RealChapterDispatcher {
|
|||||||
// Scope the lease so it (and the borrowing FetchContext) drop
|
// Scope the lease so it (and the borrowing FetchContext) drop
|
||||||
// before any browser-restart handling in the match below.
|
// before any browser-restart handling in the match below.
|
||||||
let result = {
|
let result = {
|
||||||
let lease = self.browser_manager.acquire().await?;
|
let lease = match self.browser_manager.acquire().await {
|
||||||
|
Ok(l) => l,
|
||||||
|
Err(e) => {
|
||||||
|
// See the SyncChapterContent arm: defer without
|
||||||
|
// burning an attempt when the browser is unavailable.
|
||||||
|
tracing::warn!(error = ?e, "dispatch: browser unavailable — deferring job");
|
||||||
|
return Ok(SyncOutcome::BrowserUnavailable);
|
||||||
|
}
|
||||||
|
};
|
||||||
let ctx = crate::crawler::source::FetchContext {
|
let ctx = crate::crawler::source::FetchContext {
|
||||||
browser: &lease,
|
browser: &lease,
|
||||||
rate: &self.rate,
|
rate: &self.rate,
|
||||||
|
|||||||
@@ -441,6 +441,13 @@ async fn sync_bookmarked_chapter_content(
|
|||||||
s.fetched += 1;
|
s.fetched += 1;
|
||||||
}
|
}
|
||||||
Ok(SyncOutcome::Skipped) => s.skipped += 1,
|
Ok(SyncOutcome::Skipped) => s.skipped += 1,
|
||||||
|
// Unreachable in the one-shot CLI (it holds its own lease
|
||||||
|
// and never dispatches through the queue), but count it as a
|
||||||
|
// failure for exhaustiveness.
|
||||||
|
Ok(SyncOutcome::BrowserUnavailable) => {
|
||||||
|
tracing::warn!(%chapter_id, "crawler browser unavailable");
|
||||||
|
s.failed += 1;
|
||||||
|
}
|
||||||
Ok(SyncOutcome::SessionExpired) => {
|
Ok(SyncOutcome::SessionExpired) => {
|
||||||
tracing::error!(
|
tracing::error!(
|
||||||
%chapter_id,
|
%chapter_id,
|
||||||
|
|||||||
@@ -71,6 +71,13 @@ pub enum SyncOutcome {
|
|||||||
/// Session probe failed mid-sync (avatar selector missing on the
|
/// Session probe failed mid-sync (avatar selector missing on the
|
||||||
/// chapter page). Caller should abort the whole crawler run.
|
/// chapter page). Caller should abort the whole crawler run.
|
||||||
SessionExpired,
|
SessionExpired,
|
||||||
|
/// The headless browser could not be acquired (down or mid-restart).
|
||||||
|
/// Produced only by the queue dispatcher (which calls `acquire()`); it is
|
||||||
|
/// an *infrastructure* outage, not a job failure, so the daemon returns the
|
||||||
|
/// job to `pending` WITHOUT burning a retry attempt. `sync_chapter_content`
|
||||||
|
/// itself never returns this — callers that already hold a lease can treat
|
||||||
|
/// it as unreachable.
|
||||||
|
BrowserUnavailable,
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Per-chapter max fetch attempts when TOR is configured. `N = 3` means
|
/// Per-chapter max fetch attempts when TOR is configured. `N = 3` means
|
||||||
|
|||||||
@@ -66,6 +66,12 @@ const LEASE_DURATION: Duration = Duration::from_secs(60);
|
|||||||
/// the lease window leaves two missed-beat's slack before expiry.
|
/// the lease window leaves two missed-beat's slack before expiry.
|
||||||
const LEASE_HEARTBEAT: Duration = Duration::from_secs(20);
|
const LEASE_HEARTBEAT: Duration = Duration::from_secs(20);
|
||||||
|
|
||||||
|
/// How long a worker waits after a `BrowserUnavailable` outcome before looping
|
||||||
|
/// back to lease again. The job was released (not failed) so it stays pending;
|
||||||
|
/// this backoff keeps the worker from hot-looping lease→acquire→release while
|
||||||
|
/// the browser is down or mid-restart.
|
||||||
|
const BROWSER_UNAVAILABLE_BACKOFF: Duration = Duration::from_secs(5);
|
||||||
|
|
||||||
#[async_trait]
|
#[async_trait]
|
||||||
pub trait MetadataPass: Send + Sync {
|
pub trait MetadataPass: Send + Sync {
|
||||||
async fn run(&self) -> anyhow::Result<pipeline::MetadataStats>;
|
async fn run(&self) -> anyhow::Result<pipeline::MetadataStats>;
|
||||||
@@ -502,6 +508,24 @@ impl WorkerContext {
|
|||||||
self.status.poke();
|
self.status.poke();
|
||||||
let _ = jobs::release(&self.pool, lease.id, lease.lease_generation).await;
|
let _ = jobs::release(&self.pool, lease.id, lease.lease_generation).await;
|
||||||
}
|
}
|
||||||
|
Ok(Ok(SyncOutcome::BrowserUnavailable)) => {
|
||||||
|
// Infrastructure outage, not a job failure: the browser was
|
||||||
|
// down or mid-restart when the dispatcher tried to acquire it.
|
||||||
|
// Return the job to `pending` WITHOUT burning an attempt (like
|
||||||
|
// the cancel/session paths) so an outage doesn't chew the whole
|
||||||
|
// backlog to `dead`, then back off to avoid hot-looping while
|
||||||
|
// the browser recovers.
|
||||||
|
tracing::warn!(
|
||||||
|
worker = self.id,
|
||||||
|
lease_id = %lease.id,
|
||||||
|
"worker: browser unavailable — released lease without burning an attempt"
|
||||||
|
);
|
||||||
|
let _ = jobs::release(&self.pool, lease.id, lease.lease_generation).await;
|
||||||
|
tokio::select! {
|
||||||
|
_ = tokio::time::sleep(BROWSER_UNAVAILABLE_BACKOFF) => {}
|
||||||
|
_ = self.cancel.cancelled() => {}
|
||||||
|
}
|
||||||
|
}
|
||||||
Ok(Err(e)) => {
|
Ok(Err(e)) => {
|
||||||
tracing::warn!(
|
tracing::warn!(
|
||||||
worker = self.id,
|
worker = self.id,
|
||||||
|
|||||||
@@ -280,6 +280,12 @@ impl ResyncService for RealResyncService {
|
|||||||
SyncOutcome::SessionExpired => {
|
SyncOutcome::SessionExpired => {
|
||||||
anyhow::bail!("source session expired — operator must refresh PHPSESSID")
|
anyhow::bail!("source session expired — operator must refresh PHPSESSID")
|
||||||
}
|
}
|
||||||
|
// Unreachable here: resync already holds its own browser lease and
|
||||||
|
// `sync_chapter_content` never acquires one, so it can't report the
|
||||||
|
// browser unavailable. Handled defensively for exhaustiveness.
|
||||||
|
SyncOutcome::BrowserUnavailable => {
|
||||||
|
anyhow::bail!("crawler browser unavailable")
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -104,6 +104,20 @@ impl ChapterDispatcher for FailingDispatcher {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Always reports the browser unavailable — models a Chromium outage where
|
||||||
|
/// `acquire()` fails. The job must be returned to `pending` WITHOUT burning an
|
||||||
|
/// attempt, so an outage doesn't chew the backlog to `dead`.
|
||||||
|
struct BrowserUnavailableDispatcher {
|
||||||
|
seen: AtomicUsize,
|
||||||
|
}
|
||||||
|
#[async_trait::async_trait]
|
||||||
|
impl ChapterDispatcher for BrowserUnavailableDispatcher {
|
||||||
|
async fn dispatch(&self, _payload: JobPayload) -> anyhow::Result<SyncOutcome> {
|
||||||
|
self.seen.fetch_add(1, Ordering::AcqRel);
|
||||||
|
Ok(SyncOutcome::BrowserUnavailable)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Never completes — used to verify the worker's outer dispatch timeout.
|
/// Never completes — used to verify the worker's outer dispatch timeout.
|
||||||
struct HangingDispatcher {
|
struct HangingDispatcher {
|
||||||
seen: AtomicUsize,
|
seen: AtomicUsize,
|
||||||
@@ -204,6 +218,58 @@ async fn shutdown_mid_dispatch_releases_lease_without_burning_attempt(pool: PgPo
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[sqlx::test(migrations = "./migrations")]
|
||||||
|
async fn browser_unavailable_releases_lease_without_burning_attempt(pool: PgPool) {
|
||||||
|
// During a browser outage the dispatcher reports BrowserUnavailable. The
|
||||||
|
// worker must return the job to `pending` with the attempt refunded
|
||||||
|
// (attempts stays 0) instead of ack-failing it toward `dead`, so the whole
|
||||||
|
// pending backlog survives the outage.
|
||||||
|
enqueue_chapter_job(&pool).await;
|
||||||
|
let dispatcher = Arc::new(BrowserUnavailableDispatcher {
|
||||||
|
seen: AtomicUsize::new(0),
|
||||||
|
});
|
||||||
|
let session_expired = Arc::new(std::sync::atomic::AtomicBool::new(false));
|
||||||
|
let cancel = CancellationToken::new();
|
||||||
|
let handle = daemon::spawn(
|
||||||
|
pool.clone(),
|
||||||
|
cancel.clone(),
|
||||||
|
make_cfg(None, dispatcher.clone(), session_expired, 1),
|
||||||
|
);
|
||||||
|
|
||||||
|
// Wait until the dispatcher has been invoked at least once (the job was
|
||||||
|
// leased and deferred).
|
||||||
|
let mut dispatched = false;
|
||||||
|
for _ in 0..40 {
|
||||||
|
if dispatcher.seen.load(Ordering::Acquire) >= 1 {
|
||||||
|
dispatched = true;
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
tokio::time::sleep(Duration::from_millis(50)).await;
|
||||||
|
}
|
||||||
|
assert!(dispatched, "dispatcher must have been invoked");
|
||||||
|
|
||||||
|
handle.shutdown().await;
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
count_state(&pool, "dead").await,
|
||||||
|
0,
|
||||||
|
"browser outage must not dead-letter the job"
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
count_state(&pool, "pending").await,
|
||||||
|
1,
|
||||||
|
"job returns to pending after a browser-unavailable outcome"
|
||||||
|
);
|
||||||
|
let attempts: i32 = sqlx::query_scalar("SELECT attempts FROM crawler_jobs")
|
||||||
|
.fetch_one(&pool)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
attempts, 0,
|
||||||
|
"browser-unavailable must refund the lease attempt (no burn)"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[sqlx::test(migrations = "./migrations")]
|
#[sqlx::test(migrations = "./migrations")]
|
||||||
async fn workers_drain_jobs_through_dispatcher(pool: PgPool) {
|
async fn workers_drain_jobs_through_dispatcher(pool: PgPool) {
|
||||||
enqueue_chapter_job(&pool).await;
|
enqueue_chapter_job(&pool).await;
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
{
|
{
|
||||||
"name": "mangalord-frontend",
|
"name": "mangalord-frontend",
|
||||||
"version": "0.124.16",
|
"version": "0.124.17",
|
||||||
"private": true,
|
"private": true,
|
||||||
"type": "module",
|
"type": "module",
|
||||||
"scripts": {
|
"scripts": {
|
||||||
|
|||||||
Reference in New Issue
Block a user