tests: review 003 Findings 2-3 — pre-release dispositions
Finding 2(a) landed: a one-shot cfg(test) generation-top LISTEN-pause seam on the pg forwarder (listen_pause in seam.rs) lets the LISTEN-window death test kill the fresh reconnect generation's backend before its snapshot LISTEN — the LISTEN then fails honestly (57P01 terminating) and the production fall-through arm (listen_ok == false) runs unmodified. The harness test pins the honest mid-reconnect transient, full recovery (wake, re-issue recovery of the refused mid-gap registration), and the exactly-one-wake differentiator; a server-less unit pin covers the flag. Finding 2(b) + the defensive arms stay parked with the code-read as pin, recorded in the task file with current line refs. Finding 3 landed: the rollback_fault seam (commit-fault mechanics) feeds SQLITE_FULL-shaped errors into both ROLLBACK failure drop paths — SqliteTxHandle::drop and the cancelled op's ConnLease::drop — exercised in one test (phases: open-handle drop; single-poll-then-drop mid-op cancellation), pinning slot replenish via reopen, no ghosts, and the store staying usable.
This commit is contained in:
1 parent
c132714a77
commit
c3a0c54df0
8 files changed
+670
-10
No files matched your search
@@ -218,6 +218,12 @@ pub(crate) struct Forwarder {
|
||||
/// `#[cfg(test)]` swap accessors).
|
||||
#[cfg(test)]
|
||||
reconnect_config: ReconnectConfigSlot,
|
||||
/// The test-only generation-top LISTEN pause (review 003 Finding
|
||||
/// 2a's seam) — the loop's own [`LoopCtx`] clone is the live read
|
||||
/// path; the field exists for the test arm accessor below. Dead
|
||||
/// in production builds.
|
||||
#[cfg(test)]
|
||||
listen_pause: crate::seam::listen_pause::Flag,
|
||||
shutdown: watch::Sender<bool>,
|
||||
}
|
||||
|
||||
@@ -360,6 +366,8 @@ impl Forwarder {
|
||||
let generation: GenerationSlot = Arc::new(AtomicU64::new(0));
|
||||
let reconnect_config_slot: ReconnectConfigSlot =
|
||||
Arc::new(std::sync::Mutex::new(reconnect_config));
|
||||
#[cfg(test)]
|
||||
let listen_pause = crate::seam::listen_pause::disarmed();
|
||||
// The connection handed in was just established — the liveness
|
||||
// flag starts true and the loop clears it at each generation's
|
||||
// death (the register path's honest transient state).
|
||||
@@ -372,6 +380,8 @@ impl Forwarder {
|
||||
client,
|
||||
connection,
|
||||
reconnect_config: reconnect_config_slot.clone(),
|
||||
#[cfg(test)]
|
||||
listen_pause: listen_pause.clone(),
|
||||
fanout: fanout.clone(),
|
||||
fanout_slot: fanout_slot.clone(),
|
||||
channels: channels.clone(),
|
||||
@@ -389,6 +399,8 @@ impl Forwarder {
|
||||
connected,
|
||||
#[cfg(test)]
|
||||
reconnect_config: reconnect_config_slot,
|
||||
#[cfg(test)]
|
||||
listen_pause,
|
||||
shutdown: shutdown_tx,
|
||||
}
|
||||
}
|
||||
@@ -505,6 +517,17 @@ impl Forwarder {
|
||||
.expect("reconnect config mutex poisoned")
|
||||
.clone()
|
||||
}
|
||||
|
||||
/// Arm the test-only generation-top LISTEN pause (one-shot) — the
|
||||
/// connection-death-during-LISTEN seam (review 003 Finding 2a):
|
||||
/// the loop sleeps a short window at the next generation's top,
|
||||
/// *before* issuing its snapshot LISTEN, so a test can kill the
|
||||
/// fresh generation's backend inside the window and make the
|
||||
/// LISTEN fail honestly (the real fall-through arm runs).
|
||||
#[cfg(test)]
|
||||
pub(crate) fn arm_generation_listen_pause(&self) {
|
||||
crate::seam::listen_pause::arm(&self.listen_pause)
|
||||
}
|
||||
}
|
||||
|
||||
/// The reconnect arm's retry core (review 002 Finding 1's fix): sleep
|
||||
@@ -600,6 +623,10 @@ struct LoopCtx {
|
||||
client: tokio_postgres::Client,
|
||||
connection: ListenerConnection,
|
||||
reconnect_config: ReconnectConfigSlot,
|
||||
/// The test-only generation-top LISTEN pause (review 003 Finding
|
||||
/// 2a's seam); dead in production builds.
|
||||
#[cfg(test)]
|
||||
listen_pause: crate::seam::listen_pause::Flag,
|
||||
fanout: broadcast::Sender<RawNotification>,
|
||||
fanout_slot: FanoutSlot,
|
||||
channels: Arc<ChannelSet>,
|
||||
@@ -614,6 +641,8 @@ async fn forwarder_loop(
|
||||
client,
|
||||
connection,
|
||||
reconnect_config,
|
||||
#[cfg(test)]
|
||||
listen_pause,
|
||||
fanout,
|
||||
fanout_slot,
|
||||
channels,
|
||||
@@ -639,6 +668,22 @@ async fn forwarder_loop(
|
||||
// through the Client.
|
||||
let mut poll_task = tokio::spawn(poll_loop(connection, fanout.clone()));
|
||||
|
||||
// The test-only LISTEN-window seam (review 003 Finding 2a):
|
||||
// one-shot armed, this generation's top pauses here for the
|
||||
// window a backend-kill test can land in — the LISTEN below
|
||||
// then runs against a dead session and fails honestly, the
|
||||
// exact "connection died before/during the LISTEN" shape the
|
||||
// fall-through arm exists for. Production builds compile a
|
||||
// plain `false`.
|
||||
#[cfg(test)]
|
||||
let pre_listen_pause = crate::seam::listen_pause::take(&listen_pause);
|
||||
#[cfg(not(test))]
|
||||
let pre_listen_pause = false;
|
||||
if pre_listen_pause {
|
||||
#[cfg(test)]
|
||||
tokio::time::sleep(crate::seam::listen_pause::WINDOW).await;
|
||||
}
|
||||
|
||||
// Issue LISTENs from the dynamic channel set's snapshot (on
|
||||
// first connect it carries pre-registered channels; on
|
||||
// reconnect it carries everything `listen` ever registered).
|
||||
|
||||
@@ -42,6 +42,44 @@ pub(crate) fn database_error(message: impl Into<String>) -> Error {
|
||||
Error::database(std::io::Error::other(message.into()))
|
||||
}
|
||||
|
||||
/// The `cfg(test)` generation-top LISTEN pause seam (review 003
|
||||
/// Finding 2a): a one-shot arm that makes the forwarder loop sleep a
|
||||
/// short window at the next connection generation's top, **before**
|
||||
/// issuing its snapshot LISTEN — the "connection died before/during
|
||||
/// the LISTEN" window (the fall-through to the reconnect path at
|
||||
/// `forwarder.rs`'s `listen_ok == false` arm). A test arms it, forces
|
||||
/// the reconnect (a backend kill), then kills the fresh generation's
|
||||
/// backend inside the window: the fresh session's LISTEN batch
|
||||
/// executes against a dead connection and fails **honestly** — the
|
||||
/// error is real driver-against-dead-session, only the timing is
|
||||
/// fabricated — and the production fall-through arm runs unmodified.
|
||||
/// One-store scoping (disarms on first consumption, the commit-fault
|
||||
/// seam's shape) keeps parallel test stores from interfering.
|
||||
#[cfg(test)]
|
||||
pub(crate) mod listen_pause {
|
||||
use std::sync::Arc;
|
||||
use std::sync::atomic::{AtomicBool, Ordering};
|
||||
use std::time::Duration;
|
||||
|
||||
pub(crate) type Flag = Arc<AtomicBool>;
|
||||
|
||||
pub(crate) fn disarmed() -> Flag {
|
||||
Arc::new(AtomicBool::new(false))
|
||||
}
|
||||
|
||||
pub(crate) fn arm(flag: &Flag) {
|
||||
flag.store(true, Ordering::Release);
|
||||
}
|
||||
|
||||
pub(crate) fn take(flag: &Flag) -> bool {
|
||||
flag.swap(false, Ordering::AcqRel)
|
||||
}
|
||||
|
||||
/// The pause window: wide enough for a kill probe to land inside
|
||||
/// it, narrow enough that the test's waits stay bounded.
|
||||
pub(crate) const WINDOW: Duration = Duration::from_millis(500);
|
||||
}
|
||||
|
||||
/// The `cfg(test)` tick-fault seam (`pg-fix-scheduler-tick-seam`,
|
||||
/// review 003 Finding 1): a per-store arm that, when set, substitutes
|
||||
/// a fabricated pool/database-shaped error for one `tick_once`
|
||||
|
||||
@@ -108,6 +108,22 @@ async fn listener_sessions(admin: &tokio_postgres::Client, app_name: &str) -> i6
|
||||
.get(0)
|
||||
}
|
||||
|
||||
/// Kill every listener session of `app_name` — the backend-killing
|
||||
/// probe the reconnect tests ride (`false` when no listener backend
|
||||
/// is currently live, e.g. while the forwarder is mid-reconnect).
|
||||
async fn kill_listener(admin: &tokio_postgres::Client, app_name: &str) -> bool {
|
||||
admin
|
||||
.query_one(
|
||||
"SELECT pg_terminate_backend(pid) FROM pg_stat_activity
|
||||
WHERE application_name = $1 AND pid <> pg_backend_pid()",
|
||||
&[&app_name],
|
||||
)
|
||||
.await
|
||||
.ok()
|
||||
.and_then(|row| row.try_get::<_, bool>(0).ok())
|
||||
.unwrap_or(false)
|
||||
}
|
||||
|
||||
/// The forwarder's fanout subscription surface for the minimal-delivery
|
||||
/// proof (bridging into `WakeReceiver`s is the notify-listen task's;
|
||||
/// the skeleton's proof fans the raw notification channel names out).
|
||||
@@ -314,6 +330,183 @@ async fn forwarder_reconnects_after_backend_kill_and_delivers() {
|
||||
drop_schema(&admin, &schema).await;
|
||||
}
|
||||
|
||||
/// Acceptance (review 003 Finding 2a): the "connection died
|
||||
/// before/during the LISTEN" fall-through arm — a generation whose
|
||||
/// LISTEN issue fails because the session died inside the LISTEN
|
||||
/// window — falls through to the reconnect path with the honest
|
||||
/// transient state (`connected` cleared: registrations during the gap
|
||||
/// fail the transient `Database`, never a spurious success), reaps
|
||||
/// the dead generation's poll task, reconnects, and recovers fully:
|
||||
/// the reconnect-wake broadcasts, the pre-death registration's
|
||||
/// NOTIFY delivers (the re-issue), and the mid-gap registration —
|
||||
/// refused with the transient but registry-written-first — is
|
||||
/// recovered by the re-issue too.
|
||||
///
|
||||
/// The seam is the `cfg(test)` generation-top LISTEN pause
|
||||
/// (`seam.rs`'s one-shot `listen_pause`): the fresh reconnect
|
||||
/// generation pauses before its LISTEN issue, and the test kills that
|
||||
/// fresh backend inside the window — the LISTEN batch then executes
|
||||
/// against a dead session and fails *honestly* (real driver error,
|
||||
/// fabricated timing only), exercising the production fall-through
|
||||
/// arm unmodified.
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn forwarder_recovers_from_connection_death_during_the_listen_window() {
|
||||
let Some(dsn) = harness_dsn() else {
|
||||
eprintln!("skip: no harness server");
|
||||
return;
|
||||
};
|
||||
let schema = instance_namer("listen-death")();
|
||||
let store = open_store(&dsn, test_opts(&schema)).await.unwrap();
|
||||
let forwarder = store.forwarder().clone();
|
||||
let admin = harness_client().await.unwrap();
|
||||
|
||||
let channel = format!("lstndeath_probe_{}", instance_namer("ch")());
|
||||
let mut rx = forwarder.subscribe().unwrap();
|
||||
forwarder.register(&channel).await.unwrap();
|
||||
|
||||
// Pre-death delivery verified (the normal path is not what broke).
|
||||
let notify_sql = |ch: &str, payload: &str| {
|
||||
format!(
|
||||
"NOTIFY {}, '{payload}'",
|
||||
crate::schema::quote_identifier(ch)
|
||||
)
|
||||
};
|
||||
admin
|
||||
.batch_execute(¬ify_sql(&channel, "pre-death"))
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(
|
||||
wait_for_channel(&mut rx, &channel, Duration::from_secs(3)).await,
|
||||
"pre-death delivery must work first"
|
||||
);
|
||||
|
||||
// A dedicated wake counter (subscribed before the kills): the
|
||||
// fall-through arm's differentiator — see the exactly-one-wake
|
||||
// pin after recovery.
|
||||
let mut wake_counter = forwarder.subscribe().unwrap();
|
||||
|
||||
// Arm the seam, then kill the listener's first-generation backend:
|
||||
// the reconnect's fresh generation pauses before its LISTEN
|
||||
// issue — the window the second kill lands in.
|
||||
forwarder.arm_generation_listen_pause();
|
||||
let app_name = store.listener_application_name().to_string();
|
||||
assert!(
|
||||
kill_listener(&admin, &app_name).await,
|
||||
"the first listener backend was killed"
|
||||
);
|
||||
|
||||
// Kill the fresh reconnect generation's backend inside its
|
||||
// pre-LISTEN pause (retry until a listener backend is found
|
||||
// again — the fresh connection lives only behind the pause).
|
||||
let mut fresh_killed = false;
|
||||
for _ in 0..80 {
|
||||
if kill_listener(&admin, &app_name).await {
|
||||
fresh_killed = true;
|
||||
break;
|
||||
}
|
||||
tokio::time::sleep(Duration::from_millis(25)).await;
|
||||
}
|
||||
assert!(
|
||||
fresh_killed,
|
||||
"the fresh reconnect generation's backend was killed during its \
|
||||
pre-LISTEN pause (the connection death inside the LISTEN window)"
|
||||
);
|
||||
|
||||
// The fall-through's honest transient: a registration during the
|
||||
// gap fails with the transient `Database` (the connection is
|
||||
// gone; never a spurious success) — and its registry entry stays
|
||||
// written-first, recovered by the fresh generation's re-issue.
|
||||
let interrupted = instance_namer("interrupted-ch")();
|
||||
let err = forwarder.register(&interrupted).await.unwrap_err();
|
||||
assert!(
|
||||
matches!(err, alkstore::Error::Database(_)),
|
||||
"registration during the LISTEN-death gap fails with the honest \
|
||||
transient, got: {err:?}"
|
||||
);
|
||||
|
||||
// Recovery: the fall-through reaps the dead generation, reconnects,
|
||||
// and re-issues the snapshot — reconnect-wake, both channels'
|
||||
// delivery back, fresh registrations healthy again.
|
||||
let got_wake = wait_for_channel(
|
||||
&mut rx,
|
||||
alkstore::RESERVED_LISTENER_RECONNECTED,
|
||||
Duration::from_secs(10),
|
||||
)
|
||||
.await;
|
||||
let mut delivered = false;
|
||||
let mut interrupted_delivered = false;
|
||||
for _ in 0..30 {
|
||||
let _ = admin
|
||||
.batch_execute(¬ify_sql(&channel, "post-recovery"))
|
||||
.await;
|
||||
let _ = admin
|
||||
.batch_execute(¬ify_sql(&interrupted, "interrupted-recovered"))
|
||||
.await;
|
||||
if !delivered && wait_for_channel(&mut rx, &channel, Duration::from_millis(150)).await {
|
||||
delivered = true;
|
||||
}
|
||||
if !interrupted_delivered
|
||||
&& wait_for_channel(&mut rx, &interrupted, Duration::from_millis(150)).await
|
||||
{
|
||||
interrupted_delivered = true;
|
||||
}
|
||||
if delivered && interrupted_delivered {
|
||||
break;
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
got_wake && delivered && interrupted_delivered,
|
||||
"after the LISTEN-window death the forwarder must fall through to \
|
||||
reconnect and recover (wake {got_wake}, pre-death channel \
|
||||
{delivered}, mid-gap registration recovered {interrupted_delivered})"
|
||||
);
|
||||
|
||||
// Exactly one reconnect-wake for the whole kill pair, observed on
|
||||
// the dedicated counter subscribed before the kills: the paused
|
||||
// generation's LISTEN never succeeded, so it never verified and
|
||||
// never broadcast — the count is the fall-through arm's
|
||||
// observable (a kill landing *after* a healthy LISTEN would have
|
||||
// produced a verified generation and a second wake). First the
|
||||
// single wake is received; then silence is pinned.
|
||||
match tokio::time::timeout(Duration::from_secs(10), wake_counter.recv()).await {
|
||||
Ok(Ok(n)) => assert_eq!(
|
||||
n.channel,
|
||||
alkstore::RESERVED_LISTENER_RECONNECTED,
|
||||
"the counter's first delivery is the single reconnect-wake, got: {n:?}"
|
||||
),
|
||||
other => panic!("the reconnect wake must reach the pre-kill counter: {other:?}"),
|
||||
}
|
||||
let quiet_until = tokio::time::Instant::now() + Duration::from_secs(1);
|
||||
let second_wake = loop {
|
||||
let remaining = quiet_until.saturating_duration_since(tokio::time::Instant::now());
|
||||
if remaining.is_zero() {
|
||||
break false;
|
||||
}
|
||||
match tokio::time::timeout(remaining, wake_counter.recv()).await {
|
||||
Ok(Ok(n)) if n.channel == alkstore::RESERVED_LISTENER_RECONNECTED => {
|
||||
break true;
|
||||
}
|
||||
Ok(Ok(_)) => continue,
|
||||
Ok(Err(_)) | Err(_) => break false,
|
||||
}
|
||||
};
|
||||
assert!(
|
||||
!second_wake,
|
||||
"exactly one reconnect-wake expected — the LISTEN-failed generation \
|
||||
never verified (a second wake would mean the kill landed after a \
|
||||
healthy LISTEN, not during the LISTEN window)"
|
||||
);
|
||||
|
||||
let recovery_channel = instance_namer("recovery-ch")();
|
||||
forwarder
|
||||
.register(&recovery_channel)
|
||||
.await
|
||||
.expect("registrations succeed again post-recovery");
|
||||
|
||||
store.close();
|
||||
drop_schema(&admin, &schema).await;
|
||||
}
|
||||
|
||||
/// Acceptance: pool bounds — `max_size` caps concurrent checkouts (the
|
||||
/// +1 listener sits outside the pool).
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
@@ -926,6 +1119,34 @@ fn fanout_release_guard_empties_the_shared_slot_on_drop() {
|
||||
|
||||
const FANOUT_TEST_CAPACITY: usize = 1;
|
||||
|
||||
/// Unit pin (server-less): the LISTEN-window pause seam (review 003
|
||||
/// Finding 2a) — born disarmed, arming causes exactly one generation-
|
||||
/// top pause (disarms on consumption), and re-arming re-fires exactly
|
||||
/// once: parallel-armed test stores cannot interfere and a paused run
|
||||
/// cannot leak pauses into later generations.
|
||||
#[test]
|
||||
fn generation_listen_pause_consumes_once_per_arm() {
|
||||
let flag = crate::seam::listen_pause::disarmed();
|
||||
assert!(
|
||||
!crate::seam::listen_pause::take(&flag),
|
||||
"born disarmed — no pause without arming"
|
||||
);
|
||||
crate::seam::listen_pause::arm(&flag);
|
||||
assert!(
|
||||
crate::seam::listen_pause::take(&flag),
|
||||
"one pause after the arm"
|
||||
);
|
||||
assert!(
|
||||
!crate::seam::listen_pause::take(&flag),
|
||||
"the pause is one-shot — disarmed by its consumption"
|
||||
);
|
||||
crate::seam::listen_pause::arm(&flag);
|
||||
assert!(
|
||||
crate::seam::listen_pause::take(&flag),
|
||||
"re-arming fires exactly once again"
|
||||
);
|
||||
}
|
||||
|
||||
/// Unit pin (server-less) — the stale-command reconcile (review 002
|
||||
/// Finding 4's decision), **all four command/staleness combinations**:
|
||||
/// a stale-generation UNLISTEN is dropped (the hazard — replayed after
|
||||
|
||||
Reference in new issue
Block a user