Skip to content

Fix Stage E concurrent-dispatch vote-collision IntegrityError - #448

Merged
WilfordGrimley merged 2 commits into
masterfrom
worktree-agent-aed7d968a5fd22516
Jul 24, 2026
Merged

Fix Stage E concurrent-dispatch vote-collision IntegrityError#448
WilfordGrimley merged 2 commits into
masterfrom
worktree-agent-aed7d968a5fd22516

Conversation

@WilfordGrimley

@WilfordGrimley WilfordGrimley commented Jul 24, 2026

Copy link
Copy Markdown

Description

Fixes the vote-collision half of the first shakedown-sweep incident: two
CONCURRENT dispatch_micro_batch invocations (django-q2's
Q_CLUSTER["workers"] = 8, or the cron backstop sweep overlapping an event
trigger) can both pass run_join_key_calculator's or
run_fallback_calculator's own per-identity eligibility check before either
commits, then race to bulk_create the same (card, anonymous_id)
CardPrintingTag - the loser hit IntegrityError on
cardprintingtag_unique_no_match_vote/cardprintingtag_unique_printing_vote
and aborted its whole micro-batch. The shakedown run produced exactly
seven failed PilotRunLedger rows (run_ids stage-e-stream-20260724T2144*)
plus the one dispatch that won the race - seven losers + one winner = eight
total concurrent dispatches, exactly Q_CLUSTER["workers"] = 8.

A second, separate failure the same run hit is explicitly NOT fixed by
this PR
: envtrip-20260724T214616-be6e5db9 is a bar=host_load envelope
trip (observed load 11.85 against the 7.0 ceiling, tripped 0.43s after
the winning vote landed), caused by those same eight concurrent dispatches
saturating this host's 7 usable cores running OCR/phash extraction at once
(docs/features/catalog-completion-plan.md L1794/2248/2366's hardware
citation and 0.31x concurrency finding) - a resource-contention problem,
not a vote-write correctness one. Fixing the vote collision does nothing to
stop eight dispatches from re-tripping the load bar the moment streaming
resumes; that's the companion PR's job (a
settings.STAGE_E_MAX_CONCURRENT_DISPATCHES concurrency cap), tracked
separately per Tron's review of this PR.

