diff --git a/.gitignore b/.gitignore index ec55ede..b19b0f1 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ .claude .gga .atl/ +.opencode/ # Personal documentation (Obsidian) — not committed to git .doc/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 133b558..b14f165 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- E2E validation of `player_info` buffered dispatch and warm-pool execution paths complete (Phases A0–J3): direct mode, buffered mode, and buffered+warm-pool mode validated against an isolated smoke environment +- Architecture decision: buffered dispatch model generalized as a shared always-active dispatch runtime; `player_info` is the reference implementation for future workloads +- `docs/architecture/buffered-dispatch-engine.md`: architecture decision record for the buffered dispatch engine + ## [0.26.0] — 2026-07-30 ### Changed diff --git a/docs/architecture/buffered-dispatch-engine.md b/docs/architecture/buffered-dispatch-engine.md new file mode 100644 index 0000000..89b1a0e --- /dev/null +++ b/docs/architecture/buffered-dispatch-engine.md @@ -0,0 +1,107 @@ +# Buffered Dispatch Engine — Architecture Decision + +**Status:** Decided +**Date:** 2026-08-09 +**Validated by:** E2E smoke run, Phases A0–J3 + +--- + +## Decision summary + +The buffering and warm-pool execution model validated through `player_info` (PR #142) must +be treated as a **shared, always-active dispatch runtime**, not as a `player_info`-specific +feature and not as a per-scraper duplicated subsystem. + +A single dispatch engine runs at the top of the scraping runtime. Different workloads +attach to it via per-workload policy objects. The engine itself is workload-agnostic; +the policy object carries all workload-specific behavior. + +This eliminates the risk of N independent buffering implementations drifting apart and +prevents the accidental growth of parallel buffering stacks as new scraping domains are added. + +--- + +## Validation evidence + +| Phase | What was validated | +|---|---| +| D3 | Direct mode baseline: workers=1, no buffer, no warm pool. Job reaches DONE via the standard claim→process→write path. | +| E4 | Buffered mode: workers=2, buffer=ON, warm pool=OFF. `BoundedCandidateBuffer` drains correctly; no starvation, no deadlock. | +| F3 | Buffered+warm-pool: workers=2, buffer=ON, warm pool=ON. `WarmBrowserPool` and `WorkerSlot` coordinate without contention. | +| G0–G7 | Fake-worker scalability characterization at 5, 10, 25, and 50 concurrent workers. Claim throughput measured; no queue corruption observed. | +| H1–H4 + H-REPAIR | Controlled SIGKILL interruption (full process group via `os.killpg`). `recover_all_stale()` correctly resets IN_PROGRESS and PENDING rows. Jobs restart cleanly on the next run. | +| I1 | Real-source soak: 3 candidates scraped from FBRef in direct mode with workers=2. All 3 jobs completed DONE with real data written to `tbl_player_info`. | + +--- + +## Target architecture + +One shared dispatch engine instance per scraping runtime process. Each workload registers +a **policy object** that carries all workload-specific configuration: + +``` +WorkloadPolicy: + job_type # scrape_queue job_type discriminator + candidate_source # query or repository method that produces candidate IDs + claim_strategy # how rows are claimed (SELECT FOR UPDATE SKIP LOCKED, etc.) + processor_adapter # callable that maps a candidate ID to a scrape execution + concurrency_limit # max simultaneous in-flight jobs for this workload + buffer_size # BoundedCandidateBuffer capacity + pool_size # WarmBrowserPool slot count + retry_backoff # BackoffPolicy instance for claim and warmup failures + recovery_contract # which stale states to reset at startup (IN_PROGRESS, ACTIVE) + observability_labels # label set for metrics and structured log fields +``` + +The engine owns the event loop plumbing: drain scheduling, buffer fill, slot assignment, +`on_warmup_success` / `on_engine_teardown` callbacks, and backpressure signaling. The +policy object owns what is scraped and how the result is persisted. + +--- + +## Reference implementation + +`player_info` is the **reference implementation** for all future workloads. The primitives +it introduced are the canonical building blocks: + +| Primitive | Role | +|---|---| +| `BoundedCandidateBuffer` | Bounded async queue between drain and workers. Prevents thundering-herd at claim time. | +| `CandidateProducer` | Drain loop that fills the buffer from the database on a polling cadence. | +| `WarmBrowserPool` | Pre-warmed browser slot pool. Eliminates per-job cold-start latency. | +| `WorkerSlot` | Unit of concurrency. Owns one browser instance and one in-flight job at a time. | +| `RateLimitGate` | Admission gate that enforces inter-request delays and domain-level rate limits. | + +When adding a new scraping domain, implement the `WorkloadPolicy` interface and wire it +into the shared engine. Do not copy the buffer or pool logic into the new domain module. + +--- + +## Critical caveat — G-phase scalability does not apply to real FBRef concurrency + +The G-phase scalability characterization (5/10/25/50 workers) used **local fake workers**: +asyncpg claim followed by an immediate DONE write, with no real browser and no real HTTP +request. These results characterize claim throughput and queue mechanics only. + +They do **not** imply that 25 or 50 concurrent real browser sessions against FBRef are +safe or sustainable. FBRef applies Cloudflare protection, per-IP rate limits, and +Turnstile challenges. The maximum safe real-browser concurrency for FBRef requires a +separate, dedicated validation run with real browsers and real HTTP traffic under +controlled observation. + +The current validated ceiling for real FBRef scraping is **workers=2** (from Phase I). +Do not exceed this without dedicated concurrency validation. + +--- + +## Rollout guidance + +- Feature flags (`PLAYER_INFO_USE_BUFFER`, `PLAYER_INFO_USE_WARM_POOL`) remain `false` by + default. Direct mode is the stable production path until the shared engine is promoted. +- Each new workload must be validated against an isolated smoke database (Gate A pattern) + before enabling any buffering or warm-pool flag in a production environment. +- `player_info` direct mode remains the stable fallback. Any rollback of buffered or + warm-pool mode requires no code change and no migration — only a flag change and process + restart. +- `recover_all_stale()` runs unconditionally at startup for all workloads. Any IN_PROGRESS + or stale rows from a previous interrupted run are reset to PENDING before the first claim. diff --git a/docs/operations/player_info_buffered_warm_pool_smoke.md b/docs/operations/player_info_buffered_warm_pool_smoke.md index 9911d21..3a80f70 100644 --- a/docs/operations/player_info_buffered_warm_pool_smoke.md +++ b/docs/operations/player_info_buffered_warm_pool_smoke.md @@ -663,12 +663,88 @@ using a documented recovery operation (`recover_all_stale`, `recover_failed`). ## Live validation status -**No live scraping was run to produce this document.** +**Validation complete. All phases PASS.** -Stage 0 (static verification) is the only stage completable without a live database. -Stages 1–4 require a local and disposable PostgreSQL instance with a bounded candidate -set. The operator must execute each stage and record their own evidence before proceeding -to the next. +Live end-to-end validation was executed against an isolated PostgreSQL smoke environment +(Docker container, `127.0.0.1:15432`, run_id=`gate-a-20260809T171510Z-9364-38542daffed250a6`). +No production database was touched at any point during the validation run. -This runbook does not claim Stage 1, 2, 3, or 4 success. Evidence must be captured -and reviewed by the operator before advancing. +### Checkpoint summary + +| Checkpoint | Phases | Result | +|---|---|---| +| CHECKPOINT 1 — execution modes | D3 (direct) + E4 (buffered) + F3 (buffered+warm-pool) | ✅ PASS | +| CHECKPOINT 2 — scalability characterization | G0–G7 (fake workers, 5/10/25/50 concurrency) | ✅ PASS | +| CHECKPOINT 3 — interruption and recovery | H0–H4 + H-REPAIR (SIGKILL + recover_all_stale) | ✅ PASS | + +### Phase summary + +| Phase range | Description | Result | +|---|---|---| +| A0–A3 | Gate A: isolated PostgreSQL container, schema verified | PASS | +| B0–B3 | Alembic migrations applied to smoke DB | PASS | +| C0–C3 | Controlled candidate seeded (Lionel Messi, player_id=`d70ce98e`) | PASS | +| D0–D3 | Direct mode: workers=1, buffer=OFF, warm_pool=OFF | PASS | +| E0–E4 | Buffered mode: workers=2, buffer=ON, warm_pool=OFF | PASS | +| F0–F3 | Buffered+warm-pool: workers=2, buffer=ON, warm_pool=ON | PASS | +| G0–G7 | Fake-worker concurrency characterization at 5/10/25/50 workers | PASS | +| H0–H4 + H-REPAIR | Controlled SIGKILL interruption and recovery validation | PASS | +| I0–I1 | Real-source soak: 3 candidates, direct mode, workers=2 | PASS | +| J0–J3 | Evidence consolidation, correctness matrix, operational conclusions | PASS | + +### H-REPAIR: process-group kill strategy + +During Phase H, a critical discovery changed the interruption procedure: + +`uv run python` spawns Python as a child process of the `uv` supervisor. Sending SIGKILL +to the `uv` PID leaves the Python process as an orphan — it continues running and the +test never validates actual interruption. The correct strategy is to kill the entire +process group using `start_new_session=True` and `os.killpg`. + +**Kill-at-PENDING rationale:** The kill must be issued immediately upon the drain creating +a PENDING row, not after IN_PROGRESS detection. Chrome profile warm-up and CDN-cached +FBRef responses reduce processing time to under 200ms — waiting for IN_PROGRESS detection +is a race the test consistently loses. Killing at PENDING is the reliable gate. + +H1-R (`recover_all_stale` after process-group SIGKILL, PENDING state) and H2-R (restart +from PENDING, job completes at t+17s) both passed after H-REPAIR. + +### Phase I real-source soak + +Three real candidates were scraped against FBRef using direct mode (workers=2, no buffer, +no warm pool): + +| Candidate | player_id | +|---|---| +| Lionel Messi | `d70ce98e` | +| Cristiano Ronaldo | `dea698d9` | +| Erling Haaland | `1f44ac21` | + +All 3 jobs completed with status DONE and real FBRef data written to `tbl_player_info`. + +Note: The post-soak verification script contained a minor asyncpg bug (unused parameter +causing `IndeterminateDatatypeError`). The soak itself was durable and clean; the bug +was in the verification query only, not in the scraping pipeline. + +### Schema gotchas discovered during validation + +The following schema details are non-obvious and must be respected in future tooling and queries: + +- `tbl_player_info` lives in `sch_fbref_shared`, **not** `sch_fbref_backend` +- The `scrape_queue` row lock column is `locked_at`, **not** `claimed_at` +- The `tbl_player_urls` FK column is `fk_player`, **not** `fk_player_id` +- `tbl_players.career_start` and `career_end` are `NOT NULL` — any reset script must provide values +- Drain eligibility filter: `url_type='profile'`, `status IN ('PENDING','ACTIVE')`, `next_scrape_at <= now()` +- After a successful scrape: `tbl_player_urls.next_scrape_at` is advanced by `cadence_hours` (168h). Any + test reset that manipulates this column must restore it to a value in the past to re-trigger drain eligibility. + +### What not to capture + +This document does not and must not contain: + +- Database passwords, DSNs, or connection strings +- `SCRAPING__WORK_SERVER_TOKEN` or any other secret value +- CDP tokens, WebSocket URLs, browser session data, or cookies +- Browser profile paths or ephemeral volume names +- Raw HTML responses or scraped page content +- Any value that identifies a specific operator environment