Use one recovery routine at start and on every sweep tick (closes #542)
check / check (push) Waiting to run

Start-up recovery and the 60-second sweep were two parallel sets of
functions doing the same job. recoverDeliveries now does it, run once at
start and then on every tick from one goroutine. A retrying delivery is
scheduled after what is left of its backoff, now if nothing is left, so
backoffElapsed is gone. The run at start still takes every pending
delivery and opens a webhook database whose file is missing; a tick
still takes only pending deliveries untouched for 15 minutes, at most
500, and skips a missing file. A retrying delivery is taken before its
event is read. The README describes one routine.

Model: opus-5-5
This commit was merged in pull request #553.
This commit is contained in:
2026-10-15 21:57:05 +02:00
parent 0cdda43e6c
commit ded39aa158
16 changed files with 705 additions and 594 deletions
+44 -32
View File
@@ -1346,11 +1346,11 @@ webhooker solves this by acting as a durable intermediary:
**That guarantee is at-least-once, not exactly-once.** When a send reaches
its target but the write recording that outcome fails, the delivery is
deliberately left in a recoverable state rather than marked done — losing a
delivery is the worse failure — so the pending sweep picks it up about
fifteen minutes later, or the next restart does, and the target receives a
payload it already got. A send still in flight when the service stops is
left the same way: its attempt is not recorded, and the next start sends it
again (see Shutdown). webhooker adds no delivery identifier of its own to an
delivery is the worse failure — so the sweep picks it up about fifteen
minutes later, or the next restart does, and the target receives a payload
it already got. A send still in flight when the service stops is left the
same way: its attempt is not recorded, and the next start sends it again
(see Shutdown). webhooker adds no delivery identifier of its own to an
outbound request, so **make your receiver idempotent** against whatever the
payload itself carries.
@@ -1865,15 +1865,15 @@ These indexes on the per-webhook event databases are created by
`internal/db/migrations/events/001_schema.sql`, and declared in the model tags
to match:
| Table | Columns | Serves |
| ------------------ | ------------------------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `deliveries` | `status`, `deleted_at`, `finished_at`, `target_id` | Startup recovery, the retry and pending sweeps every 60 seconds and the queue-depth sampler every 30 seconds, which select deliveries by status, the webhook page's statistics and target list and the webhook list, which count each target's deliveries by status and when they finished, and the event log, which lists and counts the events with a failed delivery or one pending or retrying |
| `deliveries` | `event_id`, `deleted_at` | The event log, which loads each event's deliveries, and retention, which counts and deletes the deliveries of expired events |
| `delivery_results` | `delivery_id`, `deleted_at` | The event log, which loads the attempts of a page's deliveries, and retention, which deletes the attempts of expired events |
| `events` | `deleted_at`, `created_at` | The webhook page's statistics, which count recent events |
| `events` | `resubmitted_from_id`, `deleted_at` | The event log, which counts the events resubmitted from each event on a page |
| `events` | `entrypoint_id`, `deleted_at`, `resubmitted_from_id`, `created_at` | The webhook page's entrypoint list, which counts the events that arrived on each entrypoint's URL within the retention period |
| `events` | `created_at` | Retention, which selects expired events by age |
| Table | Columns | Serves |
| ------------------ | ------------------------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `deliveries` | `status`, `deleted_at`, `finished_at`, `target_id` | Delivery recovery at start and every 60 seconds and the queue-depth sampler every 30 seconds, which select deliveries by status, the webhook page's statistics and target list and the webhook list, which count each target's deliveries by status and when they finished, and the event log, which lists and counts the events with a failed delivery or one pending or retrying |
| `deliveries` | `event_id`, `deleted_at` | The event log, which loads each event's deliveries, and retention, which counts and deletes the deliveries of expired events |
| `delivery_results` | `delivery_id`, `deleted_at` | The event log, which loads the attempts of a page's deliveries, and retention, which deletes the attempts of expired events |
| `events` | `deleted_at`, `created_at` | The webhook page's statistics, which count recent events |
| `events` | `resubmitted_from_id`, `deleted_at` | The event log, which counts the events resubmitted from each event on a page |
| `events` | `entrypoint_id`, `deleted_at`, `resubmitted_from_id`, `created_at` | The webhook page's entrypoint list, which counts the events that arrived on each entrypoint's URL within the retention period |
| `events` | `created_at` | Retention, which selects expired events by age |
GORM's soft delete adds `deleted_at IS NULL` to these queries; retention leaves
it out. SQLite keeps no statistics on these tables, and without them it rates
@@ -2199,7 +2199,7 @@ preventing goroutine explosions regardless of queue depth.
Workers are the ONLY goroutines doing actual HTTP delivery.
- **Retry backpressure with DB fallback:** When a retry timer fires and the
retry channel is full, the timer is dropped — the delivery stays in `retrying`
status in the database. A periodic sweep (every 60s) scans for these
status in the database. The sweep (every 60s, see Recovery below) finds these
"orphaned" retries and re-queues them. No blocked goroutines, no unbounded
timer chains.
- **Bounded concurrency:** At most N deliveries (N = number of workers) are
@@ -2226,24 +2226,36 @@ This means:
that did not finish are still running — an unclean shutdown is reported rather
than hidden.
**Recovery paths:**
**Recovery:** one routine finds the deliveries the engine is not working on and
hands them back. It runs once when the engine starts, and then every 60 seconds;
that periodic run is the sweep. For every webhook it reads the `pending` and
`retrying` deliveries. One that already has a successful attempt is marked
`delivered` and not sent again. A `retrying` delivery gets a backoff timer for
what is left of its backoff, and is queued at once if nothing is left. A
`pending` delivery is queued on the delivery channel. A delivery the engine has
queued, is attempting, or holds a backoff timer for is left alone, at start as
on every sweep. This is what catches "orphaned" retries, whose in-memory timer
was dropped because the retry channel was full, and deliveries a stop or a crash
cut off: the database is the durable fallback that ensures no delivery is
permanently lost, even under extreme backpressure.
1. **Startup recovery:** When the engine starts, it scans all per-webhook
databases for `pending` and `retrying` deliveries. Pending deliveries are
sent to the delivery channel; retrying deliveries get backoff timers
scheduled.
2. **Periodic retry sweep (DB-mediated fallback):** Every 60 seconds the engine
scans for `retrying` deliveries whose backoff period has elapsed. This
catches "orphaned" retries — ones whose in-memory timer was dropped because
the retry channel was full. The database is the durable fallback that ensures
no retry is permanently lost, even under extreme backpressure.
The run at start and the sweep differ in two ways:
- At start every `pending` delivery is taken, however recent, so one a stop cut
off moments ago is sent again at once (see Shutdown). The sweep takes only
`pending` deliveries untouched for 15 minutes, at most 500 per webhook each
time, so a delivery whose outcome the database will not record is sent again
every 15 minutes rather than every minute.
- At start every webhook's database is opened, so that a missing one is reported
then (see Per-Webhook Event Databases). The sweep skips a webhook whose
database file is missing rather than create it.
**Changing a target's type does not migrate in-flight deliveries.** Only `http`
and `slack` targets own durable retries; `database` and `log` targets are
fire-and-forget and never produce a `retrying` delivery. If a target's `type` is
edited from a retrying type to a non-retrying (or unknown) one while one of its
deliveries is still `retrying`, both recovery paths above terminally mark that
delivery `failed` and record a `DeliveryResult` naming the current target type
deliveries is still `retrying`, the recovery routine above terminally marks that
delivery `failed` and records a `DeliveryResult` naming the current target type
as the reason, logging it at warn level. The delivery is not re-dispatched under
the new type — the operator never asked for that delivery — and the event itself
remains stored in the per-webhook event database. Nothing is lost: the delivery
@@ -2256,8 +2268,8 @@ re-inject it to every currently active target.
`http` and `slack` targets with `max_attempts` > 1 are protected by a
**per-target circuit breaker** that prevents hammering a down target with
repeated failed delivery attempts. The circuit breaker is in-memory only and
resets on restart (which is fine — startup recovery rescans the database
anyway).
resets on restart (which is fine — the recovery routine rescans the database at
start anyway).
**States:**
@@ -3336,9 +3348,9 @@ normal operation.
The delivery engine cancels a delivery request in flight. That attempt is not
recorded or counted in the metrics, and the delivery keeps its `pending` or
`retrying` status, so startup recovery sends it again on the next start (see
Recovery paths under Bounded Worker Pool). If the receiver already had it, it
receives it twice: the at-least-once duplicate described under Rationale.
`retrying` status, so recovery at the next start sends it again (see Recovery
under Bounded Worker Pool). If the receiver already had it, it receives it
twice: the at-least-once duplicate described under Rationale.
The HTTP drain budget is deliberately **shorter** than the sequence budget. Were
the two equal, a drain that used its whole budget would exhaust the sequence