7.9 KiB
id, name, status, depends_on, scope, risk, impact, level, tags
| id | name | status | depends_on | scope | risk | impact | level | tags | ||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| pg-fix-tx-wake | Fix tx enqueue/publish paths issuing no pg_notify wake (review 002 Finding 2 — F-1's root cause) | completed | narrow | low | component | implementation |
|
Description
Fix the HIGH finding from the wave-4 general review
(docs/reviews/002-wave-4-general-review.md §Finding 2, live-proven):
none of the tx-side enqueue/publish paths run pg_notify in the
caller's transaction. Sites: alkstore-postgres/src/tx.rs:290-329
(publish_tx/publish_with_key_tx), tx.rs:272-288 (enqueue_tx),
tx.rs:545-562 (outbox_enqueue_tx). notify_tx proves pg_notify
inside a tx is native commit-atomic (delivers at the caller's commit)
— the exact mechanism these paths have available and don't use. A
committed publish_tx/enqueue_tx never wakes a registered
listener (deterministic silence, live-proven), while the auto-commit
publish/Queue::enqueue wake immediately.
This is F-1's root cause, engine-side — tasks/review-wave-4.md
F-1's "root cause unresolved" and the gate's "the tx INSERT's commit
is the NOTIFY carrier" conclusion were both wrong. Mechanism: the
subscriber bridge attach-read + drain race the commit; when both beat
the commit the bridge drains empty and parks on wait_wake — and no
wake ever comes from a tx-committed publish, so the 15 s deadline
exhausts (~2% under the 8-thread suite).
Conformance angle: engine-postgres.md's mapping names pg_notify
as the streams wake trigger; core-contract.md pins "events never
require polling to become visible" and the tx/no-ghosts rows pin
rollback dropping the event row with its wake. SQLite's watcher
fires on any committed write, so its tx path wakes — the asymmetry
would also have failed wave 5's cross-engine equivalence rows.
Fix shape (the review's):
publish_with_key_tx: after the INSERT, oneSELECT pg_notify($1, '')inside the caller's tx — the channel is the stream name (the mechanism-name-is-the-channel realization, already pinned instream.rs's docs). Empty payload: the wake carriesWake { channel }only, never payloads (ADR-008 §3). This is the contract obligation — the load-bearing fix.enqueue_txandoutbox_enqueue_tx: add the same wake (channel = queue name; the outbox's derived__alkstore_outbox:{name}) for posture parity with SQLite's watcher — a weaker obligation (the queues row's pinned re-poll safety net covers correctness; the wake is latency posture), but the asymmetry is free to close and the review recommends it.run_onceis a pull op, consumer-driven — no wake needed there (do not add one).
Record updates: this fix retires F-1's engine arm. Update
tasks/review-wave-4.md's F-1 note (engine arm retired by this fix;
the wave-5 suite-hardening candidates — parked-recv deadline, lag
diagnostics — remain valid as defense-in-depth but are no longer
load-bearing for this flake) and docs/plans/implementation.md's
review-rounds line for the general review.
Acceptance Criteria
publish_with_key_txissues thepg_notifywake commit-atomically (channel = stream name, empty payload) — pinned by test: a registered listener receives the wake at/after the caller's commit, deterministically- The no-ghosts arm pinned: a rolled-back
publish_txproduces no wake (the wake is commit-atomic, not statement-atomic) enqueue_txandoutbox_enqueue_txissue their wakes (channel = queue name / derived backing queue) — pinned by testtx_publishes_compose_with_the_handleis deterministic: repeated solo runs (≥ 20) green with zero deadline misses- Auto-commit
publish/enqueuebehavior unchanged (still wake; no double-wake regression pinned where the suite observes it) tasks/review-wave-4.mdF-1 note +docs/plans/implementation.mdreview-rounds line updated to record the retirementcargo test -p alkstore-postgres(harness server), clippy-D warnings, fmt clean; gates green server-less
References
- docs/reviews/002-wave-4-general-review.md §Finding 2 (the finding, the probe, the fix shape, the F-1 mechanism)
- docs/architecture/engine-postgres.md (pg_notify as the streams wake trigger)
- docs/architecture/core-contract.md (streams visibility; tx/no-ghosts rows)
- docs/architecture/decisions/008-contract-v1-pinning.md §3 (Wake carries channel only)
- tasks/review-wave-4.md (F-1's record — updated by this task)
- alkstore-postgres/src/tx.rs (
notify_tx— the proven commit-atomic mechanics these paths adopt)
Notes
Fix-shape decisions of record (what the description left open):
- One-owner wake statement: all three producer paths call a
shared
tx::wake_tx(client, channel)—SELECT pg_notify($1, '')(empty payload,Wake { channel }only per ADR-008 §3) issued on the caller's tx client after the INSERT. Channel = the stream name (publish_txridespublish_with_key_tx, so one site), the queue name (enqueue_tx), and the outbox's derived__alkstore_outbox:{name}(outbox_enqueue_tx). - Wake error posture: best-effort log-and-swallow (the engine-wide stance — a wake must never fail a write), matching the auto-commit publish/enqueue twins. Documented hazard: a failed statement aborts the pg tx server-side, so a swallowed wake failure still surfaces typed at COMMIT — no silent half-state; a rolled-back tx's queued notifies are discarded server-side with the row (the no-ghosts arm is native).
- No double-wake: the wake is applied at the tx call sites, NOT
inside the shared
enqueue_row/INSERT — the auto-commitQueue::enqueueand the scheduler's fire-wake keep their own single statements on their pool connections, untouched. run_onceuntouched (pull op, consumer-driven — no wake, per the task).- Test shape: one harness test in
tx_tests.rs(tx_producer_wakes_are_commit_atomic_and_rollback_discards_them) pins all four arms — pre-commit silence (commit-atomic, not statement-atomic: 300 ms statement window + 100 ms listen, then commit), deterministic wake delivery at commit on all three registered channels, rollback silence, committed row-count truth. Usesforwarder().register+ the raw fanout (listener_hears);registerdoes no name validation, so the reserved derived outbox channel registers directly. - Record updates went slightly beyond the strict line: besides
the review-rounds line,
docs/plans/implementation.md's frontmatterlast_updatedand the wave-4 gate bullet's "root cause unresolved" text got dated supersede/correction pointers (the stale-reference discipline — those conclusions were factually wrong, not just unresolved).
Summary
Landed: the tx producer paths' commit-atomic pg_notify wakes
(tx.rs: wake_tx helper + the three call sites + the "The tx wakes"
module-doc section pinning the semantics) — publish_with_key_tx/
publish_tx (the contract obligation: streams never require polling;
rollback drops the row with its wake) and the posture-parity
enqueue_tx/outbox_enqueue_tx twins (channel = queue name / derived
backing queue); the new pinned tx-wake test; the F-1 retirement
recorded in tasks/review-wave-4.md (including the correction that
the gate's "the tx INSERT's commit is the NOTIFY carrier" conclusion
was wrong) and docs/plans/implementation.md.
Verified: full pg lib suite 116/116 green against the harness
server (two consecutive full runs, ~70 s each); the new wake test
green; tx_publishes_compose_with_the_handle deterministic — 20
consecutive solo runs green with zero deadline misses; workspace
cargo test server-less green (harness tests skip cleanly);
cargo clippy --all-targets -- -D warnings clean; cargo fmt --check
clean. Auto-commit publish/enqueue unchanged (their wake tests ride
the suite untouched).