164 lines
8.3 KiB
Markdown
164 lines
8.3 KiB
Markdown
---
|
||
id: pg-fix-scheduler-tick-seam
|
||
name: "pg scheduler — transient-fault tick seam: retry/exhaustion arms tested (review 003 Finding 1)"
|
||
status: completed
|
||
depends_on: []
|
||
scope: narrow
|
||
risk: low
|
||
impact: component
|
||
level: implementation
|
||
tags: [wave-6, postgres-engine, scheduler, test-hardening]
|
||
---
|
||
|
||
## Description
|
||
|
||
Review 003 Finding 1 (MEDIUM — `docs/reviews/003-wave-5-general-review.md`):
|
||
the pg scheduler's transient-error retry loop
|
||
(`alkstore-postgres/src/scheduler.rs:680-704`, `tick_with_retries`)
|
||
has never executed under any test — `pg-fix-scheduler-resilience`
|
||
pinned its *constants* (`the_tick_retry_policy_is_pinned`) but the
|
||
loop that uses them (retry, backoff sleeps, exhaustion error, the
|
||
`eprintln` diagnostic — lcov 171-184, 237-260) is dead code under
|
||
test, with no fault seam anywhere in the pg scheduler for `tick_once`.
|
||
This is the last uncovered hardening arm from the wave-4 fix batch,
|
||
and the exact class that produced review 002's live bugs.
|
||
|
||
Fix shape (the review's recommendation): a `cfg(test)` transient-
|
||
fault seam on the pg tick path, following the `sqlite-commit-error-arm
|
||
` precedent — a one-shot armed flag feeding a fabricated
|
||
transient error into the *real* retry loop (never a replica):
|
||
|
||
- (a) one retried tick then success: the seam faults once, the next
|
||
`tick_once` succeeds, the job processes — the retry + backoff-sleep
|
||
arms execute for real;
|
||
- (b) exhaustion: the seam faults the max retry count, the loop
|
||
exhausts to the typed error shape and the `eprintln` diagnostic
|
||
fires, the runner survives (the resilience property the fix
|
||
promised).
|
||
|
||
Cross-test safety is per-store scoping (the commit-fault seam's
|
||
pattern); no production-path changes — the seam is cfg-gated, born
|
||
disarmed, consumed-on-use.
|
||
|
||
## Acceptance Criteria
|
||
|
||
- [x] The seam lands cfg(test)-gated; the real `tick_with_retries`
|
||
executes under test (no loop replication)
|
||
- [x] Arms (a) and (b) pinned as above; the runner survives
|
||
exhaustion
|
||
- [x] `cargo test -p alkstore-postgres` green against the harness
|
||
server; skips clean server-less
|
||
- [x] Clippy `-D warnings`, fmt clean; workspace gates green
|
||
|
||
## References
|
||
|
||
- docs/reviews/003-wave-5-general-review.md §Finding 1 (sites, lcov
|
||
inventory, recommendation)
|
||
- docs/architecture/decisions/024-mem-engine.md (the wave-6 window
|
||
this rider rides — the plan's wave-6 section, "Riding the window")
|
||
- tasks/sqlite-commit-error-arm.md (the seam pattern of record)
|
||
- tasks/pg-fix-scheduler-resilience.md (the constants the loop uses)
|
||
|
||
## Notes
|
||
|
||
> Decisions of record the implementation made that the description
|
||
> didn't pin:
|
||
|
||
- **The seam is a per-store *counting* fault arm, not a boolean.** A
|
||
boolean arm (the sqlite commit-fault shape) reaches only the
|
||
retry-once arm; exhaustion needs the fault to survive the loop's
|
||
whole budget. `crate::seam::tick_fault` holds
|
||
`Flag = Arc<AtomicU64>` — born disarmed at 0, `arm(n)` adds n,
|
||
`take` consumes one (CAS `try_update`, no negative underflow) — so
|
||
arm(1) gives the retry-then-success arm and arm(`TICK_RETRIES + 1`)
|
||
the exhaustion arm with one mechanism. Consumption-exactness is
|
||
pinned by loading the flag after the run (0 = exactly the armed
|
||
count consumed).
|
||
- **Plumbing follows the `sqlite-commit-error-arm` precedent
|
||
exactly** (the `pg-fix-forwarder-reconnect` house pattern):
|
||
`#[cfg(test)]` field on `EngineCtx` and `PgStore`;
|
||
`PgStore::engine_ctx()` carries the store's flag clone into every
|
||
scheduler ctx; `#[cfg(test)]` param on `tick_once` / `tick_with_retries`
|
||
(the seam check sits at `tick_once`'s top — one *attempt* faults,
|
||
never ticks) with cfg-branched attempt statements — production
|
||
builds compile the plain path and never materialize the field.
|
||
Arming/observation surfaces: `PgStore::arm_tick_fault(faults)` and
|
||
`PgStore::tick_fault_flag()` (a `Flag` clone so the consumption pin
|
||
survives the store moving into a runner task — a load is enough,
|
||
no extra accessor). Per-store `Arc` scoping keeps parallel test
|
||
stores isolated.
|
||
- **The fault enters the loop via the entry points it already calls**
|
||
— both tests drive the full real path (`run_schedules` → the loop's
|
||
`tick_with_retries` → `tick_once`), no loop or error-shape
|
||
replication anywhere. The fabricated error is pool/database-shaped
|
||
(`Error::database(io::Error)` — the taxonomy's opaque fallback; the
|
||
loop retries on any `Err`, so the shape is an honesty choice, not a
|
||
correctness need).
|
||
- **"The runner survives exhaustion" pinned as written.** The
|
||
exhausting loop's exit *is* the typed error (the module docs'
|
||
contract — exhaustion exits `Err`); "survives" is pinned as: no
|
||
panic, opaque `Database` (never `LeadershipLost`), the tick-phase
|
||
context riding the source chain with the fabricated fault at its
|
||
bottom, the leadership lock row left (not released) to lapse at its
|
||
TTL (present, expiry bounded by the TTL of the run — never renewed
|
||
past the exit), and the store fully usable after (subsequent entry
|
||
points work). Arm (a) carries the *hiccups-don't-kill* survival
|
||
property (fault → backoff → real tick fires the due boundary →
|
||
clean stop `Ok(())`).
|
||
- **Replay-proofed live** (the sqlite task's discipline): with the
|
||
seam's fault consumption disabled (`false && take(…)` probe), the
|
||
exhaustion test cannot complete — the runner loops forever on its
|
||
nothing-due schedule because no fault ever enters the budget (the
|
||
mutation run was killed on timeout, which is itself the proof the
|
||
seam is load-bearing); reverted, green. The exhaustion test was
|
||
then wrapped in a 20 s bounded wait so a future regression of this
|
||
shape fails instead of hanging.
|
||
- **The backoff-sleep arms are pinned by wall-clock**, not value: the
|
||
exhaustion run must take ≥ 1.7 s (the 250 ms → 500 ms → 1 s window;
|
||
a fast-fail exit would not). The retry/exhaustion arms' `eprintln`
|
||
diagnostics execute (lcov-verified) but are not value-pinned —
|
||
stderr capture isn't in the harness convention; the exhaustion
|
||
*context* message (chain-carried) is the pinned diagnostic shape.
|
||
- `LEADER_TTL_S` widened `private` → `pub(crate)` (the lock-left-to-
|
||
TTL expiry pin reads it; no production-path change).
|
||
- lcov after: the Finding-1 sites (the retry loop — retry arm, backoff
|
||
sleeps, exhaustion `context_error`, diagnostics — and the `tick_once`
|
||
seam arm) carry **zero missed lines**.
|
||
|
||
## Summary
|
||
|
||
> What landed, verified how:
|
||
|
||
- **`alkstore-postgres/src/seam.rs`**: the `#[cfg(test)]`
|
||
`tick_fault` module — the per-store counting `Flag`
|
||
(`Arc<AtomicU64>`), `disarmed`/`arm`/`take` (consume-one CAS), and
|
||
the fabricated pool/database-shaped transient error, with the
|
||
pattern provenance (review 003 Finding 1 /
|
||
`sqlite-commit-error-arm`) in its doc comment.
|
||
- **`alkstore-postgres/src/scheduler.rs`**: cfg-gated `tick_fault`
|
||
field on `EngineCtx` (wildcard-safe destructures elsewhere); the
|
||
cfg-gated fault param on `tick_once` (seam check precedes the real
|
||
attempt) and `tick_with_retries`, attempt calls cfg-branched;
|
||
`run_schedules` threads the store's flag into the loop. The
|
||
production path never materializes the seam.
|
||
- **`alkstore-postgres/src/store.rs`**: cfg-gated `tick_fault` field
|
||
on `PgStore` (born `disarmed()`), `engine_ctx()` clones it into the
|
||
scheduler ctx, and the `arm_tick_fault` / `tick_fault_flag`
|
||
test-observation surfaces.
|
||
- **`alkstore-postgres/src/store/scheduler_tests.rs`**: two pins —
|
||
`one_faulted_tick_is_retried_and_the_job_fires` (arm a: faulted
|
||
attempt 1 → 250 ms backoff → the retried attempt fires the
|
||
backdated due boundary; exactly-one consumption; clean stop `Ok(())`)
|
||
and `tick_retry_exhaustion_exits_typed_and_leaves_the_lock_to_lapse`
|
||
(arm b: `TICK_RETRIES + 1` armed faults exhaust the budget; ≥ 1.7 s
|
||
real backoff window; opaque `Database` with the tick-phase context
|
||
and the fabricated fault in the source chain; exact consumption; the
|
||
leadership lock row left to its TTL lapse; the store usable after).
|
||
- **Gates**: `cargo build`; the full pg column live vs the harness
|
||
(124 lib + 25 suite + 9 schema; the two new tests green solo ×3 —
|
||
4.2–4.4 s); `cargo test --workspace` server-less green (skips
|
||
clean: core 25 + harness 3; sqlite 191 + 25); `cargo clippy
|
||
--all-targets -- -D warnings`; `cargo fmt --check`. cargo-llvm-cov
|
||
confirms scheduler.rs's retry-loop region zero missed lines (the
|
||
crate stands at 90.73% scheduler line coverage; the residue is the
|
||
accepted defensive-arm posture). |