fix(slack): back off between failed outbox deliveries (audit H4) - #428
Merged
Conversation
`mark_failed` incremented `attempts` but never moved `deliver_after`, which `claim_pending` filters on. A failed row was therefore reclaimed on the very next 5-second tick, so all ten attempts were spent in about a minute — after which the row is stamped `gave_up_at` and never retried again. That made the retry budget a count of ticks rather than a span of time: any Slack outage past ~a minute permanently dropped every incident open and resolve enqueued during it, and operators were never paged for them once Slack recovered. `mark_failed` now advances `deliver_after` by `retry_backoff(attempts)` — 15s doubling to a 15-minute cap — which spends the same ten attempts over about an hour, enough to ride out a routine Slack incident. It also returns the new attempt count, and the drainer logs the wait alongside its retry warning. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SGfH1cdFKPnKpM7ytRThft
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes H4 (high) from the audit in #370.
The bug
SlackOutbox::mark_failedincrementedattemptsand recorded the error, but never toucheddeliver_after— the columnclaim_pendingfilters on. A failed row was immediately claimable again, and the drainer ticks every 5 seconds, so allMAX_ATTEMPTS = 10were burnt in roughly 45–90 seconds. After that the row is stampedgave_up_atand never retried.The retry budget was effectively a count of ticks rather than a span of time. A Slack outage of a few minutes — a routine occurrence — permanently dropped every incident open and resolve enqueued during it, and nobody was paged for them once Slack came back.
(The give-up escalation goes out through the same failing path, so it's dropped the same way. That's a separate concern and not addressed here.)
The fix
mark_failednow advancesdeliver_afterbyretry_backoff(attempts): 15s, doubling per attempt, capped at 15 minutes. The same ten attempts now span about an hour (15s, 30s, 1m, 2m, 4m, 8m, then 15m a few times), which rides out a routine Slack incident.The schedule is a plain function in
database::slack_outbox, so it's unit-testable and the drainer can log the wait alongside its retry warning.mark_failedalso returns the new attempt count.Nothing about give-up changes: a row that exhausts its attempts is still abandoned, it just takes an hour of real outage to get there instead of a minute.
Tests
mark_failed_holds_the_row_back_for_its_backoff— aftermark_failedthe row is not reclaimable on the next tick, and is reclaimable once its backoff has elapsed (failure is not give-up).retry_backoff_doubles_then_holds_at_the_cap— the schedule itself, including the assertion that the ten-attempt budget spans at least 45 minutes.Generated by Claude Code