GKaszewski/movies-diary
1
0
Fork 0
Code Issues 9 Pull Requests Actions Packages Projects Releases Wiki Activity

fix: defer blocked deliveries, claim owner token

Browse Source
This commit is contained in:
Gabriel Kaszewski
2026-10-26 00:33:29 +01:00
parent 53892847f2
commit 0236a488fa
13 changed files with 528 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
crates/application/src/worker.rs
View File

@@ -1,4 +1,5 @@
use std::sync::Arc;
use std::time::Duration;
use domain::{
errors::DomainError,
@@ -88,6 +89,10 @@ impl WorkerService {
}
}
/// Margin added to a claim's remaining lease before a blocked delivery is
/// retried, so the retry lands after the lease has lapsed rather than on it.
const DEFER_MARGIN: Duration = Duration::from_secs(5);
async fn dispatch(
handlers: Arc<Vec<Arc<dyn EventHandler>>>,
ledger: Arc<dyn ProcessedEventLedger>,
@@ -95,21 +100,21 @@ async fn dispatch(
) {
let id = envelope.id().copied();
let mut any_transient = false;
// Longest wait among handlers blocked behind another delivery's live claim.
let mut blocked_for: Option<Duration> = None;
for handler in handlers.iter() {
let name = handler.name();
let mut claim = None;
if let Some(id) = &id {
match ledger.begin(id, name).await {
Ok(Begin::Run) => {}
Ok(Begin::Run(token)) => claim = Some(token),
Ok(Begin::AlreadyDone) => {
tracing::debug!(handler = name, "already processed, skipping");
continue;
}
Ok(Begin::InProgressElsewhere) => {
tracing::debug!(
handler = name,
"running in another delivery, retrying later"
);
any_transient = true;
Ok(Begin::InProgressElsewhere { retry_after }) => {
tracing::debug!(handler = name, "running in another delivery, deferring");
blocked_for = Some(blocked_for.map_or(retry_after, |b| b.max(retry_after)));
continue;
}
Err(e) => {
@@ -123,8 +128,8 @@ async fn dispatch(
let result = handler.handle(&envelope.event).await;
match &result {
Ok(()) => {
if let Some(id) = &id
&& let Err(e) = ledger.complete(id, name).await
if let (Some(id), Some(token)) = (&id, &claim)
&& let Err(e) = ledger.complete(id, name, token).await
{
// The work is done; failing the delivery would only repeat it.
// The claim lapses after its lease and the handler may run once more.
@@ -135,8 +140,8 @@ async fn dispatch(
// Permanent failures are released too, not recorded: if a sibling
// handler's transient failure redelivers this event, a handler that
// failed permanently runs (and fails) again.
if let Some(id) = &id
&& let Err(le) = ledger.release(id, name).await
if let (Some(id), Some(token)) = (&id, &claim)
&& let Err(le) = ledger.release(id, name, token).await
{
tracing::warn!(handler = name, "releasing ledger claim failed: {le}");
}
@@ -152,7 +157,15 @@ async fn dispatch(
}
}
}
if any_transient {
// A blocked handler cannot be retried before its claim is done or lapsed,
// so wait that long without spending a retry attempt; any transient failure
// in the same delivery is retried then too. A plain nack would burn the
// retry budget (and dead-letter) long before a crashed claim's lease ends.
if let Some(wait) = blocked_for {
if let Err(e) = envelope.defer(wait + DEFER_MARGIN).await {
tracing::error!("defer failed: {e}");
}
} else if any_transient {
if let Err(e) = envelope.nack().await {
tracing::error!("nack failed: {e}");
}

5
crates/application/tests/jobs/processed_events_cleanup_test.rs
View File

@@ -26,7 +26,10 @@ async fn removes_only_rows_older_than_the_retention_window() {
.unwrap();
use domain::ports::{Begin, ProcessedEventLedger};
assert_eq!(store.begin(&stale, "h").await.unwrap(), Begin::Run);
assert!(matches!(
store.begin(&stale, "h").await.unwrap(),
Begin::Run(_)
));
assert_eq!(store.begin(&fresh, "h").await.unwrap(), Begin::AlreadyDone);
}

109
crates/application/tests/worker/worker_test.rs
View File

@@ -28,6 +28,9 @@ impl AckHandle for TrackingAck {
*self.nacked.lock().unwrap() = true;
Ok(())
}
async fn defer(&self, _: std::time::Duration) -> Result<(), DomainError> {
Ok(())
}
}
struct VecConsumer {
@@ -380,9 +383,10 @@ struct RedeliveringConsumer {
event: DomainEvent,
times: usize,
nacks: Arc<Mutex<u32>>,
defers: Arc<Mutex<Vec<std::time::Duration>>>,
}
struct CountNack(Arc<Mutex<u32>>);
struct CountNack(Arc<Mutex<u32>>, Arc<Mutex<Vec<std::time::Duration>>>);
#[async_trait]
impl AckHandle for CountNack {
@@ -393,16 +397,24 @@ impl AckHandle for CountNack {
*self.0.lock().unwrap() += 1;
Ok(())
}
async fn defer(&self, delay: std::time::Duration) -> Result<(), DomainError> {
self.1.lock().unwrap().push(delay);
Ok(())
}
}
impl EventConsumer for RedeliveringConsumer {
fn consume(&self) -> BoxStream<'_, Result<EventEnvelope, DomainError>> {
let nacks = Arc::clone(&self.nacks);
let defers = Arc::clone(&self.defers);
let id = self.id;
let event = self.event.clone();
Box::pin(stream::iter(0..self.times).then(move |_| {
let envelope =
EventEnvelope::new(id, event.clone(), Box::new(CountNack(Arc::clone(&nacks))));
let envelope = EventEnvelope::new(
id,
event.clone(),
Box::new(CountNack(Arc::clone(&nacks), Arc::clone(&defers))),
);
async move {
// Let the previous delivery's handlers finish first: a real
// broker redelivers only after the nack.
@@ -423,6 +435,7 @@ async fn a_redelivered_event_skips_handlers_that_already_succeeded() {
event: movie_discovered(),
times: 2,
nacks: Arc::clone(&nacks),
defers: Default::default(),
};
WorkerService::new(
@@ -455,6 +468,7 @@ async fn events_without_id_run_every_time() {
event: movie_discovered(),
times: 2,
nacks: Arc::new(Mutex::new(0)),
defers: Default::default(),
};
WorkerService::new(Arc::new(consumer), vec![handler], InMemoryStore::new())
@@ -485,13 +499,18 @@ async fn a_handler_already_running_elsewhere_is_not_run_concurrently() {
let store = InMemoryStore::new();
let id = domain::events::EventId::new();
// Another delivery holds a live claim on this (event, handler).
assert_eq!(store.begin(&id, "slow").await.unwrap(), Begin::Run);
assert!(matches!(
store.begin(&id, "slow").await.unwrap(),
Begin::Run(_)
));
let nacks = Arc::new(Mutex::new(0));
let defers: Arc<Mutex<Vec<std::time::Duration>>> = Default::default();
let consumer = RedeliveringConsumer {
id: Some(id),
event: movie_discovered(),
times: 1,
nacks: Arc::clone(&nacks),
defers: Arc::clone(&defers),
};
WorkerService::new(Arc::new(consumer), vec![handler], store)
@@ -504,7 +523,85 @@ async fn a_handler_already_running_elsewhere_is_not_run_concurrently() {
0,
"must not run beside the claim holder"
);
assert_eq!(*nacks.lock().unwrap(), 1, "delivery is retried later");
assert_eq!(
*nacks.lock().unwrap(),
0,
"a blocked delivery is not a failure"
);
assert_eq!(defers.lock().unwrap().len(), 1, "delivery is retried later");
}
#[tokio::test]
async fn a_delivery_blocked_by_a_live_claim_is_deferred_without_consuming_an_attempt() {
use domain::ports::{Begin, HANDLER_LEASE_SECONDS, ProcessedEventLedger};
let (handler, calls) = CountingHandler::new("slow", 0);
let store = InMemoryStore::new();
let id = domain::events::EventId::new();
assert!(matches!(
store.begin(&id, "slow").await.unwrap(),
Begin::Run(_)
));
let nacks = Arc::new(Mutex::new(0));
let defers: Arc<Mutex<Vec<std::time::Duration>>> = Default::default();
let consumer = RedeliveringConsumer {
id: Some(id),
event: movie_discovered(),
times: 1,
nacks: Arc::clone(&nacks),
defers: Arc::clone(&defers),
};
WorkerService::new(Arc::new(consumer), vec![handler], store)
.unwrap()
.run(tokio::sync::watch::channel(false).1)
.await;
assert_eq!(*calls.lock().unwrap(), 0);
assert_eq!(*nacks.lock().unwrap(), 0, "a nack would consume an attempt");
let defers = defers.lock().unwrap();
assert_eq!(defers.len(), 1);
// Retried only once the claim's lease has run out (plus a margin).
let lease = std::time::Duration::from_secs(HANDLER_LEASE_SECONDS as u64);
assert!(
defers[0] > lease - std::time::Duration::from_secs(5),
"{:?}",
defers[0]
);
assert!(
defers[0] <= lease + std::time::Duration::from_secs(6),
"{:?}",
defers[0]
);
}
#[tokio::test]
async fn a_lapsed_owner_cannot_release_a_claim_taken_over_by_another() {
use domain::ports::{Begin, HANDLER_LEASE_SECONDS, ProcessedEventLedger};
let store = InMemoryStore::new();
let id = domain::events::EventId::new();
store.seed_processed_event(
&id,
"h",
false,
chrono::Utc::now().naive_utc() - chrono::Duration::seconds(HANDLER_LEASE_SECONDS + 1),
);
let Begin::Run(lapsed) = store.begin(&id, "h").await.unwrap() else {
panic!("stale claim should be taken over");
};
// Simulate: the original owner's token is from before the takeover.
let original = domain::ports::ClaimToken::new("2020-01-01T00:00:00.000Z".into());
assert_ne!(original, lapsed);
store.release(&id, "h", &original).await.unwrap();
store.complete(&id, "h", &original).await.unwrap();
assert!(
matches!(
store.begin(&id, "h").await.unwrap(),
Begin::InProgressElsewhere { .. }
),
"the new owner's claim must survive the old owner's release and complete"
);
}
#[tokio::test]
@@ -525,6 +622,7 @@ async fn a_stale_in_progress_claim_is_taken_over() {
event: movie_discovered(),
times: 1,
nacks: Arc::clone(&nacks),
defers: Default::default(),
};
WorkerService::new(Arc::new(consumer), vec![handler], store)
@@ -546,6 +644,7 @@ async fn a_failed_handler_releases_its_claim_so_redelivery_retries() {
event: movie_discovered(),
times: 3,
nacks: Arc::clone(&nacks),
defers: Default::default(),
};
WorkerService::new(Arc::new(consumer), vec![handler], store.clone())