pg: run_schedules' failed-renewal arm classifies as Database, never LeadershipLost (review 004 item 9's pg twin) — a lost lease (Ok(false)) keeps the respawn arm unchanged; a failed renewal (closed store / storage failure) propagates its opaque classification, matching the sqlite task's 2026-10-11 classification landed engine-neutral in ADR-009 §1 / core-contract §Errors (cited, not re-decided); module doc aligned; two tests pin both arms on the harness server (the sqlite twin's pin shapes); task -> completed
This commit is contained in:
1 parent
c03c798e34
commit
608104ab13
3 files changed
+248
-9
No files matched your search
@@ -35,7 +35,16 @@
|
|||||||
//! its own lease by drift; a failed mid-sleep renewal is not acted
|
//! its own lease by drift; a failed mid-sleep renewal is not acted
|
||||||
//! on at the sleep site — the loop's top-of-iteration renew owns
|
//! on at the sleep site — the loop's top-of-iteration renew owns
|
||||||
//! the loss decision). Acquire-time refusal is `Err(LeadershipLost)`
|
//! the loss decision). Acquire-time refusal is `Err(LeadershipLost)`
|
||||||
//! — the same matchable loss a mid-run loser returns. The tick:
|
//! — the same matchable loss a mid-run loser returns. A mid-run
|
||||||
|
//! top-of-iteration renewal's arms (the SQLite twin's shape, the
|
||||||
|
//! 2026-10-11 classification, review 004 item 9): a *refused*
|
||||||
|
//! renewal (`renew` → `Ok(false)` — the lease expired and was
|
||||||
|
//! re-acquired elsewhere) is the loss, `Err(LeadershipLost)`
|
||||||
|
//! return-before-tick (the respawn arm); a *failed* renewal is not
|
||||||
|
//! a loss and propagates its opaque `Database` classification
|
||||||
|
//! unchanged (the closed-store posture — respawning a leader
|
||||||
|
//! against a dead store is misdirected — and any other storage
|
||||||
|
//! failure). The tick:
|
||||||
//! one pool-checked-out transaction (the [`crate::queue::in_tx`]
|
//! one pool-checked-out transaction (the [`crate::queue::in_tx`]
|
||||||
//! frame), the schedule row's `SELECT … FOR UPDATE` inside the tick
|
//! frame), the schedule row's `SELECT … FOR UPDATE` inside the tick
|
||||||
//! tx as the row lock that serializes rogue second tickers (ADR-009
|
//! tx as the row lock that serializes rogue second tickers (ADR-009
|
||||||
@@ -679,13 +688,27 @@ pub(crate) fn run_schedules(stop: StopToken, ctx: EngineCtx) -> BoxedFuture<'sta
|
|||||||
let _ = lease.release().await;
|
let _ = lease.release().await;
|
||||||
return Ok(());
|
return Ok(());
|
||||||
}
|
}
|
||||||
let renewed = lease.renew(LEADER_TTL_S).await.unwrap_or(false);
|
let renewed = lease.renew(LEADER_TTL_S).await;
|
||||||
if !renewed {
|
match renewed {
|
||||||
// The lock was lost (expired and re-acquired elsewhere
|
Ok(true) => {}
|
||||||
// — exclusion lapses silently at TTL expiry). Return
|
Ok(false) => {
|
||||||
// before ticking: never fire on a stolen lease.
|
// The lock was lost (expired and re-acquired
|
||||||
|
// elsewhere — exclusion lapses silently at TTL
|
||||||
|
// expiry). Return before ticking: never fire on a
|
||||||
|
// stolen lease.
|
||||||
return Err(Error::LeadershipLost);
|
return Err(Error::LeadershipLost);
|
||||||
}
|
}
|
||||||
|
Err(e) => {
|
||||||
|
// A failed renewal is not a leadership loss: the
|
||||||
|
// lock semantics were never reached. The closed-
|
||||||
|
// store arm (the engine-wide `Database` posture —
|
||||||
|
// respawning a leader against a dead store is
|
||||||
|
// misdirected, review 004 item 9) and every other
|
||||||
|
// renewal failure propagate their opaque
|
||||||
|
// classification unchanged.
|
||||||
|
return Err(e);
|
||||||
|
}
|
||||||
|
}
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
let soonest = tick_with_retries(&pool, &schema, Some(&tick_fault)).await?;
|
let soonest = tick_with_retries(&pool, &schema, Some(&tick_fault)).await?;
|
||||||
#[cfg(not(test))]
|
#[cfg(not(test))]
|
||||||
|
|||||||
@@ -1713,3 +1713,171 @@ async fn tick_retry_exhaustion_exits_typed_and_leaves_the_lock_to_lapse() {
|
|||||||
|
|
||||||
drop_schema(&fx.admin, &fx.schema).await;
|
drop_schema(&fx.admin, &fx.schema).await;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Acceptance: the closed-store renewal arm reports the engine-wide
|
||||||
|
/// closed-store `Database` classification — never `LeadershipLost`
|
||||||
|
/// (review 004 item 9: flattening every renew `Err` into the loss arm
|
||||||
|
/// misdirected the documented respawn recipe at a dead store — an
|
||||||
|
/// operator respawns a leader against a store that is closed; the
|
||||||
|
/// lock semantics were never reached when the machinery itself
|
||||||
|
/// errors, the 2026-10-11 classification). The runner runs on the
|
||||||
|
/// fixture store's own closed flag and pool (the `engine_ctx` seam);
|
||||||
|
/// the runner closes mid-run — the next loop-top renewal sees the
|
||||||
|
/// closed flag and the classification propagates unchanged. The
|
||||||
|
/// SQLite twin's pin shape (`closed_store_run_reports_the_closed_
|
||||||
|
/// classification_not_leadership_lost`).
|
||||||
|
#[tokio::test(flavor = "multi_thread")]
|
||||||
|
async fn closed_store_run_reports_the_closed_classification_not_leadership_lost() {
|
||||||
|
let Some(fx) = Fixture::open(&instance_namer("so_cls")).await else {
|
||||||
|
eprintln!("skip: no harness server");
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
|
||||||
|
fx.store
|
||||||
|
.schedule("beat", "@every 1s", "closedq", json!(1), Default::default())
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let ctx = fx.store.engine_ctx();
|
||||||
|
let runner =
|
||||||
|
tokio::spawn(async move { crate::scheduler::run_schedules(StopToken::new(), ctx).await });
|
||||||
|
|
||||||
|
// Cross the renew arm at least once: the loop is demonstrably
|
||||||
|
// mid-run (one boundary fired) before the store closes.
|
||||||
|
let deadline = tokio::time::Instant::now() + Duration::from_secs(20);
|
||||||
|
loop {
|
||||||
|
let fired: i64 = fx
|
||||||
|
.admin
|
||||||
|
.query_one(
|
||||||
|
&format!(
|
||||||
|
"SELECT count(*) FROM {} WHERE queue = 'closedq'",
|
||||||
|
crate::schema::QualifiedTable {
|
||||||
|
schema: &fx.schema,
|
||||||
|
name: crate::schema::tables::JOB,
|
||||||
|
}
|
||||||
|
),
|
||||||
|
&[],
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap()
|
||||||
|
.get(0);
|
||||||
|
if fired >= 1 {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
assert!(
|
||||||
|
tokio::time::Instant::now() < deadline,
|
||||||
|
"the runner never fired the first boundary"
|
||||||
|
);
|
||||||
|
tokio::time::sleep(Duration::from_millis(200)).await;
|
||||||
|
}
|
||||||
|
|
||||||
|
fx.store.close();
|
||||||
|
|
||||||
|
// The exit is prompt (the sleep is at most one 1 s boundary wide;
|
||||||
|
// the next loop-top renewal sees the closed store).
|
||||||
|
let outcome = tokio::time::timeout(Duration::from_secs(10), runner)
|
||||||
|
.await
|
||||||
|
.expect("the closed-store exit must terminate promptly")
|
||||||
|
.unwrap();
|
||||||
|
match outcome {
|
||||||
|
Err(Error::Database(source)) => {
|
||||||
|
assert!(
|
||||||
|
source.to_string().contains("closed"),
|
||||||
|
"the closed-store arm carries the closed source chain: {source}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
Err(Error::LeadershipLost) => {
|
||||||
|
panic!("a closed store must never report LeadershipLost (review 004 item 9)")
|
||||||
|
}
|
||||||
|
other => {
|
||||||
|
panic!("the closed-store arm reports the closed Database classification, got {other:?}")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
drop_schema(&fx.admin, &fx.schema).await;
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Acceptance: the genuine mid-run leadership-loss arm keeps its
|
||||||
|
/// disposition — `Err(LeadershipLost)`. Deleting the leadership lock
|
||||||
|
/// row while the runner runs (the expired-and-re-acquired-elsewhere
|
||||||
|
/// shape — exclusion lapses silently at TTL expiry, ADR-008 §7) makes
|
||||||
|
/// the next loop-top renewal report the refused-lease outcome and the
|
||||||
|
/// loop returns the loss before any tick; the respawn recipe's arm is
|
||||||
|
/// unchanged by the classification. The SQLite twin's pin shape
|
||||||
|
/// (`mid_run_lease_loss_still_returns_leadership_lost`).
|
||||||
|
#[tokio::test(flavor = "multi_thread")]
|
||||||
|
async fn mid_run_lease_loss_still_returns_leadership_lost() {
|
||||||
|
let Some(fx) = Fixture::open(&instance_namer("so_lss")).await else {
|
||||||
|
eprintln!("skip: no harness server");
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
|
||||||
|
fx.store
|
||||||
|
.schedule("beat", "@every 1s", "lossq", json!(1), Default::default())
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let runner_store = open_store(&harness_dsn().unwrap(), test_opts(&fx.schema))
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
let runner = tokio::spawn(async move { runner_store.run_schedules(StopToken::new()).await });
|
||||||
|
|
||||||
|
// Demonstrably mid-run: leadership held, one boundary fired.
|
||||||
|
let deadline = tokio::time::Instant::now() + Duration::from_secs(20);
|
||||||
|
loop {
|
||||||
|
let fired: i64 = fx
|
||||||
|
.admin
|
||||||
|
.query_one(
|
||||||
|
&format!(
|
||||||
|
"SELECT count(*) FROM {} WHERE queue = 'lossq'",
|
||||||
|
crate::schema::QualifiedTable {
|
||||||
|
schema: &fx.schema,
|
||||||
|
name: crate::schema::tables::JOB,
|
||||||
|
}
|
||||||
|
),
|
||||||
|
&[],
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap()
|
||||||
|
.get(0);
|
||||||
|
if fired >= 1 {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
assert!(
|
||||||
|
tokio::time::Instant::now() < deadline,
|
||||||
|
"the runner never fired the first boundary"
|
||||||
|
);
|
||||||
|
tokio::time::sleep(Duration::from_millis(200)).await;
|
||||||
|
}
|
||||||
|
|
||||||
|
// The rogue deletion: the runner's lease row vanishes.
|
||||||
|
let deleted = fx
|
||||||
|
.admin
|
||||||
|
.execute(
|
||||||
|
&format!(
|
||||||
|
"DELETE FROM {} WHERE name = $1",
|
||||||
|
crate::schema::QualifiedTable {
|
||||||
|
schema: &fx.schema,
|
||||||
|
name: crate::schema::tables::LOCKS,
|
||||||
|
}
|
||||||
|
),
|
||||||
|
&[&crate::scheduler::SCHEDULER_LEADER_LOCK],
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
deleted, 1,
|
||||||
|
"the leadership lock row must exist to be deleted mid-run"
|
||||||
|
);
|
||||||
|
|
||||||
|
let outcome = tokio::time::timeout(Duration::from_secs(10), runner)
|
||||||
|
.await
|
||||||
|
.expect("the lease-loss exit must terminate promptly")
|
||||||
|
.unwrap();
|
||||||
|
assert!(
|
||||||
|
matches!(outcome, Err(Error::LeadershipLost)),
|
||||||
|
"a deleted lease mid-run must return LeadershipLost: {outcome:?}"
|
||||||
|
);
|
||||||
|
|
||||||
|
drop_schema(&fx.admin, &fx.schema).await;
|
||||||
|
}
|
||||||
@@ -1,7 +1,7 @@
|
|||||||
---
|
---
|
||||||
id: pg-fix-scheduler-error-classification
|
id: pg-fix-scheduler-error-classification
|
||||||
name: Classify run_schedules' renew-failure arms on pg (the sqlite twin, review 004 item 9)
|
name: Classify run_schedules' renew-failure arms on pg (the sqlite twin, review 004 item 9)
|
||||||
status: pending
|
status: completed
|
||||||
depends_on: []
|
depends_on: []
|
||||||
scope: narrow
|
scope: narrow
|
||||||
risk: low
|
risk: low
|
||||||
@@ -77,8 +77,56 @@ a mid-run close exits with the closed `Database` classification, never
|
|||||||
|
|
||||||
## Notes
|
## Notes
|
||||||
|
|
||||||
> To be filled by implementation agent
|
- **No discrepancy with the decisions of record**: the sqlite task's
|
||||||
|
arm set implemented verbatim — `Ok(false)` keeps
|
||||||
|
`Err(LeadershipLost)` return-before-tick (the respawn arm),
|
||||||
|
`Err(e)` propagates unchanged (no new variant, no re-deciding; the
|
||||||
|
`Closed`-stays-wake-receiver posture untouched).
|
||||||
|
- **Harness server (was the recorded blocker)**: the `pglo-poc`
|
||||||
|
container was up on :15432; the standard env
|
||||||
|
(`ALKSTORE_PG_HOST/PORT/USER/PASSWORD/DB=…/15432/postgres/poc/blobs`)
|
||||||
|
ran the whole pg suite live — nothing was held back to the
|
||||||
|
server-less skip posture.
|
||||||
|
- **Engine note (how the closed arm arrives)**: pg's closed-store
|
||||||
|
classification is *proactive* — `PgLockHandle::renew` checks the
|
||||||
|
store's shared `closed` flag before any SQL (locked.rs closed-sign
|
||||||
|
first, the sqlite twin's shape; pinned by pg lock_tests) — so the
|
||||||
|
test drives it through the fixture store's own `EngineCtx`
|
||||||
|
(`run_schedules` runs on `fx.store.engine_ctx()`, sharing the
|
||||||
|
closed flag and pool; `close()` then flips the runner's flag), not
|
||||||
|
through a second store instance the way the sqlite pin's
|
||||||
|
same-file DB works. The genuine-loss arm drives through the
|
||||||
|
runner-store pattern (second `open_store` on the same dsn/schema)
|
||||||
|
with an admin-client `DELETE` of the lock row — the twin's rogue
|
||||||
|
deletion shape.
|
||||||
|
- **Doc alignment**: the pg scheduler's module doc got the arm-set
|
||||||
|
sentence (the pg-specific text this criterion calls out) — the
|
||||||
|
SQLite twin's wording (`Ok(false)` = the loss / `Err(e)` =
|
||||||
|
opaque-propagate, review 004 item 9). ADR-009 §1 and
|
||||||
|
core-contract.md §Errors verified engine-neutral as landed
|
||||||
|
(2026-10-11, dated annotation; no engine names) — cited, not
|
||||||
|
touched; engine-postgres.md's scheduler row doesn't describe the
|
||||||
|
renew arm, so it needs nothing.
|
||||||
|
|
||||||
## Summary
|
## Summary
|
||||||
|
|
||||||
> To be filled on completion
|
The pg leader loop's top-of-iteration renew no longer flattens a
|
||||||
|
failed renewal into `LeadershipLost` (`scheduler.rs` — the
|
||||||
|
`unwrap_or(false)` the sqlite task recorded): `Ok(false)` keeps the
|
||||||
|
`Err(LeadershipLost)` respawn arm unchanged, and a failed renewal
|
||||||
|
propagates its opaque `Database` classification unchanged (the
|
||||||
|
closed-store arm arrives pre-classified — the engine-wide "the store
|
||||||
|
is closed" source, the proactive closed-sign posture) — respawning a
|
||||||
|
leader against a dead store is no longer the pg outcome either; the
|
||||||
|
sqlite/pg twins now classify identically. Two acceptance tests pin
|
||||||
|
the arms against the harness server, modeled on the sqlite pair:
|
||||||
|
`closed_store_run_reports_the_closed_classification_not_leadership_
|
||||||
|
lost` (the runner runs on the fixture's own EngineCtx and the store
|
||||||
|
closes mid-run — the exit is the closed `Database` classification,
|
||||||
|
never `LeadershipLost`) and `mid_run_lease_loss_still_returns_
|
||||||
|
leadership_lost` (a deleted lease row mid-run still returns the
|
||||||
|
loss, the respawn arm unchanged). Verified: full
|
||||||
|
`cargo test -p alkstore-postgres` green on the live harness server
|
||||||
|
(133 lib + 25 contract suite + 9 schema tests), workspace gates green
|
||||||
|
server-less, clippy `-D warnings` clean, `cargo fmt --check` clean,
|
||||||
|
doc gate clean, fuzz corpus replay (6/6) green.
|
||||||
Reference in new issue
Block a user