Two lines of defense against the vote collision, both required:

  1. A pre-write skip-if-exists guard
    (local_calculate_verdicts._split_new_printing_tag_votes, mirroring PR
    Fix local_lands_identify write-path vote collision and dry-run yield display #411's own precedent) - skip-and-count, not retract-and-recast, GIVEN
    both racing reads see the same evidence under the same code version and
    lexicon (reparse_collector_evidence remains the correct remedy if that
    assumption doesn't hold, e.g. a mid-race deploy or re-extraction).
  2. bulk_create(..., ignore_conflicts=True) on both guarded call sites -
    the pre-write check alone is check-then-insert, not atomic, and leaves a
    narrow residual query-to-insert race window; ignore_conflicts=True is
    the actual crash-proofing, matching this codebase's own established
    precedent for the identical shape (local_layout_class_cast.py:300,
    local_detect_ai_art.py:459, local_identify_printing_tags.py:1246).

run_slow_path_calculator was checked for the same hazard and needs no
guard - it writes only CardScanLog rows, which carry no DB uniqueness
constraint at all (append-only by design).

Also corrects docs/features/stage-e-operations.md's "Resume contract"
section (previously overstated the eligibility-exclude alone as sufficient
for idempotence - true only for a sequential re-invocation, not a
concurrent one), and adds a runbook line: do not run a BULK-mode write
command while PASSIVE streaming is enabled, since BULK mode is entirely
outside the envelope's own bars and outside any per-worker concurrency cap.

Checklist

  • I have installed pre-commit and installed the hooks with pre-commit install before creating any commits.
  • I have updated any related tests for code I modified or added new tests where appropriate.
  • I have manually tested my changes as follows:
    • pytest cardpicker/tests/test_local_calculate_verdicts.py cardpicker/tests/test_stage_e_dispatch.py (137 passed), plus the neighboring test_local_lands_identify.py/test_reparse_collector_evidence.py/test_command_local_lands_identify.py (89 passed) and a broader sweep of every file importing local_calculate_verdicts/stage_e_dispatch (474 passed) - all via the host mpcautofill-pilot venv against ephemeral testcontainer Postgres/ES (docs/troubleshooting.md's documented convention), never the live prod stack.
    • black --check, ruff check, isort --check, mypy all clean on every touched file; docs_lint.py clean.
    • New regression coverage: a direct unit test class for the guard (TestSplitNewPrintingTagVotes), an end-to-end "seed the winner's vote, confirm the loser survives it" test in both TestRunJoinKeyCalculator/TestRunFallbackCalculator, a full-conveyor version in test_stage_e_dispatch.py's TestConcurrentDispatchVoteCollision, and (added in the Tron-gate correction round) a test that defeats the pre-write check itself to prove ignore_conflicts=True alone survives the residual race window.
  • I have updated any relevant documentation or created new documentation where appropriate.
    • docs/features/stage-e-operations.md's "Resume contract, extended to a streamed micro-batch" section (idempotence correction, the two-separate-failures distinction, the BULK-mode-vs-PASSIVE-streaming runbook line) and its Observability counter list.

Tron gate history

Round 1 (CONFIRMED with required corrections): incident count was wrong
(said four failed rows, actually seven + one winner = eight, matching
Q_CLUSTER["workers"]); the docs conflated the vote-collision failure with
the separate host-load envelope trip; the "never a crash" claim overstated
a check-then-insert guard's actual guarantee; the "necessarily same verdict"
premise needed to be stated as contingent, not absolute. All four addressed
in the second commit on this branch, plus a new regression test proving the
ignore_conflicts=True belt-and-suspenders survives even when the
pre-write check itself is defeated. The companion concurrency-cap change
(settings.STAGE_E_MAX_CONCURRENT_DISPATCHES) ships as a separate PR per
Tron's own instruction to keep it decoupled from this correctness fix.

Two concurrent dispatch_micro_batch invocations (django-q2's 8 workers,
or the backstop sweep racing an event trigger) could both pass Stage D's
per-identity eligibility check before either committed, then race to
bulk_create the same (card, anonymous_id) CardPrintingTag - the loser hit
IntegrityError and aborted its whole micro-batch (trip
envtrip-20260724T214616-be6e5db9, failed run_ids stage-e-stream-20260724T2144*).

Adds a pre-write skip-if-exists guard (_split_new_printing_tag_votes,
mirroring PR #411's precedent) to run_join_key_calculator and
run_fallback_calculator - skip-and-count, not retract-and-recast, since a
concurrent race yields the same verdict from the same evidence, not a
genuine conclusion change. run_slow_path_calculator needs no equivalent
guard (CardScanLog carries no DB uniqueness constraint). Corrects
stage-e-operations.md's overstated "eligibility exclude alone is
idempotent" claim for the concurrent (not just sequential) case.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n gate)

Tron gate on PR #448: seven failed run_ids + one winner (not four) =
Q_CLUSTER workers=8; separates the vote-collision failure (this guard)
from the SEPARATE envtrip-20260724T214616 host-load trip (11.85 vs 7.0,
8 concurrent OCR dispatches on 7 cores, not fixed by this change);
qualifies the "same verdict" premise as contingent on unchanged code/
evidence/lexicon, naming reparse_collector_evidence as the remedy
otherwise; adds bulk_create(..., ignore_conflicts=True) as the actual
crash-proofing against the guard's own residual check-then-insert race
window (precedent: local_layout_class_cast.py:300,
local_detect_ai_art.py:459, local_identify_printing_tags.py:1246), with
a regression test defeating the pre-write check to prove it; adds an
ops-doc runbook line against running BULK-mode writes while PASSIVE
streaming is enabled.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@WilfordGrimley
WilfordGrimley merged commit 41181e4 into master Jul 24, 2026
9 checks passed
WilfordGrimley added a commit that referenced this pull request Jul 24, 2026
… body

Verifies dispatch_micro_batch itself (post-#448 merge) returns
throttled-concurrency-cap and does zero work when every slot is held by
a genuinely independent connection, and proceeds normally once released.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
WilfordGrimley added a commit that referenced this pull request Jul 25, 2026
* Add Stage E streaming dispatch concurrency cap

Companion to PR #448's vote-collision fix (Tron gate round 1, COMPANION
item), kept as a separate PR per that gate's own instruction. The
shakedown's first live run had eight concurrent dispatch_micro_batch
calls - all running CPU-bound OCR/phash extraction at once - trip the
envelope's host-load bar (11.85 vs 7.0 ceiling) on a host with only 7
usable compute cores. The envelope only trips REACTIVELY, after load
has already spiked; this cap is PROACTIVE, refusing to even start a
dispatch once settings.STAGE_E_MAX_CONCURRENT_DISPATCHES (default 2)
concurrent dispatches are already running.

Mechanism: Postgres session-scoped advisory locks
(cardpicker.stage_e_concurrency), not a cache-based counter (this app's
cache is per-process LocMemCache, useless across django-q2's 8 separate
worker processes) or a dedicated low-worker queue (disproportionate
infra for a conservative cap). Chosen over a DB-row counter specifically
for crash safety: a killed process's session-scoped lock auto-releases,
no reconciliation code needed - no new migration either.

New DispatchOutcome status "throttled-concurrency-cap" (no ledger row,
matching the other halted statuses). Tests include genuine cross-session
races (a raw psycopg2 connection standing in for a second django-q
worker process) and a real-thread contention test with a shared
max-concurrently-held counter, not just sequential calls.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Add integration coverage for the cap against the real merged dispatch body

Verifies dispatch_micro_batch itself (post-#448 merge) returns
throttled-concurrency-cap and does zero work when every slot is held by
a genuinely independent connection, and proceeds normally once released.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Stop backstop sweep on throttled-concurrency-cap; floor slot count at 1

* Fix docs_lint path: MPCAutofill/MPCAutofill/settings.py (doubled dir)

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant