8.3 KiB
id, name, status, depends_on, scope, risk, impact, level, tags
| id | name | status | depends_on | scope | risk | impact | level | tags | ||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| pg-fix-scheduler-tick-seam | pg scheduler — transient-fault tick seam: retry/exhaustion arms tested (review 003 Finding 1) | completed | narrow | low | component | implementation |
|
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_oncesucceeds, 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
eprintlndiagnostic 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
- The seam lands cfg(test)-gated; the real
tick_with_retriesexecutes under test (no loop replication) - Arms (a) and (b) pinned as above; the runner survives exhaustion
cargo test -p alkstore-postgresgreen against the harness server; skips clean server-less- 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_faultholdsFlag = Arc<AtomicU64>— born disarmed at 0,arm(n)adds n,takeconsumes one (CAStry_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-armprecedent exactly (thepg-fix-forwarder-reconnecthouse pattern):#[cfg(test)]field onEngineCtxandPgStore;PgStore::engine_ctx()carries the store's flag clone into every scheduler ctx;#[cfg(test)]param ontick_once/tick_with_retries(the seam check sits attick_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)andPgStore::tick_fault_flag()(aFlagclone so the consumption pin survives the store moving into a runner task — a load is enough, no extra accessor). Per-storeArcscoping 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'stick_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 anyErr, 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, opaqueDatabase(neverLeadershipLost), 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 stopOk(())). - 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'
eprintlndiagnostics 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_Swidenedprivate→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 thetick_onceseam arm) carry zero missed lines.
Summary
What landed, verified how:
alkstore-postgres/src/seam.rs: the#[cfg(test)]tick_faultmodule — the per-store countingFlag(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-gatedtick_faultfield onEngineCtx(wildcard-safe destructures elsewhere); the cfg-gated fault param ontick_once(seam check precedes the real attempt) andtick_with_retries, attempt calls cfg-branched;run_schedulesthreads the store's flag into the loop. The production path never materializes the seam.alkstore-postgres/src/store.rs: cfg-gatedtick_faultfield onPgStore(borndisarmed()),engine_ctx()clones it into the scheduler ctx, and thearm_tick_fault/tick_fault_flagtest-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 stopOk(())) andtick_retry_exhaustion_exits_typed_and_leaves_the_lock_to_lapse(arm b:TICK_RETRIES + 1armed faults exhaust the budget; ≥ 1.7 s real backoff window; opaqueDatabasewith 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 --workspaceserver-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).