Skip to content

Add Stage E streaming dispatch concurrency cap - #449

Closed
WilfordGrimley wants to merge 3 commits into
masterfrom
stage-e-concurrency-cap-companion
Closed

Add Stage E streaming dispatch concurrency cap#449
WilfordGrimley wants to merge 3 commits into
masterfrom
stage-e-concurrency-cap-companion

Conversation

@WilfordGrimley

Copy link
Copy Markdown

Description

Stacked on #448 (base branch is worktree-agent-aed7d968a5fd22516, PR
#448's own branch) - kept as a genuinely separate PR per Tron's own
instruction on #448's review, but the code depends on fields/wiring #448
introduces (stage_d_join_key_already_voted/_fallback_already_voted,
the try block shape around Stage C/D), so it is sequenced to merge
after #448, not independently. Please merge #448 first, then retarget/
rebase this PR onto master before merging it
- do not delete #448's
branch until this PR (a stacked child) has been retargeted, per this
repo's own merge-duty discipline.

Adds a concurrency cap for Stage E's streaming dispatch loop, addressing
the SECOND, separate failure the 2026-07-24 shakedown run hit (the first,
the vote-collision IntegrityError, is #448's own fix): eight concurrent
dispatch_micro_batch calls, all running CPU-bound OCR/phash extraction
at once, tripped the envelope's host-load bar
(envtrip-20260724T214616-be6e5db9, observed load 11.85 against the
7.0 ceiling) on a host with only 7 usable compute cores
(docs/features/catalog-completion-plan.md L1794/2248/2366's hardware
citation and its own 0.31x-slower-than-sequential CPU-bound-
oversubscription finding).

The envelope's own host-load bar is REACTIVE - check_envelope only trips
after a fresh signal sample crosses 7.0, necessarily after load has
already spiked (the incident's own trip landed 0.43s after the vote had
already landed). This cap is PROACTIVE: dispatch_micro_batch now refuses
to even start once settings.STAGE_E_MAX_CONCURRENT_DISPATCHES (default
2, env-tunable) dispatches are already running concurrently, anywhere
across this box's django-q2 worker processes - the host is never driven
past a bounded concurrency level by Stage E's own dispatches in the first
place. Both mechanisms stay in place; neither supersedes the other.

Mechanism - Postgres session-scoped advisory locks
(cardpicker.stage_e_concurrency, new module, no migration), not a
cache-based counter or a dedicated low-worker django-q queue:

  • A cache-based counter was rejected: this app's cache backend is
    Django's default, per-PROCESS LocMemCache (no CACHES override in
    settings.py) and django-q2's 8 workers are separate OS processes
    (multiprocessing) - a cache-based counter would silently fail to
    coordinate across them.
  • A dedicated low-worker django-q queue was rejected as disproportionate -
    a second Cluster process needs its own supervisor/deployment wiring, a
    much larger blast radius than a primitive enforced inside
    dispatch_micro_batch itself.
  • A DB-row-based atomic counter was considered and rejected specifically
    for CRASH SAFETY: a kill -9'd worker would leave a row-based counter's
    slot permanently "claimed" with no reconciliation mechanism, unlike a
    Postgres session-scoped advisory lock, which Postgres auto-releases the
    instant the holding connection dies - matching this pipeline's own
    "truthful ledger, idempotent re-entry, zero manual cleanup" ethos
    (scripts/ops/crash_drill.sh, TestKillSafetyResumeContract) for free.

New DispatchOutcome status "throttled-concurrency-cap" - writes no
PilotRunLedger row, matching the other halted statuses' own convention.

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_stage_e_concurrency.py cardpicker/tests/test_stage_e_dispatch.py (31 passed) via the host mpcautofill-pilot venv against the ephemeral testcontainer Postgres (never the live prod stack) - includes genuine cross-session race tests (a raw psycopg2 connection standing in for a second django-q worker process, matching production's real "each dispatch = its own connection" shape - a discovered, load-bearing subtlety: Postgres session-level advisory locks are RE-ENTRANT within one session, so simulating "two dispatchers" by calling the acquire function twice on the SAME connection silently re-acquires the same slot instead of testing anything real; every multi-dispatcher test here uses a genuinely independent connection per simulated dispatcher) and a real-OS-thread contention test with a shared "max simultaneously held" counter, not just sequential calls made to look concurrent.
    • black --check, ruff check, isort --check, mypy all clean on every touched file; docs_lint.py clean (including the mechanical CLEAN-row tether for the new extractable-primitives.md row).
  • I have updated any relevant documentation or created new documentation where appropriate.
    • docs/features/stage-e-operations.md: new "Concurrency cap" subsection, renumbered dispatch-ordering steps, and the reactive-vs-proactive framing against the envelope.
    • docs/upstreaming/extractable-primitives.md: new CLEAN row for cardpicker.stage_e_concurrency (zero fork-only imports - a genuinely generic Postgres-advisory-lock concurrency primitive).

WilfordGrimley and others added 3 commits July 24, 2026 22:07
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>
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>
@WilfordGrimley
WilfordGrimley changed the base branch from worktree-agent-aed7d968a5fd22516 to master July 24, 2026 23:00
@WilfordGrimley

Copy link
Copy Markdown
Author

Superseded by #450: #448 merged (squash) and this branch (stacked on #448's now-gone branch) could not be cleanly rebased onto master without hand-resolving a conflict, and force-push is against this repo's own standing rule. #450 carries the same content, rebased onto current master, plus one added commit with integration tests verifying the cap against the real merged dispatch_micro_batch body.

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