Rate pressure throttles the pass instead of shutting it down; Google ceiling 8.0 -> 7.0 by WilfordGrimley · Pull Request #644 · ProxyPrints/ProxyPrints.github.io

WilfordGrimley · 2026-07-30T09:31:15Z

Owner ruling, 2026-07-30:

"the limit needs to be on the amount we are fetching from google api 7/s or hardware whichever comes first. and the limit needs to throttle not shut it down"

1. The ceiling: 7/s, and where "or hardware" actually lives

harvest_fetch_limiter.GOOGLE_IMAGE.rate_per_sec drops 8.0 → 7.0. That contradicts nothing task #165's concurrency probe established — 7.0 sits strictly under the concurrency=6 step's measured 8.116/s, so it only tightens it. max_concurrency is unchanged at 6 and is deliberately not the subject: the rate limit, not the concurrency limit, is what actually protects the destination, which is why the owner specified it in requests/second.

"or hardware, whichever comes first" gets no second constant, on purpose. _DestinationLimiter is a strict minimum-interval pacer — it can only ever delay a request, never issue one, never make a slow host go faster. The achieved rate is therefore min(7.0, whatever this host and network sustain) by construction, and current_rate() (logged every 50 requests) reports which term is binding. There is a test for exactly this property.

A configured hardware-derived rate would need a mean per-fetch latency term nobody has measured at catalog scale. stage_e_batch_sizing.HostProfile (#589) gives usable cores, a memory budget and aggregate_fetch_threads — counts, not a rate. Deriving a req/s from those would be precisely the fabricated ceiling #589 removed when it deleted the false GOOGLE_IMAGE.max_concurrency term. #589's own HostProfile.fetch_overcommitted remains the honest hardware-vs-destination signal and is untouched here.

2. Throttle, don't shut down — the bar classification

This is the substance. Of the four ratified envelope bars, exactly one halted for what is really rate pressure:

bar	classification	behaviour
Host load average	genuine breach	Halts, unchanged. Fetching Google more slowly does not reduce this box's own load.
RSS per worker	genuine breach	Halts, unchanged. A leak or a pathological image; a slower cadence addresses neither.
Google fetch lockout (403)	terminal rate pressure	Halts, unchanged — deliberately. A 403 is rate pressure (429 → 403 under sustained load), but by then throttling is not the available remedy: it is an IP-level lockout on an endpoint shared with the live site's own PDF-export/bulk-download image serving. Throttling 429/503 is what makes reaching a 403 less likely.
Fetch-failure rate	mixed — converted	Narrowed. Bar, 1% ceiling and 500-card window all unchanged; a 429/503 simply no longer feeds it.

What was wrong. A Google 429 reached the dispatcher as fetch_card_image_bytes() -> None — byte-for-byte indistinguishable from a 404 or a corrupt download, because rate_limited_get returned the 429 response and the caller's own raise_for_status() flattened it into a generic exception. It was recorded on the operating envelope's rolling fetch-outcome window, and >1% of a 500-card window hard-stopped a 230,753-card unattended pass at exit 3, demanding a human resolve_envelope_trip acknowledgement for a condition whose correct answer is "go slower".

The fix. Rate pressure gets its own channel end to end: DestinationThrottledError → _StageCFetchOutcome.throttled → DispatchOutcome.stage_c_fetch_throttled. The limiter widens its pacing interval, the one card is deferred, the fetch-ahead thread carries on (unlike a lockout or a crash, which both stop it), and the pass completes at exit 0. The failure window never sees it. This is a narrowing, not a weakening — every bar still halts for everything it ever halted for, except a destination asking us to slow down.

3. Extending the existing throttle path rather than duplicating it

stream_full_catalog's pre-existing --max-throttle-retries / exit-4 budget covers throttled-concurrency-cap only — our own STAGE_E_MAX_CONCURRENT_DISPATCHES slots being full. It is denominated in dispatch attempts and has nothing to do with Google or any request rate. Spending that budget on a destination's request rate would put a hard stop straight back onto the condition the ruling says must never stop the run, so the fetch-rate throttle sits alongside it: no retry budget, no exit code of its own, cannot stop the command. Both are documented as two distinct throttles in the module docstring and the ops doc.

4. Also in this change

503 joins 429 as rate pressure for the Google destination — the shared image-cdn Worker in front of Google answers 503 under its own overload.
Backoff now decays. It was sticky for the life of the process; on a one-shot multi-hour pass one early 429 pinned the whole run at half speed with no way back. Now asymmetric: one signal doubles, 100 consecutive clean responses halve, never past the configured ceiling. Slow to recover, fast to yield.
run_image_evidence_cohort is BULK mode, which operating_envelope explicitly does not govern at all — no envelope there to spare. It catches the new exception and reports the same shape a 429 produced before: bit-for-bit unchanged behaviour for that command, and the exception can't escape through a Future and kill the fetch pool.
Default-on, no flag. Per "default the default things, disable them with flags", throttling is unconditional.

Verification

Both proof tests mutation-checked (red → green). Mutating _run_stage_c's throttle branch to fall through (if False:) made the degradation test fail; raising HOST_LOAD_CEILING to 9999.0 made the halt test fail. Both restored, both green.
- test_sustained_rate_pressure_degrades_and_completes_at_exit_zero — 100% of fetches throttled, far past the >1% bar: exit 0, zero EnvelopeTrip rows, all 3 batches dispatched, stage_c_fetch_throttled=6 / stage_c_fetch_failures=0, and the envelope window ends at (0, 0).
- test_a_genuine_envelope_breach_still_hard_stops_at_exit_three — load 9.0: exit 3, one HOST_LOAD trip, one attempt, no retry.
- test_non_throttle_fetch_failures_still_reach_the_envelope_window — the narrowing is a narrowing: a -> None fetch still feeds the window (4, 4).
Full cardpicker/tests/ suite green. pre-commit run --all-files green (ruff, isort, black, mypy, prettier, eslint). docs_lint.py --strict clean.

Docs

Per the owner documentation rule, this edits the wiki-facing docs/features/stage-e-operations.md (published as Stage-E-Operations) rather than adding a dated report: a new "Rate pressure is throttled, not halted" section with the bar-classification table, the corrected exit-code table (code 3 can no longer be reached by a 429/503; code 4 relabelled as our own dispatch-slot cap), and the two-throttles distinction in the stop-conditions list.

Interaction with #589

#589 is already merged (128492dc, 2026-07-30T09:07Z) and this branch is cut from origin/master at 80bd7b3d, which contains it — so there is no ordering hazard left to reason about. Nothing #589 removed is reintroduced: no hardware term is added to stage_e_batch_sizing, fetch_overcommitted is untouched, and the only file both changes touch is stream_full_catalog.py, in disjoint regions (#589's batch-size plumbing vs. this change's docstring and one counter).

🤖 Generated with Claude Code

https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN

…ceiling 8.0 -> 7.0 Owner ruling, 2026-07-30: "the limit needs to be on the amount we are fetching from google api 7/s or hardware whichever comes first. and the limit needs to throttle not shut it down." THE CEILING. `harvest_fetch_limiter.GOOGLE_IMAGE.rate_per_sec` drops 8.0 -> 7.0. The "or hardware, whichever comes first" half gets NO second constant, on purpose: `_DestinationLimiter` is a strict minimum-interval pacer, so it can only ever DELAY a request - the achieved rate is already `min(7.0, what this host and network sustain)` by construction, and `current_rate()` reports which term is binding. A configured hardware-derived RATE would need a mean per-fetch latency nobody has measured at catalog scale; inventing one would be exactly the fabricated ceiling PR #589 removed from `stage_e_batch_sizing`. #589's `HostProfile.fetch_overcommitted` remains the honest hardware signal and is untouched here. THROTTLE, NOT SHUTDOWN - the substantive change. Of the four ratified envelope bars, exactly one halted for what is really rate pressure: FETCH_FAILURE_RATE. A Google 429 reached the dispatcher as `fetch_card_image_bytes() -> None`, byte-for-byte indistinguishable from a 404 or a corrupt download (the limiter returned the 429 and the caller's own `raise_for_status()` flattened it), so it landed on the operating envelope's rolling fetch-outcome window - and >1% of a 500-card window HARD-STOPPED a 230,753-card unattended pass at exit 3, demanding a human `resolve_envelope_trip` acknowledgement for a condition whose correct answer is "go slower". Rate pressure now has its own channel end to end: `DestinationThrottledError` -> `_StageCFetchOutcome.throttled` -> `DispatchOutcome.stage_c_fetch_throttled`. The limiter widens its pacing interval, the one card is deferred, the fetch thread carries on, and the pass completes at exit 0. That window never sees it. The other three bars are UNCHANGED and still halt: host load and RSS because slowing fetches does not address either, and GOOGLE_LOCKOUT (403) because by then throttling is not the available remedy - it is an IP-level lockout on an endpoint shared with the live site's own image serving. Throttling 429/503 is what makes reaching a 403 less likely. Also: 503 joins 429 as rate pressure for the Google destination; backoff decays (one signal doubles, 100 consecutive clean responses halve, never past the configured ceiling) instead of being sticky for the life of the process, which on a one-shot multi-hour pass meant one early blip pinned the whole run at half speed. `run_image_evidence_cohort` is BULK mode, which the envelope does not govern at all, so it catches the new exception and reports the same shape a 429 produced there before - bit-for-bit unchanged behaviour for that command. Default-on, no flag required. Wiki-facing `docs/features/stage-e-operations.md` carries the bar classification, the exit-code table correction (3 can no longer be reached by a 429/503; 4 is our own dispatch-slot cap, not a destination rate limit) and the two-throttles distinction. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN

Owner clarification, 2026-07-30: "to be clear: the 7 fetches per second cap is a global cap, it shouldn't be per process or per core." THE GAP. `_DestinationLimiter` paces with a `threading.Lock` and a process-local `_next_allowed`, so PR #644's `rate_per_sec = 7.0` bought 7/s PER PROCESS. The pooled runner (`run_image_evidence_cohort`) is single-process and was therefore correct by accident. The conveyor is not: django-q2 workers are separate OS processes, each building its own limiter in its own address space, so N concurrent dispatches issued N x 7/s -- 14/s at the production STAGE_E_MAX_CONCURRENT_DISPATCHES = 2, scaling with the cap. Same per-process trap as threading.Semaphore(max_concurrency), which #589 removed a false ceiling term over. #644 established that the rate limit is what protects the destination; that makes its globality load-bearing. MECHANISM. One Postgres row per destination (GlobalFetchPace, migration 0102), advanced by a single atomic UPDATE ... RETURNING that lifts the existing Python pacing arithmetic into the one place every process can see. The caller then sleeps to its slot locally, holding no lock and no transaction. Concurrent reservers serialise on the row lock and, under READ COMMITTED, the blocked statement re-evaluates against the committed row -- so reservations form one strictly-increasing sequence regardless of how many processes compete. WHY NOT stage_e_concurrency's ADVISORY LOCKS (evaluated first, as asked). A lock is a binary held/not-held token: it expresses "how many at once", not "how many per unit time", because a rate needs a remembered timestamp and a lock stores no value. And that module's crash-safety objection to a DB row -- a killed worker leaves a claimed slot claimed forever -- is about a CLAIM. This row holds only a timestamp: a killed process leaves next_allowed_at at most one interval ahead, self-healing in ~143ms with zero reconciliation. The property that made a row unsafe for a slot makes it correct for a pace. No Redis exists in docker-compose.prod.yml, so Postgres is the only shared state. Timestamps come from the database's clock_timestamp(), never time.monotonic(), whose epoch is per-process -- persisting one for another process to read would reintroduce the defect while looking coordinated. The backoff multiplier and clean streak move into the same row because a global rate ceiling requires a global backoff term: processes holding different multipliers would write conflicting paces into the same row. #644's 429/503 semantics and decay schedule are unchanged; only their storage moved. max_concurrency stays per-process on purpose -- it is a local resource bound, not the destination-protecting ceiling. COST: 1.5ms mean / 2.3ms p95 per reservation over 300 calls -- 0.78% of a ~300ms image fetch, ~1% of the 143ms interval it schedules. The pooled runner is not measurably slowed. If the row is unreachable, pacing degrades to per-process and the run continues, logged at ERROR -- deliberately the opposite of stage_e_concurrency's fail-closed, since an unavailable rate budget still leaves every process individually paced. Default-on, no flag. Wiki-facing docs/features/stage-e-operations.md states the per-process-vs-global distinction explicitly, with the mechanism, the rejected alternatives and the measured cost. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN

Owner clarification, 2026-07-30: "to be clear: the 7 fetches per second cap is a global cap, it shouldn't be per process or per core". `harvest_fetch_limiter._DestinationLimiter` kept its pacing state in one process's memory, so PR #644's `rate_per_sec = 7.0` bought 7/s PER PROCESS. That is a real ceiling for the pooled runner (one process, one thread pool) and no ceiling at all for the conveyor: django-q2's workers are separate OS processes, so N concurrent dispatches fetched at N x 7/s - 14/s at the shipped STAGE_E_MAX_CONCURRENT_DISPATCHES = 2, rising with that cap. New `cardpicker/harvest_rate_coordinator.py` moves the pacer's own `max(now, next_allowed) + interval` arithmetic into a single atomic `INSERT ... ON CONFLICT DO UPDATE` over one cursor row every fetching process shares. One round trip per fetch, Postgres's own clock, no advisory lock, no explicit transaction. No new migration: the cursor lives in the existing `shared_cache` table (0092), under a key Django's own `make_key` cannot produce. The migration graph is contended and a one-row table is not worth a leaf. Losing the row is benign - the next reservation re-inserts at "now". Degradation is divided, not open and not closed: if Postgres is unreachable, each process falls back to its own pacer at rate / (STAGE_E_MAX_CONCURRENT_DISPATCHES + 1), so the aggregate ceiling still holds with every process fetching. Failing open would restore the exact defect; failing closed would halt an unattended 230,753-card pass over a blip. A degraded reservation waits, never raises, never trips. Purely additive to #644: the throttle-not-halt conversion, the 429/503 classification, the backoff decay and the envelope bar classification are untouched. Backoff stays per-process and reaches the shared cursor as a widened interval, which can only slow the aggregate. Proven with real forked OS processes, not one limiter: three processes at a 20/s ceiling deliver 20/s together. Against the pre-fix pacer the same test measures 63/s. Coordination costs 1.1-1.3 ms per reservation (0.8-0.9% of the 143 ms interval a 7/s ceiling already imposes); eight acquisitions at the real 7.0/s ceiling take 1.003s against 1.000s for the ceiling alone, +0.3%. Claude-Session: https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

…ess + census leak, mid-pass envelope re-sampling (#665) FIX 1 - THE MD5 GROUP BEHAVES AS ONE UNIT THROUGH THE MONOLITH Owner: "the md5 dedupe should only fetch each identical image once across sources and then apply votes to the entire group as the fetched card passes through the monolith." The fetch half already existed (`evidence_transfer`, keyed on `Card.md5_checksum`). The vote half existed only on the phash distance-0 key, so the set that got a fetch saved and the set that got a vote propagated were DIFFERENT SETS - byte-identical files always share a phash, but files sharing a phash are not necessarily byte-identical. CHECKED BEFORE BUILDING, as instructed: propagation is NOT redundant. `evidence_transfer` gives every md5 sibling its own ImageEvidence row with byte-identical extractor values, so it is reasonable to ask whether each member already reaches the same conclusion independently. It does not, structurally: a Stage D printing deduction is not a function of the evidence row alone. `_resolve_candidates_for_card` keys the candidate list on `Card.name`, and md5-identical uploads from different sources routinely carry different names. Members also differ on per-card eligibility. `test_the_unfetched_twin_has_no_verdict_of_its_own_without_propagation` falsifies the "N independent deductions already agree" hypothesis on the fixture rather than arguing it. Stage C+ now runs TWO tiers through ONE propagation engine (`_propagate_over_groups`), which takes the grouping as a parameter and knows nothing about how it was keyed: md5 exact identity -> shares a PRINTING vote. New. Runs first. phash d0 unchanged from PR #660. Runs second, filling only what md5 did not. Two defects in the PR #660 propagation are fixed on the way: - SOURCE VOTES WERE READ FROM REPRESENTATIVES ONLY. Stage D has no reason to reach a group's lowest pk first, so whenever it reached any other member, nothing propagated. Source votes are now read across every member, and one source per (group, identity) is chosen deterministically so two vote-holders in one group cannot generate duplicate rows inside a single write batch. - REPRESENTATIVES WERE NEVER PROPAGATION TARGETS. Same root cause, opposite direction. PROPAGATION NEVER OVERRIDES A MEMBER'S OWN INELIGIBILITY (owner constraint). A member already resolved, already confirmed to a `canonical_card`, not a CARD, or carrying a resolved custom-art/non-english tag is skipped. custom-art is the catalogue DECLARING the image is not a faithful depiction of a printing; a checksum must not overturn that. THE PHASH TIER IS LEFT IN PLACE, flagged rather than accreted. Issue #661 holds what phash grouping is FOR - the owner's direction is that it should eventually share an ILLUSTRATION (same artwork, possibly a different printing), not a printing verdict. Removing it now would itself be a behaviour change, and it is currently the only propagation reaching cards with no md5 at all (md5 is NULL for every LOCAL_FILE source by design). The `groups` parameter is the seam that tier plugs into later. FIX 2a - `run_name_frequency_elimination` NEVER LOOKED AT THE IMAGE Owner: "just because a card was printed exactly once doesn't mean that the image in our catalogue is an accurate depiction of that card, it may have a different border or another issue." Owner leaned toward adding the conjunct rather than dropping the tier; the conjunct is what shipped, and the reasoning for keeping the tier is in its own docstring. Everything the 1:1 gate checked was a COUNT. Counting establishes that IF the card depicts one of the name's printings THEN it is the uncovered one; nothing established the antecedent, and the only filters that spoke to it were the DECLARED custom-art/non-english tags - so an untagged altered border sailed through. "It is only a vote" is weaker than it sounds: #593 established a machine vote is what the question feed renders as the suggestion to confirm, and the human's click returns as a full-weight USER vote. The missing conjunct now requires the card's ALREADY-STORED evidence to be consistent with the candidate printing. NOT a new implementation: `_apply_agreement_checks`' border/frame check was lifted to `local_identify_printing_tags.printing_attribute_disagreement` and both callers now share it. That direction is forced - `local_calculate_verdicts` imports `local_identify_printing_tags`, never the reverse. Sharing also inherits PR #656's `artist_ocr` gate for free, which is the half a second copy would most likely have got wrong. NO STORED EVIDENCE MEANS ABSTAIN. This module's "missing data is not evidence" rule protects a match from being VETOED by silence; here silence is being asked to ESTABLISH something, so it points the other way. Counted separately from mismatches so the cost is legible. FIX 2b - THE CENSUS LEAK (a fresh wrong positive, not a stale vote) `_eligible_base_queryset(NAME_FREQUENCY_ANONYMOUS_ID)` was called with no `run_id`, making its "exclude cards already carrying this calculator's vote" LIFETIME. The gate is a COUNT over exactly that population, so the calculator was taking a census over a pool it permanently shrinks itself: run 1 votes on a card, a second upload of that name arrives, and run 2 sees one unresolved card where there are really two - and votes. Nothing about the second card changed; only the size of the population the gate counts. `compute_covered_printing_pks()` stays catalogue-wide and unscoped, deliberately: "covered" is a fact about the world, not about this calculator's progress. `run_pilot`'s `select_candidates` and `count_below_resolution_floor` are LEFT UNSCOPED - neither gates on a count over the returned population, so neither has this defect. Stated in `_eligible_base_queryset`'s docstring so the asymmetry is visible from the function rather than only from its callers. FIX 3 - THE MONOLITH RE-SAMPLES THE ENVELOPE MID-PASS Owner: "host resampling is likely required (for steps that aren't fetch) as the same monolith will run for small datasets and large ones so needs to fit the available compute appropriately." PR #660 checked the envelope ONCE, before Stage C. `_EnvelopeSentry` now re-samples at every stage seam: after Stage C, between each of Stage D's calculators/casters (via a new OPTIONAL `envelope_check` callback on `stage_e_dispatch._run_stage_d`, defaulting to None so the conveyor is byte-identical), and before each Stage C+ tier. Sample counts land on the ledger. HALT SEMANTICS PRESERVED. A breach still persists an EnvelopeTrip, still exits 3, still needs `resolve_envelope_trip` - no self-resume, and NOT converted to a throttle (that is rate pressure's channel, beneath Stage C, PR #644). A mid-pass halt message differs from the preflight's: rows already written STAY written, and it names the `--run-id` to resume with. Interval-gated at 60s so the check cannot become its own load. The number is derived, not tuned: the host-load bar reads the ONE-MINUTE load average, so sampling faster re-reads a number that has not finished moving. RESIDUAL, reported not hidden: the seams are BETWEEN calculators, not inside them. Closing that gap means threading a callback into each of seven calculators' own batch loops - a real refactor of shared code, deliberately not done here. DELIBERATE DUPLICATION, WITH A TRIPWIRE. `_members_eligible_for_a_propagated_vote` expresses four catalogue-level facts `_eligible_base_queryset` also expresses. It does not call that function (which bundles workload rules wrong for a propagation target) and that function could not be refactored to expose them (its own docstring records that tests and `stream_backstop_sweep` assert against its COMPILED SQL). `TestPropagationEligibilityMatchesTheBaseQueryset` fails if the two ever disagree. VERIFICATION - mutation red, restore green (7 mutants, all red; 263 tests green restored): M1 md5 tier returns no groups 3 failed M2 source votes read from representatives only 2 failed M3 propagation ignores member ineligibility 1 failed M4 envelope re-sampling reverted to preflight 2 failed M5 visual conjunct never disagrees 1 failed M6 no-evidence no longer abstains 1 failed M7 propagation eligibility drops the tag excludes 1 failed (the tripwire) Suites: test_run_pipeline, test_local_identify_printing_tags, test_local_calculate_verdicts, test_stage_e_dispatch - 520+ tests, all green. No model changes, so no migration. Docs: living pages only, no dated report - docs/identification-pipeline.md (Stage C+ md5 section), docs/features/printing-tags.md (both name-frequency fixes), docs/features/stage-e-operations.md (mid-pass re-sampling). Claude-Session: https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

github-actions Bot assigned WilfordGrimley Jul 30, 2026

WilfordGrimley merged commit 848c81b into master Jul 30, 2026
14 checks passed

WilfordGrimley mentioned this pull request Jul 30, 2026

Make the 7/s Google fetch cap global, not per process #648

Closed

WilfordGrimley mentioned this pull request Jul 30, 2026

The md5 group as one unit; name-frequency's missing conjunct and its census leak; the monolith re-samples the envelope #665

Merged

WilfordGrimley mentioned this pull request Aug 3, 2026

harvest_rate_coordinator: backoff stays per-process, so a 429 slows the fleet by only 1/N #671

Open

Navigation Menu

Uh oh!

Rate pressure throttles the pass instead of shutting it down; Google ceiling 8.0 -> 7.0 - #644

Rate pressure throttles the pass instead of shutting it down; Google ceiling 8.0 -> 7.0#644
WilfordGrimley merged 1 commit into
masterfrom
feat/fetch-rate-throttle

WilfordGrimley commented Jul 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

WilfordGrimley commented Jul 30, 2026

1. The ceiling: 7/s, and where "or hardware" actually lives

2. Throttle, don't shut down — the bar classification

3. Extending the existing throttle path rather than duplicating it

4. Also in this change

Verification

Docs

Interaction with #589

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant