mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(relay): claim reminders before publishing
The NIP-ER reminder scheduler published the reminder event to Redis first and only then claimed it (`claim_due_reminder`), treating a duplicate publish as harmless because subscribers dedup by event id. That is the old unsafe ordering: across N pods a due reminder could be published more than once, violating the claim-before-side-effect rule for periodic producers (every side effect must be claimed exactly once, not deduped after the fact). Rewire to claim-before-publish using the stamp-guarded primitives that were already built and wired into buzz-db but had zero callers: `claim_due_reminder_with_stamp` (event.rs:1186) and `release_due_reminder` (event.rs:1213). Each attempt mints a unique per-pod stamp; the scheduler claims first, publishes only on a winning claim (`Ok(true)`) and `continue`s on the loser (`Ok(false)`) so the loser never produces the side effect, and releases its own claim on publish failure via compare-and-clear so the reminder is redeliverable next tick. `events.delivered_at` is only ever read as a NULL/non-NULL sentinel (due-reminder query guard + partial index), never as a wall-clock value, so an opaque stamp is safe to store there. The unused convenience wrapper `claim_due_reminder` (seconds stamp) is left in place as public API; the scheduler no longer uses it. Tests (buzz-db, Postgres-backed, verified locally against the dev DB): - claim_due_reminder_is_won_by_exactly_one_of_two_racing_pods: two pods, two stamps, one reminder -> exactly one wins; the single winning claim is the proof of exactly one publish side effect. - release_due_reminder_rolls_back_only_the_matching_stamp: a wrong-stamp release is a no-op (cannot clear another pod's claim); a matching-stamp release makes the reminder reclaimable for retry. Co-authored-by: Tyler Longwell <tlongwell@block.xyz> Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
This commit is contained in:
co-authored by
Tyler Longwell
parent
c77f2c27d7
commit
6846a8f10c
@@ -1500,4 +1500,111 @@ mod tests {
|
||||
row.id == event_b.id.as_bytes() && row.community_id == community_b && row.host == host_b
|
||||
}));
|
||||
}
|
||||
|
||||
/// Two pods race to claim the same due reminder: exactly one wins. The
|
||||
/// scheduler publishes only on a winning claim (`Ok(true)`) and `continue`s
|
||||
/// on the loser (`Ok(false)`), so a single winning claim *is* the proof of
|
||||
/// exactly one publish side effect across N pods.
|
||||
#[tokio::test]
|
||||
#[ignore = "requires Postgres"]
|
||||
async fn claim_due_reminder_is_won_by_exactly_one_of_two_racing_pods() {
|
||||
let pool = setup_pool().await;
|
||||
let community = CommunityId::from_uuid(make_test_community(&pool).await);
|
||||
let not_before = Utc::now().timestamp() - 1;
|
||||
let keys = Keys::generate();
|
||||
let event = EventBuilder::new(Kind::Custom(KIND_EVENT_REMINDER as u16), "due")
|
||||
.tags([
|
||||
Tag::parse(["d", "due-reminder-claim-race"]).unwrap(),
|
||||
Tag::parse(["not_before", ¬_before.to_string()]).unwrap(),
|
||||
])
|
||||
.sign_with_keys(&keys)
|
||||
.expect("sign reminder");
|
||||
insert_event(&pool, community, &event, None)
|
||||
.await
|
||||
.expect("insert reminder");
|
||||
|
||||
let id = event.id.as_bytes().to_vec();
|
||||
let created_at = event.created_at.as_secs() as i64;
|
||||
let created_at = chrono::DateTime::from_timestamp(created_at, 0).expect("created_at");
|
||||
|
||||
// Two pods, two distinct per-attempt stamps, same reminder.
|
||||
let stamp_p1: i64 = 0x1111_1111_1111_1111;
|
||||
let stamp_p2: i64 = 0x2222_2222_2222_2222;
|
||||
let won_p1 = claim_due_reminder_with_stamp(&pool, &id, created_at, stamp_p1)
|
||||
.await
|
||||
.expect("p1 claim");
|
||||
let won_p2 = claim_due_reminder_with_stamp(&pool, &id, created_at, stamp_p2)
|
||||
.await
|
||||
.expect("p2 claim");
|
||||
|
||||
assert!(
|
||||
won_p1 ^ won_p2,
|
||||
"exactly one pod must win the claim (p1={won_p1}, p2={won_p2}) — \
|
||||
the loser never reaches the publish side effect"
|
||||
);
|
||||
}
|
||||
|
||||
/// A failed publish releases the claim so the reminder is redeliverable,
|
||||
/// and the compare-and-clear stamp guard prevents one pod from rolling back
|
||||
/// another pod's claim.
|
||||
#[tokio::test]
|
||||
#[ignore = "requires Postgres"]
|
||||
async fn release_due_reminder_rolls_back_only_the_matching_stamp() {
|
||||
let pool = setup_pool().await;
|
||||
let community = CommunityId::from_uuid(make_test_community(&pool).await);
|
||||
let not_before = Utc::now().timestamp() - 1;
|
||||
let keys = Keys::generate();
|
||||
let event = EventBuilder::new(Kind::Custom(KIND_EVENT_REMINDER as u16), "due")
|
||||
.tags([
|
||||
Tag::parse(["d", "due-reminder-release"]).unwrap(),
|
||||
Tag::parse(["not_before", ¬_before.to_string()]).unwrap(),
|
||||
])
|
||||
.sign_with_keys(&keys)
|
||||
.expect("sign reminder");
|
||||
insert_event(&pool, community, &event, None)
|
||||
.await
|
||||
.expect("insert reminder");
|
||||
|
||||
let id = event.id.as_bytes().to_vec();
|
||||
let created_at = event.created_at.as_secs() as i64;
|
||||
let created_at = chrono::DateTime::from_timestamp(created_at, 0).expect("created_at");
|
||||
let stamp: i64 = 0x3333_3333_3333_3333;
|
||||
|
||||
assert!(
|
||||
claim_due_reminder_with_stamp(&pool, &id, created_at, stamp)
|
||||
.await
|
||||
.expect("claim"),
|
||||
"first claim wins"
|
||||
);
|
||||
|
||||
// A release with the *wrong* stamp must be a no-op (does not clear
|
||||
// another pod's claim).
|
||||
assert!(
|
||||
!release_due_reminder(&pool, &id, created_at, stamp ^ 0xFFFF)
|
||||
.await
|
||||
.expect("wrong-stamp release"),
|
||||
"release with a non-matching stamp must not clear the claim"
|
||||
);
|
||||
assert!(
|
||||
!claim_due_reminder_with_stamp(&pool, &id, created_at, stamp)
|
||||
.await
|
||||
.expect("re-claim after no-op release"),
|
||||
"reminder must still be claimed after a no-op release"
|
||||
);
|
||||
|
||||
// The matching-stamp release rolls the claim back; the reminder is
|
||||
// redeliverable and a subsequent claim wins again.
|
||||
assert!(
|
||||
release_due_reminder(&pool, &id, created_at, stamp)
|
||||
.await
|
||||
.expect("matching-stamp release"),
|
||||
"release with the claiming stamp must clear the claim"
|
||||
);
|
||||
assert!(
|
||||
claim_due_reminder_with_stamp(&pool, &id, created_at, stamp)
|
||||
.await
|
||||
.expect("re-claim after release"),
|
||||
"released reminder must be reclaimable for retry"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -515,14 +515,45 @@ async fn main() -> anyhow::Result<()> {
|
||||
info!(count = due.len(), "Reminder scheduler: due reminders found");
|
||||
|
||||
for reminder in due {
|
||||
// Publish first, then claim. If publish fails the reminder
|
||||
// stays unclaimed and will be retried next tick. If claim
|
||||
// fails after a successful publish, duplicate fan-out on the
|
||||
// next tick is harmless (subscribers dedup by event ID).
|
||||
// Claim before side effect (§5c: claim-before-publish). A
|
||||
// unique per-attempt stamp lets a failed publish roll back
|
||||
// exactly this pod's claim via compare-and-clear, without a
|
||||
// racing pod's later claim being clobbered. `delivered_at`
|
||||
// is only ever read as a NULL/non-NULL sentinel (the
|
||||
// due-reminder query guard and the partial index), never as
|
||||
// a wall-clock value, so an opaque stamp is safe to store.
|
||||
let reminder_tenant = buzz_core::tenant::TenantContext::resolved(
|
||||
reminder.community_id,
|
||||
reminder.host.clone(),
|
||||
);
|
||||
let delivery_stamp = chrono::Utc::now()
|
||||
.timestamp_nanos_opt()
|
||||
.unwrap_or_else(|| chrono::Utc::now().timestamp())
|
||||
^ rand::random::<i64>();
|
||||
|
||||
match scheduler_state
|
||||
.db
|
||||
.claim_due_reminder_with_stamp(
|
||||
&reminder.id,
|
||||
reminder.created_at,
|
||||
delivery_stamp,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(true) => {} // We won the claim — proceed to publish.
|
||||
Ok(false) => continue, // Another pod claimed it; no side effect here.
|
||||
Err(e) => {
|
||||
warn!(
|
||||
event_id = hex::encode(&reminder.id),
|
||||
"Reminder scheduler: claim failed, skipping publish: {e}"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
// Publish the single side effect. On failure, release our
|
||||
// claim so the next tick (this pod or another) can retry —
|
||||
// the stamp guard ensures we only clear our own claim.
|
||||
if let Err(e) = scheduler_state
|
||||
.pubsub
|
||||
.publish_event(
|
||||
@@ -534,23 +565,21 @@ async fn main() -> anyhow::Result<()> {
|
||||
{
|
||||
error!(
|
||||
event_id = hex::encode(&reminder.id),
|
||||
"Reminder scheduler: Redis publish failed, skipping claim: {e}"
|
||||
"Reminder scheduler: Redis publish failed after claim, releasing: {e}"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
// Atomic cross-pod claim — only the winner marks it delivered.
|
||||
match scheduler_state
|
||||
.db
|
||||
.claim_due_reminder(&reminder.id, reminder.created_at)
|
||||
.await
|
||||
{
|
||||
Ok(true) => {}
|
||||
Ok(false) => {} // Another pod claimed it; duplicate publish is harmless.
|
||||
Err(e) => {
|
||||
if let Err(release_err) = scheduler_state
|
||||
.db
|
||||
.release_due_reminder(
|
||||
&reminder.id,
|
||||
reminder.created_at,
|
||||
delivery_stamp,
|
||||
)
|
||||
.await
|
||||
{
|
||||
warn!(
|
||||
event_id = hex::encode(&reminder.id),
|
||||
"Reminder scheduler: claim failed after publish (duplicate delivery possible): {e}"
|
||||
"Reminder scheduler: release after failed publish errored \
|
||||
(reminder stays claimed, will not retry): {release_err}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user