Add Stage E streaming dispatch concurrency cap - #450
Merged
Conversation
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>
… 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>
4 tasks
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Replaces #449, which was stacked on #448's now-merged (squashed) branch
and could not be cleanly rebased onto
masterwithout hand-resolving thelock-context re-indentation of
dispatch_micro_batchagainst the squashedalready_voted/ignore_conflictsbody - rebased and force-push was ruledout per this repo's own no-force-push rule, so this is a fresh branch/PR
carrying the same content plus one added commit (integration tests against
the real, merged
dispatch_micro_batchbody). See #449 for the fulloriginal review history/discussion.
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, was #448's own fix, now onmaster):eight concurrent
dispatch_micro_batchcalls, all running CPU-boundOCR/phash extraction at once, tripped the envelope's host-load bar
(
envtrip-20260724T214616-be6e5db9, observed load11.85against the7.0ceiling) on a host with only 7 usable compute cores(
docs/features/catalog-completion-plan.mdL1794/2248/2366's hardwarecitation and its own 0.31x-slower-than-sequential CPU-bound-
oversubscription finding).
The envelope's own host-load bar is REACTIVE -
check_envelopeonly tripsafter 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_batchnow refusesto even start once
settings.STAGE_E_MAX_CONCURRENT_DISPATCHES(default2, env-tunable) dispatches are already running concurrently, anywhereacross 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 acache-based counter or a dedicated low-worker django-q queue:
Django's default, per-PROCESS
LocMemCache(noCACHESoverride insettings.py) and django-q2's 8 workers are separate OS processes
(multiprocessing) - a cache-based counter would silently fail to
coordinate across them.
a second
Clusterprocess needs its own supervisor/deployment wiring, amuch larger blast radius than a primitive enforced inside
dispatch_micro_batchitself.for CRASH SAFETY: a
kill -9'd worker would leave a row-based counter'sslot 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
DispatchOutcomestatus"throttled-concurrency-cap"- writes noPilotRunLedgerrow, matching the other halted statuses' own convention.Checklist
pre-commitand installed the hooks withpre-commit installbefore creating any commits.pytest cardpicker/tests/test_stage_e_concurrency.py cardpicker/tests/test_stage_e_dispatch.py cardpicker/tests/test_local_calculate_verdicts.py(149 passed) via the hostmpcautofill-pilotvenv against the ephemeral testcontainer Postgres (never the live prod stack), run AFTER rebasing this branch onto currentmaster(which now contains Fix Stage E concurrent-dispatch vote-collision IntegrityError #448's squashed content) to confirm the cap still triggers correctly against the real, mergeddispatch_micro_batchbody - not just the pre-merge module-level unit tests.psycopg2connection 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), a real-OS-thread contention test with a shared "max simultaneously held" counter, and (new in this branch) a full-dispatch_micro_batch-level integration test confirmingstatus="throttled-concurrency-cap"with zero ledger/evidence/vote writes when every slot is externally held, and normal completion once released.black --check,ruff check,isort --check,mypyall clean on every touched file;docs_lint.pyclean (including the mechanical CLEAN-row tether for the new extractable-primitives.md row).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 forcardpicker.stage_e_concurrency(zero fork-only imports - a genuinely generic Postgres-advisory-lock concurrency primitive).