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>
51 lines
2.3 KiB
SQL
51 lines
2.3 KiB
SQL
-- Dedup analyze_page jobs in flight (mirrors 0014 for sync_chapter_content).
|
|
--
|
|
-- Without this, `repo::page_analysis::enqueue_pages`' `NOT EXISTS(... pending
|
|
-- | running ...)` read-then-insert race let concurrent admin POSTs land
|
|
-- duplicate jobs for the same page — visible as duplicate "now analyzing"
|
|
-- SSE events and inflated `pending` counters in the admin dashboard, even
|
|
-- though the worker's `skip-if-done` net (analysis/daemon.rs:210-217)
|
|
-- prevented the actual duplicate vision call.
|
|
--
|
|
-- The partial unique index lets enqueue_pages drop the NOT EXISTS subquery
|
|
-- and rely on `ON CONFLICT DO NOTHING` instead. At most one (pending|running)
|
|
-- job per page_id can exist; the slot frees the moment the job transitions
|
|
-- to done/failed/dead, so a force re-analyze still re-enqueues cleanly.
|
|
|
|
-- **Pre-dedup step.** Production runs of the prior buggy producer can
|
|
-- have left duplicate rows tied on (kind='analyze_page', payload->>'page_id',
|
|
-- state IN ('pending','running')). Adding the UNIQUE INDEX onto a dirty
|
|
-- table would fail to create and crash `sqlx::migrate!` at boot.
|
|
--
|
|
-- Demote all-but-the-lowest-id duplicates to `dead` first. Why dead and
|
|
-- not done: the dropped sibling never actually ran, and `dead` is the
|
|
-- terminal state operators inspect via the dead-jobs requeue endpoint —
|
|
-- so a curator can recover any genuinely-needed page. The lowest-id row
|
|
-- is the one a worker most likely already leased / has heartbeat'd, so
|
|
-- keeping it minimises lease-state thrash.
|
|
UPDATE crawler_jobs
|
|
SET state = 'dead',
|
|
last_error = COALESCE(last_error, '') ||
|
|
CASE WHEN last_error IS NULL OR last_error = '' THEN ''
|
|
ELSE ' | ' END ||
|
|
'pre-0031 duplicate analyze_page; superseded by earlier sibling',
|
|
updated_at = now()
|
|
WHERE id IN (
|
|
SELECT id FROM (
|
|
SELECT id,
|
|
row_number() OVER (
|
|
PARTITION BY payload->>'page_id'
|
|
ORDER BY id
|
|
) AS rn
|
|
FROM crawler_jobs
|
|
WHERE payload->>'kind' = 'analyze_page'
|
|
AND state IN ('pending', 'running')
|
|
) ranked
|
|
WHERE rn > 1
|
|
);
|
|
|
|
CREATE UNIQUE INDEX crawler_jobs_analyze_page_dedup_idx
|
|
ON crawler_jobs ((payload->>'page_id'))
|
|
WHERE state IN ('pending', 'running')
|
|
AND payload->>'kind' = 'analyze_page';
|