sneak/webhooker
1
0
Fork 0
Code Issues 6 Pull Requests Actions 56 Packages Projects Releases Wiki Activity

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

Browse Source 
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 is contained in:
clawbot
2026-10-15 19:05:14 +00:00
committed by sneak
parent 0cdda43e6c
commit 3c7b7ebf59
16 changed files with 704 additions and 593 deletions
Download Patch File Download Diff File Expand all files Collapse all files
README.md
+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
internal/database/event_tier_indexes_test.go
+3 -2
View File
@@ -79,8 +79,9 @@ func TestEventTierQueriesUseTheirIndexes(t *testing.T) {
byStatus := "idx_deliveries_status (status=? AND deleted_at=?)"
byEvent := "idx_deliveries_event_id (event_id=? AND deleted_at=?)"
// The delivery engine: recovery and the retry sweep, the sweep for
// stranded pending deliveries, and the queue depth count.
// The delivery engine: the recovery routine, which reads the
// retrying deliveries and, on a tick of the sweep, the pending ones
// untouched for a while; and the queue depth count.
assertPlanUses(t, db, dry.Where(
"status = ?", database.DeliveryStatusRetrying,
).Find(&deliveries), byStatus)
internal/database/retention.go
+3 -2
View File
@@ -184,8 +184,9 @@ func (r *RetentionReaper) sweep(ctx context.Context) {
wh := webhooks[i]
// A missing database has nothing to reap. Restart recovery
// reports a lost one (see WebhookDBManager.GetDB).
// A missing database has nothing to reap. The delivery
// engine's recovery routine reports a lost one at start (see
// WebhookDBManager.GetDB).
if !r.dbManager.DBExists(wh.ID) {
continue
}
internal/delivery/engine.go
+159 -375
View File
@@ -40,8 +40,9 @@ const (
// explosions regardless of queue depth.
defaultWorkers = 10
// retrySweepInterval is how often the periodic retry
// sweep runs.
// retrySweepInterval is how often the recovery routine runs
// once it has run at start. Each of those runs is a tick of
// the sweep.
retrySweepInterval = 60 * time.Second
// pendingSweepMinAge is how long a delivery must have sat
@@ -63,10 +64,10 @@ const (
pendingSweepMinAge = 15 * time.Minute
// pendingSweepBatch bounds how many stranded pending deliveries
// one sweep of one webhook re-dispatches. The sweep runs every
// retrySweepInterval, so a larger backlog drains across
// successive sweeps instead of arriving as one burst against a
// database that was already struggling to accept writes.
// one tick of the sweep re-dispatches for one webhook. The sweep
// ticks every retrySweepInterval, so a larger backlog drains
// across successive ticks instead of arriving as one burst against
// a database that was already struggling to accept writes.
pendingSweepBatch = 500
// MaxInlineBodySize is the maximum event body size that
@@ -151,7 +152,7 @@ type EngineParams struct {
// Engine processes queued deliveries in the background
// using a bounded worker pool architecture. It owns only
// the cross-target machinery: the worker pool, the queue
// and retry channels, restart recovery, and the persistence
// and retry channels, the recovery routine, and the persistence
// helpers and Scheduler that individual targets rely on.
// Each target type owns its own delivery, including retries,
// backoff, and circuit breaking.
@@ -190,7 +191,7 @@ type Engine struct {
archives *archive.Writers
// inflight is the set of deliveries this engine currently owns.
// Recovery and the sweeps re-dispatch only what it does not
// The recovery routine re-dispatches only what it does not
// hold. Held by value: its zero value works, so no constructor
// can leave it out. See inflight.go.
inflight inflightSet
@@ -342,8 +343,8 @@ func (e *Engine) registerHooks(lc fx.Lifecycle) {
})
}
// start launches the worker pool, restart recovery, and the
// periodic retry sweep.
// start launches the worker pool and the recovery routine, which
// runs now and then on every tick of the sweep (see runRecovery).
//
// Their context is derived from context.Background(), NOT from
// the fx OnStart hook context. The hook context carries fx's
@@ -366,11 +367,7 @@ func (e *Engine) start() {
e.wg.Add(1)
go e.recoverPending(ctx)
e.wg.Add(1)
go e.retrySweep(ctx)
go e.runRecovery(ctx)
e.wg.Add(1)
@@ -453,12 +450,6 @@ func (e *Engine) runTask(
run(ctx, task)
}
func (e *Engine) recoverPending(ctx context.Context) {
defer e.wg.Done()
e.recoverInFlight(ctx)
}
func (e *Engine) processNewTask(
ctx context.Context, task *Task,
) {
@@ -473,9 +464,10 @@ func (e *Engine) processNewTask(
return
}
// Restart recovery can send and release this delivery before the
// receiver's Notify queues it. Ownership cannot refuse a delivery
// nobody holds, so the row decides whether it still needs sending.
// The recovery routine's run at start can send and release this
// delivery before the receiver's Notify queues it. Ownership cannot
// refuse a delivery nobody holds, so the row decides whether it
// still needs sending.
row, err := e.loadDelivery(webhookDB, task.DeliveryID)
if err != nil {
e.log.Error(
@@ -605,9 +597,9 @@ func (e *Engine) processRetryTask(
// else on this path reads the target row. Without this check a
// deletion stops nothing: the timer keeps firing and keeps sending to
// the destination the operator removed, for the whole remaining
// backoff chain. Terminalising in the recovery and sweep paths alone
// is not enough, because those only see the delivery once nothing
// holds it in memory — which is to say after a restart.
// backoff chain. Terminalising in the recovery routine alone is not
// enough, because it only sees the delivery once nothing holds it in
// memory — which is to say after a restart.
//
// The worker already owns this delivery, so the terminal write happens
// here directly, exactly as a target's own Deliver fails one. Claiming
@@ -655,7 +647,47 @@ func (e *Engine) abandonRetryForMissingTarget(
return true
}
func (e *Engine) recoverInFlight(ctx context.Context) {
// runRecovery runs the recovery routine, recoverDeliveries, once when
// the engine starts and then on every tick of the sweep, every
// retrySweepInterval, until the engine stops.
func (e *Engine) runRecovery(ctx context.Context) {
defer e.wg.Done()
ticker := time.NewTicker(retrySweepInterval)
defer ticker.Stop()
e.recoverDeliveries(ctx, true)
for {
select {
case <-ctx.Done():
return
case <-ticker.C:
e.recoverDeliveries(ctx, false)
}
}
}
// recoverDeliveries is the engine's one recovery routine: atStart is
// true for its run when the engine starts and false on a tick of the
// sweep.
//
// For every webhook it finds the pending and retrying deliveries the
// engine is not working on, such as those a stop or a crash cut off, a
// full channel dropped, or a failed bookkeeping write left behind. One
// that already has a successful attempt is marked delivered. A retrying
// one is scheduled after what is left of its backoff, or now when
// nothing is left. A pending one goes to a worker now.
//
// Ownership keeps it off a delivery the engine is working on, has
// queued, or holds a retry timer for, the same at start as on a tick:
// every delivery it acts on is taken through the inflight set first.
// See inflight.go.
//
// A run at start differs from a tick in which webhooks it opens (see
// recoverWebhookDeliveries) and which pending deliveries it takes (see
// recoverPendingDeliveries).
func (e *Engine) recoverDeliveries(ctx context.Context, atStart bool) {
var webhookIDs []string
err := e.database.DB().
@@ -677,18 +709,27 @@ func (e *Engine) recoverInFlight(ctx context.Context) {
default:
}
// Opened even when its file is missing, so that a lost
// database is reported at start, not when the webhook next
// receives an event, which for a quiet webhook may be never.
e.recoverWebhookDeliveries(ctx, webhookID)
e.recoverWebhookDeliveries(ctx, webhookID, atStart)
}
}
// recoverWebhookDeliveries runs the recovery routine over one webhook.
//
// At start the webhook's database is opened even when its file is
// missing, so that a lost database is reported at start, not when the
// webhook next receives an event, which for a quiet webhook may be
// never. A tick leaves a missing file alone rather than create one: a
// webhook being created has its row saved a moment before its file,
// and a tick in between would report that file as lost.
func (e *Engine) recoverWebhookDeliveries(
ctx context.Context, webhookID string,
ctx context.Context, webhookID string, atStart bool,
) {
// The web interface is already serving, so the webhook may have
// been deleted since the list was read. Opening its database then
if !atStart && !e.dbManager.DBExists(webhookID) {
return
}
// The web interface is serving, so the webhook may have been
// deleted since the list was read. Opening its database then
// would create the file again after the delete removed it.
stillExists := func() (bool, error) {
var count int64
@@ -720,16 +761,18 @@ func (e *Engine) recoverWebhookDeliveries(
}
e.recoverPendingDeliveries(
ctx, webhookDB, webhookID,
ctx, webhookDB, webhookID, atStart,
)
e.recoverRetryingDeliveries(
webhookDB, webhookID,
ctx, webhookDB, webhookID,
)
}
// recoverRetryingDeliveries recovers every retrying delivery of one
// webhook, at start and on a tick alike.
func (e *Engine) recoverRetryingDeliveries(
webhookDB *gorm.DB, webhookID string,
ctx context.Context, webhookDB *gorm.DB, webhookID string,
) {
var retrying []database.Delivery
@@ -756,6 +799,12 @@ func (e *Engine) recoverRetryingDeliveries(
)
for i := range retrying {
select {
case <-ctx.Done():
return
default:
}
if _, ok := settled[retrying[i].ID]; ok {
continue
}
@@ -767,8 +816,9 @@ func (e *Engine) recoverRetryingDeliveries(
}
// recoverSingleRetry hands an orphaned retrying delivery back
// to its target to recompute the remaining backoff, then
// reschedules it. Targets that do not own durable retries
// to its target to work out what is left of its backoff, then
// schedules its next attempt after that, or now when nothing is
// left. Targets that do not own durable retries
// (fire-and-forget) never produce retrying deliveries, so a
// delivery found in that state has had its target's type
// changed underneath it and is terminally failed.
@@ -816,6 +866,19 @@ func (e *Engine) recoverSingleRetry(
webhookDB, d.ID, attemptNum,
)
// Taken before the event is read, so that a delivery the engine
// already holds, most often one waiting out its backoff on a
// timer, costs a tick no read of its body.
if !e.takeForRedispatch(
webhookDB, d.ID, database.DeliveryStatusRetrying,
) {
return
}
// ScheduleRetry takes a reference of its own before it returns,
// and that one keeps the delivery owned through the backoff.
defer e.inflight.release(d.ID)
event, err := e.loadEvent(webhookDB, d.EventID)
if err != nil {
e.log.Error(
@@ -829,13 +892,9 @@ func (e *Engine) recoverSingleRetry(
return
}
task := buildRecoveryTask(
e.ScheduleRetry(buildRecoveryTask(
d, webhookID, &event, &target, attemptNum+1,
)
if !e.rescheduleRecovered(webhookDB, task, remaining) {
return
}
), remaining)
e.log.Info(
"recovering retrying delivery",
@@ -846,28 +905,54 @@ func (e *Engine) recoverSingleRetry(
)
}
// recoverPendingDeliveries recovers the pending deliveries of one
// webhook: it marks delivered those that already have a successful
// attempt and sends the rest.
//
// A delivery is created pending and leaves that state only when its
// outcome is written, so a pending row the engine does not own is one
// it stopped working on before then: for example one whose bookkeeping
// write failed, whose attempt a stop cut off, that was still queued
// when the engine last stopped, whose task was dropped because the
// delivery channel was full (by Notify or by queueRecovered), or whose
// worker could not open the webhook's database.
//
// At start every pending delivery is taken, however recent, so one a
// stop cut off moments ago is sent again now. A tick takes only those
// untouched for pendingSweepMinAge, at most pendingSweepBatch of them.
// The age bound is not what keeps a tick off live work: a delivery
// waiting in deliveryCh is pending and arbitrarily old. Ownership is,
// checked per delivery in takeForRedispatch. See inflight.go.
func (e *Engine) recoverPendingDeliveries(
ctx context.Context,
webhookDB *gorm.DB,
webhookID string,
atStart bool,
) {
query := webhookDB.Where(
"status = ?", database.DeliveryStatusPending,
)
if !atStart {
query = query.
Where(
"updated_at < ?",
time.Now().Add(-pendingSweepMinAge),
).
Limit(pendingSweepBatch)
}
var deliveries []database.Delivery
// No Preload: event bodies are read one at a time in
// sendRecoveredDeliveries, and only for the deliveries actually
// being sent.
result := webhookDB.
Where(
"status = ?",
database.DeliveryStatusPending,
).
Find(&deliveries)
if result.Error != nil {
err := query.Find(&deliveries).Error
if err != nil {
e.log.Error(
"failed to query pending deliveries",
"failed to query pending deliveries for recovery",
"webhook_id", webhookID,
"error", result.Error,
"error", err,
)
return
@@ -883,21 +968,6 @@ func (e *Engine) recoverPendingDeliveries(
"count", len(deliveries),
)
e.recoverPendingBatch(
ctx, webhookDB, webhookID, deliveries,
)
}
// recoverPendingBatch settles every delivery in the batch that was
// already delivered, and re-dispatches only the rest. Both the
// restart-time recovery and the periodic sweep go through it, so a
// pending delivery is treated the same however it was found.
func (e *Engine) recoverPendingBatch(
ctx context.Context,
webhookDB *gorm.DB,
webhookID string,
deliveries []database.Delivery,
) {
targetMap := e.loadTargetMap(deliveries)
settled := e.reconcileDelivered(
@@ -925,11 +995,11 @@ func (e *Engine) recoverPendingBatch(
// wire I/O happened and was recorded, and all that is missing is the
// status.
//
// Every recovery path runs this, not only the pending one. A delivery
// abandoned at retrying can hold a successful result just as a pending
// one can — a second attempt that reached the receiver and whose status
// write then failed sits at retrying with success recorded — and
// re-sending it is the same duplicate.
// The recovery routine runs this for retrying deliveries as well as
// pending ones. A delivery abandoned at retrying can hold a successful
// result just as a pending one can — a second attempt that reached the
// receiver and whose status write then failed sits at retrying with
// success recorded — and re-sending it is the same duplicate.
//
// Deliveries whose result row itself never landed are not in the
// returned set and are re-sent, recorded as the further attempt they
@@ -1016,247 +1086,8 @@ func (e *Engine) reconcileDelivered(
return settled
}
func (e *Engine) retrySweep(ctx context.Context) {
defer e.wg.Done()
ticker := time.NewTicker(retrySweepInterval)
defer ticker.Stop()
for {
select {
case <-ctx.Done():
return
case <-ticker.C:
e.sweepOrphanedRetries(ctx)
}
}
}
func (e *Engine) sweepOrphanedRetries(
ctx context.Context,
) {
var webhookIDs []string
err := e.database.DB().
Model(&database.Webhook{}).
Pluck("id", &webhookIDs).Error
if err != nil {
e.log.Error(
"retry sweep: failed to query webhook IDs",
"error", err,
)
return
}
for _, webhookID := range webhookIDs {
select {
case <-ctx.Done():
return
default:
}
if !e.dbManager.DBExists(webhookID) {
continue
}
e.sweepWebhookRetries(ctx, webhookID)
}
}
func (e *Engine) sweepWebhookRetries(
ctx context.Context, webhookID string,
) {
webhookDB, err := e.dbManager.GetDB(webhookID)
if err != nil {
e.log.Error(
"retry sweep: failed to get webhook database",
"webhook_id", webhookID,
"error", err,
)
return
}
var retrying []database.Delivery
err = webhookDB.
Where(
"status = ?",
database.DeliveryStatusRetrying,
).
Find(&retrying).Error
if err != nil {
e.log.Error(
"retry sweep: "+
"failed to query retrying deliveries",
"webhook_id", webhookID,
"error", err,
)
return
}
settled := e.reconcileDelivered(
webhookDB, webhookID, retrying,
e.loadTargetMap(retrying),
)
for i := range retrying {
select {
case <-ctx.Done():
return
default:
}
if _, ok := settled[retrying[i].ID]; ok {
continue
}
e.sweepSingleRetry(
webhookDB, webhookID, &retrying[i],
)
}
e.sweepWebhookPending(ctx, webhookDB, webhookID)
}
// sweepWebhookPending recovers deliveries stranded at pending.
//
// A delivery is created pending and leaves that state only when its
// outcome is written, so a pending row the engine does not own is one
// whose bookkeeping write failed — the state that used to sit there
// until a restart, and then produce a duplicate at the receiver. The
// sweep gives it the same reconcile-then-dispatch treatment restart
// recovery gets, so it costs a minute rather than an operator
// noticing.
//
// What keeps the sweep off live work is ownership, checked per
// delivery in takeForRedispatch, not the age bound in this query.
// A delivery waiting in deliveryCh is pending and arbitrarily old —
// the channel holds 10000 tasks and 10 workers drain it — so
// reasoning from the row's age alone re-sends it. See inflight.go.
func (e *Engine) sweepWebhookPending(
ctx context.Context,
webhookDB *gorm.DB,
webhookID string,
) {
var pending []database.Delivery
err := webhookDB.
Where(
"status = ? AND updated_at < ?",
database.DeliveryStatusPending,
time.Now().Add(-pendingSweepMinAge),
).
Limit(pendingSweepBatch).
Find(&pending).Error
if err != nil {
e.log.Error(
"retry sweep: "+
"failed to query pending deliveries",
"webhook_id", webhookID,
"error", err,
)
return
}
if len(pending) == 0 {
return
}
e.log.Info(
"retry sweep: recovering stranded pending deliveries",
"webhook_id", webhookID,
"count", len(pending),
)
e.recoverPendingBatch(ctx, webhookDB, webhookID, pending)
}
// sweepSingleRetry re-enqueues an orphaned retrying delivery
// whose backoff window has elapsed, delegating the backoff
// decision to the delivery's target. A delivery whose target
// no longer owns durable retries is terminally failed.
func (e *Engine) sweepSingleRetry(
webhookDB *gorm.DB,
webhookID string,
d *database.Delivery,
) {
target, err := e.loadTarget(d.TargetID)
if err != nil {
// Deleted is terminal, unreadable is not; see
// recoverSingleRetry.
if errors.Is(err, gorm.ErrRecordNotFound) {
e.failMissingTarget(webhookDB, webhookID, d)
return
}
e.log.Error(
"retry sweep: failed to load target",
"delivery_id", d.ID,
"target_id", d.TargetID,
"error", err,
)
return
}
rs, ok := e.targets[target.Type].(rescheduler)
if !ok {
e.failUnretryableRetry(
webhookDB, webhookID, d, &target,
)
return
}
attemptNum := e.countAttempts(webhookDB, d.ID)
if !rs.backoffElapsed(
webhookDB, d.ID, attemptNum,
) {
return
}
event, err := e.loadEvent(webhookDB, d.EventID)
if err != nil {
e.log.Error(
"retry sweep: failed to load event",
"delivery_id", d.ID,
"event_id", d.EventID,
"error", err,
)
return
}
task := buildRecoveryTask(
d, webhookID, &event, &target, attemptNum+1,
)
if !e.redispatch(
e.retryCh, webhookDB, task,
database.DeliveryStatusRetrying,
) {
return
}
e.log.Info(
"retry sweep: "+
"recovered orphaned retrying delivery",
"delivery_id", d.ID,
"webhook_id", webhookID,
"attempt", attemptNum+1,
)
}
// failUnretryableRetry terminally fails an orphaned retrying
// delivery whose target type no longer supports retries. Both
// restart recovery and the periodic sweep call it, so the
// terminal transition exists once.
// delivery whose target type no longer supports retries.
//
// This is only reachable when a target's type has been changed
// out from under an in-flight retrying delivery (or the type is
@@ -1304,8 +1135,8 @@ func (e *Engine) failUnretryableRetry(
}
// failMissingTarget terminally fails a recovered delivery, pending or
// retrying, whose target row is gone. Restart recovery and the periodic
// sweep call it for both statuses, so the transition exists once.
// retrying, whose target row is gone. The recovery routine calls it for
// both statuses.
//
// Until it existed those paths logged the failed lookup and moved on,
// which left the delivery where it was for the life of the database and
@@ -1830,11 +1661,11 @@ func (e *Engine) countAttempts(
// answer cannot go stale between asking and acting.
//
// Stamping updated_at is the same statement, and it is a cadence
// control rather than a claim: the pending sweep selects on that
// column, so a delivery handed out now is not selected again on the
// next tick a minute later but after pendingSweepMinAge. A delivery
// the database refuses to settle is therefore retried on that
// interval instead of every tick.
// control rather than a claim: a tick of the sweep selects pending
// deliveries on that column, so one handed out now is not selected
// again on the next tick a minute later but after pendingSweepMinAge.
// A delivery the database refuses to settle is therefore retried on
// that interval instead of every tick.
//
// A failed write is a refusal. It means the database is not accepting
// writes, which is the condition that stranded the delivery in the
@@ -1882,14 +1713,11 @@ func (e *Engine) takeForRedispatch(
return true
}
// queueRecovered puts an owned delivery's task on a worker channel,
// dropping the ownership the gate took if it does not fit.
func (e *Engine) queueRecovered(
ch chan<- Task, task Task,
) bool {
// queueRecovered puts an owned pending delivery's task on the delivery
// channel, dropping the ownership the gate took if it does not fit.
func (e *Engine) queueRecovered(task Task) {
select {
case ch <- task:
return true
case e.deliveryCh <- task:
default:
e.inflight.release(task.DeliveryID)
e.log.Warn(
@@ -1898,53 +1726,9 @@ func (e *Engine) queueRecovered(
"delivery_id", task.DeliveryID,
"webhook_id", task.WebhookID,
)
return false
}
}
// redispatch hands a recovered delivery to a worker channel through
// takeForRedispatch, and reports whether the task was queued.
func (e *Engine) redispatch(
ch chan<- Task,
webhookDB *gorm.DB,
task Task,
eligible database.DeliveryStatus,
) bool {
if !e.takeForRedispatch(
webhookDB, task.DeliveryID, eligible,
) {
return false
}
return e.queueRecovered(ch, task)
}
// rescheduleRecovered hands an orphaned retrying delivery back to the
// retry timer, through the same gate. It reports whether the delivery
// was rescheduled.
//
// The reference taken by the gate is dropped as soon as ScheduleRetry
// has taken its own, which it does before returning: what keeps the
// delivery owned through the backoff window is ScheduleRetry's
// reference, not this one.
func (e *Engine) rescheduleRecovered(
webhookDB *gorm.DB, task Task, delay time.Duration,
) bool {
if !e.takeForRedispatch(
webhookDB, task.DeliveryID,
database.DeliveryStatusRetrying,
) {
return false
}
defer e.inflight.release(task.DeliveryID)
e.ScheduleRetry(task, delay)
return true
}
// countAttemptsBatch counts the recorded attempts of every delivery
// in a batch with one grouped query, keyed by delivery id. Deliveries
// with no attempts are simply absent from the result, which reads back
@@ -2202,6 +1986,6 @@ func (e *Engine) sendRecoveredDeliveries(
attempts[deliveries[i].ID]+1,
)
e.queueRecovered(e.deliveryCh, task)
e.queueRecovered(task)
}
}
internal/delivery/engine_integration_test.go
+31 -30
View File
@@ -659,7 +659,7 @@ func TestRecoverPendingDeliveries(t *testing.T) {
s.Engine.ExportRecoverPendingDeliveries(
context.Background(), s.WebhookDB,
s.WebhookID,
s.WebhookID, atStart,
)
for i := range 3 {
@@ -756,7 +756,7 @@ func TestRecoverWebhookDeliveries_RetryingDeliveries(
)
s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID,
context.Background(), s.WebhookID, atStart,
)
select {
@@ -880,7 +880,7 @@ func TestRecoverSingleRetry_TypeNoLongerRetries(
)
s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID,
context.Background(), s.WebhookID, atStart,
)
iAssertTerminallyFailed(
@@ -888,7 +888,7 @@ func TestRecoverSingleRetry_TypeNoLongerRetries(
)
}
func TestSweepSingleRetry_TypeNoLongerRetries(
func TestRecoverSingleRetry_OnTick_TypeNoLongerRetries(
t *testing.T,
) {
t.Parallel()
@@ -903,8 +903,8 @@ func TestSweepSingleRetry_TypeNoLongerRetries(
t, s, database.TargetTypeDatabase,
)
s.Engine.ExportSweepWebhookRetries(
context.Background(), s.WebhookID,
s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, onTick,
)
iAssertTerminallyFailed(
@@ -955,8 +955,8 @@ func TestFailUnretryableRetry_WritesNoTargetRow(
iSeedFailedResult(t, s.WebhookDB, d.ID)
s.Engine.ExportSweepWebhookRetries(
context.Background(), s.WebhookID,
s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, onTick,
)
iAssertStatus(t, s.WebhookDB, d.ID,
@@ -987,13 +987,13 @@ func TestRecoverSingleRetry_UnknownTargetType(
deliveryID := iSeedRetryingWithType(t, s, unknown)
s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID,
context.Background(), s.WebhookID, atStart,
)
iAssertTerminallyFailed(t, s, deliveryID, unknown)
}
func TestSweepSingleRetry_UnknownTargetType(
func TestRecoverSingleRetry_OnTick_UnknownTargetType(
t *testing.T,
) {
t.Parallel()
@@ -1008,8 +1008,8 @@ func TestSweepSingleRetry_UnknownTargetType(
deliveryID := iSeedRetryingWithType(t, s, unknown)
s.Engine.ExportSweepWebhookRetries(
context.Background(), s.WebhookID,
s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, onTick,
)
iAssertTerminallyFailed(t, s, deliveryID, unknown)
@@ -1051,19 +1051,19 @@ func iCreateWebhook(
require.NoError(t, mainDB.Create(&webhook).Error)
}
// --- recoverInFlight Tests ---
// --- recoverDeliveries Tests ---
func TestRecoverInFlight_NoWebhooks(t *testing.T) {
func TestRecoverDeliveries_NoWebhooks(t *testing.T) {
t.Parallel()
s := newISetup(t)
s.Engine.ExportRecoverInFlight(
context.Background(),
s.Engine.ExportRecoverDeliveries(
context.Background(), atStart,
)
}
func TestRecoverInFlight_WithPendingDeliveries(
func TestRecoverDeliveries_WithPendingDeliveries(
t *testing.T,
) {
t.Parallel()
@@ -1090,8 +1090,8 @@ func TestRecoverInFlight_WithPendingDeliveries(
database.DeliveryStatusPending,
)
s.Engine.ExportRecoverInFlight(
context.Background(),
s.Engine.ExportRecoverDeliveries(
context.Background(), atStart,
)
select {
@@ -1102,16 +1102,16 @@ func TestRecoverInFlight_WithPendingDeliveries(
database.TargetTypeLog, task.TargetType,
)
case <-time.After(2 * time.Second):
t.Fatal("expected task from recoverInFlight")
t.Fatal("expected task from recovery at start")
}
}
// TestRecoverInFlight_ReportsAMissingWebhookDatabase covers a webhook
// whose database file is gone, after a partial restore say. Restart
// recovery opens every webhook's database, so the empty one made in its
// TestRecoverDeliveries_ReportsAMissingWebhookDatabase covers a webhook
// whose database file is gone, after a partial restore say. Recovery at
// start opens every webhook's database, so the empty one made in its
// place is reported at start, naming the file
// (https://git.eeqj.de/sneak/webhooker/issues/290).
func TestRecoverInFlight_ReportsAMissingWebhookDatabase(t *testing.T) {
func TestRecoverDeliveries_ReportsAMissingWebhookDatabase(t *testing.T) {
t.Parallel()
mainDB := iMainDB(t)
@@ -1131,7 +1131,7 @@ func TestRecoverInFlight_ReportsAMissingWebhookDatabase(t *testing.T) {
&http.Client{Timeout: 5 * time.Second}, 1,
)
engine.ExportRecoverInFlight(context.Background())
engine.ExportRecoverDeliveries(context.Background(), atStart)
assert.Contains(
t, logs.String(),
@@ -1140,10 +1140,11 @@ func TestRecoverInFlight_ReportsAMissingWebhookDatabase(t *testing.T) {
)
}
// TestRecoverInFlight_SkipsAWebhookDeletedAfterTheListIsRead covers a
// webhook deleted from the web interface while restart recovery runs.
// Its database file is gone, and recovery must not create it again.
func TestRecoverInFlight_SkipsAWebhookDeletedAfterTheListIsRead(
// TestRecoverDeliveries_SkipsAWebhookDeletedAfterTheListIsRead covers a
// webhook deleted from the web interface while the recovery routine
// runs. Its database file is gone, and recovery must not create it
// again.
func TestRecoverDeliveries_SkipsAWebhookDeletedAfterTheListIsRead(
t *testing.T,
) {
t.Parallel()
@@ -1179,7 +1180,7 @@ func TestRecoverInFlight_SkipsAWebhookDeletedAfterTheListIsRead(
&http.Client{Timeout: 5 * time.Second}, 1,
)
engine.ExportRecoverInFlight(context.Background())
engine.ExportRecoverDeliveries(context.Background(), atStart)
require.True(t, deleted)
assert.False(t, dbMgr.DBExists(webhookID))
internal/delivery/engine_lifecycle_test.go
+11 -10
View File
@@ -99,7 +99,7 @@ func requireStopHookExpires(
// the caller can drive OnStop too.
//
// Callers must not seed pending or retrying deliveries before
// calling this: restart recovery enqueues those during startup,
// calling this: the recovery routine's run at start enqueues those,
// which would put work in the queue while the pool is still
// racing its first select.
func startEngineViaHook(
@@ -178,8 +178,8 @@ func TestEngine_WorkersOutliveStartHookContext(t *testing.T) {
_ = lc.hooks[0].OnStop(context.Background())
})
// Seeded only after the pool has settled, so restart recovery
// cannot enqueue it during startup.
// Seeded only after the pool has settled, so the recovery
// routine's run at start cannot enqueue it.
deliveryID, task := seedLogTask(t, s)
s.Engine.Notify([]delivery.Task{task})
@@ -345,15 +345,15 @@ func stopDuringAttempt(
require.NoError(t, lc.hooks[0].OnStop(context.Background()))
}
// requireRequeued runs restart recovery on the stopped engine and
// requires it to queue task's attempt again on ch.
// requireRequeued runs the recovery routine as at start on the stopped
// engine and requires it to queue task's attempt again on ch.
func requireRequeued(
t *testing.T, s iSetup, ch <-chan delivery.Task, task delivery.Task,
) {
t.Helper()
s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID,
context.Background(), s.WebhookID, atStart,
)
select {
@@ -361,7 +361,7 @@ func requireRequeued(
assert.Equal(t, task.DeliveryID, got.DeliveryID)
assert.Equal(t, task.AttemptNum, got.AttemptNum)
case <-time.After(5 * time.Second):
t.Fatal("restart recovery did not queue the delivery again")
t.Fatal("recovery at start did not queue the delivery again")
}
}
@@ -371,7 +371,8 @@ func requireRequeued(
// no retries that was the delivery's only attempt, recorded as failed
// with "context canceled", and nothing ever sent it again. The
// delivery must instead stay pending, with no result and no count in
// the attempt metrics, for restart recovery to send.
// the attempt metrics, for the recovery routine to send at the next
// start.
func TestEngine_StopLeavesFireAndForgetAttemptPending(t *testing.T) {
t.Parallel()
@@ -399,8 +400,8 @@ func TestEngine_StopLeavesFireAndForgetAttemptPending(t *testing.T) {
// TestEngine_StopLeavesLastRetryAttemptRetrying is the same regression
// for a target with retries, on its last attempt: the cancelled attempt
// used to exhaust the retries and end the delivery failed. It must stay
// retrying with only its earlier attempt recorded, and restart recovery
// must send that last attempt again.
// retrying with only its earlier attempt recorded, and the recovery
// routine at the next start must send that last attempt again.
func TestEngine_StopLeavesLastRetryAttemptRetrying(t *testing.T) {
t.Parallel()
internal/delivery/export_test.go
+11 -16
View File
@@ -281,32 +281,27 @@ func (e *Engine) ExportRecoverPendingDeliveries(
ctx context.Context,
webhookDB *gorm.DB,
webhookID string,
atStart bool,
) {
e.recoverPendingDeliveries(
ctx, webhookDB, webhookID,
ctx, webhookDB, webhookID, atStart,
)
}
// ExportRecoverWebhookDeliveries exposes
// recoverWebhookDeliveries.
func (e *Engine) ExportRecoverWebhookDeliveries(
ctx context.Context, webhookID string,
ctx context.Context, webhookID string, atStart bool,
) {
e.recoverWebhookDeliveries(ctx, webhookID)
e.recoverWebhookDeliveries(ctx, webhookID, atStart)
}
// ExportRecoverInFlight exposes recoverInFlight.
func (e *Engine) ExportRecoverInFlight(
ctx context.Context,
// ExportRecoverDeliveries exposes recoverDeliveries, the recovery
// routine.
func (e *Engine) ExportRecoverDeliveries(
ctx context.Context, atStart bool,
) {
e.recoverInFlight(ctx)
}
// ExportSweepWebhookRetries exposes sweepWebhookRetries.
func (e *Engine) ExportSweepWebhookRetries(
ctx context.Context, webhookID string,
) {
e.sweepWebhookRetries(ctx, webhookID)
e.recoverDeliveries(ctx, atStart)
}
// ExportStart exposes start for testing.
@@ -353,9 +348,9 @@ func (e *Engine) ExportRetainDelivery(deliveryID string) bool {
// ExportRecoverRetryingDeliveries exposes recoverRetryingDeliveries.
func (e *Engine) ExportRecoverRetryingDeliveries(
webhookDB *gorm.DB, webhookID string,
ctx context.Context, webhookDB *gorm.DB, webhookID string,
) {
e.recoverRetryingDeliveries(webhookDB, webhookID)
e.recoverRetryingDeliveries(ctx, webhookDB, webhookID)
}
// ExportFailMissingTarget exposes failMissingTarget, so a test can hand
internal/delivery/inflight.go
+9 -9
View File
@@ -6,10 +6,10 @@ import "sync"
//
// A delivery is owned from the moment a task for it is handed to a
// channel or to a retry timer until the engine has no further plan for
// it in memory. Restart recovery and both arms of the periodic sweep
// re-dispatch only deliveries the set does not hold, which is what
// makes them exact rather than a guess about how long a row has sat at
// pending.
// it in memory. The recovery routine, at start and on every tick of the
// sweep, re-dispatches only deliveries the set does not hold, which is
// what makes it exact rather than a guess about how long a row has sat
// at pending.
//
// This replaces reasoning from timestamps. A delivery's row says
// pending from creation until its outcome is written, which covers
@@ -25,8 +25,8 @@ import "sync"
// In-memory state is sufficient because a data directory admits one
// process: internal/datadir takes an flock on it at startup and a
// second instance refuses to run. Deliveries owned by a process that
// died are not in any successor's set, and restart recovery is what
// picks those up.
// died are not in any successor's set, and the recovery routine's run
// at start is what picks those up.
//
// References are counted rather than held as a plain set because
// ownership outlives the worker that took it. A target that schedules
@@ -61,9 +61,9 @@ func (s *inflightSet) retain(deliveryID string) {
// retainIdle takes the first reference on a delivery, and reports
// whether it got it. It fails when the engine already owns the
// delivery, which is what makes two claimants — restart recovery and
// the sweep run concurrently, or two sweep arms — mutually exclusive
// rather than merely atomic.
// delivery, which is what makes two claimants — Notify and the
// recovery routine, say — mutually exclusive rather than merely
// atomic.
func (s *inflightSet) retainIdle(deliveryID string) bool {
s.mu.Lock()
defer s.mu.Unlock()
internal/delivery/inflight_test.go
+24 -20
View File
@@ -95,8 +95,8 @@ func TestSweepLeavesAQueuedDeliveryAlone(t *testing.T) {
require.Equal(t, 1, s.Engine.ExportInflightHeld())
s.Engine.ExportSweepWebhookRetries(
context.Background(), s.WebhookID,
s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, onTick,
)
tasks := fDrain(s.Engine)
@@ -107,9 +107,11 @@ func TestSweepLeavesAQueuedDeliveryAlone(t *testing.T) {
)
}
// TestRecoveryAndSweepDoNotDoubleDispatch drives the two entry points
// the engine starts concurrently against one aged pending row. Before
// ownership they both dispatched it.
// TestRecoveryAndSweepDoNotDoubleDispatch drives the recovery routine's
// run at start and a tick of the sweep at once against one aged pending
// row. The engine runs them one after the other, but Notify can race
// either, and ownership is what keeps any two claimants from both
// sending. Before ownership the two runs both dispatched the row.
func TestRecoveryAndSweepDoNotDoubleDispatch(t *testing.T) {
t.Parallel()
@@ -133,12 +135,12 @@ func TestRecoveryAndSweepDoNotDoubleDispatch(t *testing.T) {
wg.Go(func() {
s.Engine.ExportRecoverPendingDeliveries(
ctx, s.WebhookDB, s.WebhookID,
ctx, s.WebhookDB, s.WebhookID, atStart,
)
})
wg.Go(func() {
s.Engine.ExportSweepWebhookRetries(
ctx, s.WebhookID,
s.Engine.ExportRecoverWebhookDeliveries(
ctx, s.WebhookID, onTick,
)
})
wg.Wait()
@@ -247,9 +249,10 @@ func TestOwnershipIsReleasedAfterDelivery(t *testing.T) {
// TestNotifyAfterRecoveryDoesNotSendAgain is the startup race of
// https://git.eeqj.de/sneak/webhooker/issues/299. The receiver has
// written a delivery, restart recovery finds it pending, sends it and
// releases it, and only then does the receiver's Notify for it arrive.
// Nothing owns the delivery by then, so Notify takes it.
// written a delivery, the recovery routine's run at start finds it
// pending, sends it and releases it, and only then does the receiver's
// Notify for it arrive. Nothing owns the delivery by then, so Notify
// takes it.
func TestNotifyAfterRecoveryDoesNotSendAgain(t *testing.T) {
t.Parallel()
@@ -273,7 +276,8 @@ func TestNotifyAfterRecoveryDoesNotSendAgain(t *testing.T) {
)
}()
// Restart recovery sends the delivery and lets it go.
// The recovery routine's run at start sends the delivery and lets
// it go.
iWaitForDelivered(t, s.WebhookDB, d.ID)
require.Eventually(
t,
@@ -334,7 +338,7 @@ func TestRetryingRecoverySkipsASuccessfulResult(t *testing.T) {
rSeedResult(t, s.WebhookDB, d.ID, 2, true)
s.Engine.ExportRecoverRetryingDeliveries(
s.WebhookDB, s.WebhookID,
context.Background(), s.WebhookDB, s.WebhookID,
)
assert.Empty(
@@ -349,8 +353,8 @@ func TestRetryingRecoverySkipsASuccessfulResult(t *testing.T) {
)
}
// TestRetryingSweepSkipsASuccessfulResult is the same rule on the
// periodic sweep's retrying arm.
// TestRetryingSweepSkipsASuccessfulResult is the same rule on a tick of
// the sweep.
func TestRetryingSweepSkipsASuccessfulResult(t *testing.T) {
t.Parallel()
@@ -368,8 +372,8 @@ func TestRetryingSweepSkipsASuccessfulResult(t *testing.T) {
rSeedResult(t, s.WebhookDB, d.ID, 1, false)
rSeedResult(t, s.WebhookDB, d.ID, 2, true)
s.Engine.ExportSweepWebhookRetries(
context.Background(), s.WebhookID,
s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, onTick,
)
assert.Empty(t, fDrain(s.Engine))
@@ -420,8 +424,8 @@ func TestScheduledRetryIsNotSweptDuringBackoff(t *testing.T) {
require.Equal(t, 1, s.Engine.ExportInflightHeld())
s.Engine.ExportSweepWebhookRetries(
context.Background(), s.WebhookID,
s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, onTick,
)
assert.Empty(
@@ -453,7 +457,7 @@ func TestRedispatchStampsTheRow(t *testing.T) {
ctx := context.Background()
s.Engine.ExportSweepWebhookRetries(ctx, s.WebhookID)
s.Engine.ExportRecoverWebhookDeliveries(ctx, s.WebhookID, onTick)
require.Len(t, fDrain(s.Engine), 1)
var row database.Delivery
internal/delivery/metrics_test.go
+2 -2
View File
@@ -444,8 +444,8 @@ func TestDeliveryMetrics_OrphanedRetryFailureLabelled(
t, s, database.TargetTypeLog,
)
s.Engine.ExportSweepWebhookRetries(
context.Background(), s.WebhookID,
s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, onTick,
)
iAssertStatus(t, s.WebhookDB, deliveryID,
internal/delivery/queue_depth.go
+2 -1
View File
@@ -20,7 +20,8 @@ const queueDepthSampleInterval = 30 * time.Second
// to be seeded correctly at startup from rows written by a previous
// process, and would drift permanently on any transition that failed
// to persist. Counting is the measurement that cannot go wrong, and
// it is the same whole-database walk the retry sweep already makes.
// it is the same whole-database walk each tick of the sweep already
// makes.
func (e *Engine) queueDepthSampler(ctx context.Context) {
defer e.wg.Done()
internal/delivery/recovery_durability_test.go
+14 -13
View File
@@ -88,7 +88,7 @@ func TestRecoverySkipsPendingWithSuccessfulResult(
)
s.Engine.ExportRecoverPendingDeliveries(
context.Background(), s.WebhookDB, s.WebhookID,
context.Background(), s.WebhookDB, s.WebhookID, atStart,
)
select {
@@ -145,7 +145,7 @@ func TestRecoveryContinuesTheAttemptNumbering(t *testing.T) {
rSeedResult(t, s.WebhookDB, d.ID, 2, false)
s.Engine.ExportRecoverPendingDeliveries(
context.Background(), s.WebhookDB, s.WebhookID,
context.Background(), s.WebhookDB, s.WebhookID, atStart,
)
select {
@@ -183,8 +183,8 @@ func TestSweepRecoversStrandedPending(t *testing.T) {
database.DeliveryStatusPending,
)
s.Engine.ExportSweepWebhookRetries(
context.Background(), s.WebhookID,
s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, onTick,
)
select {
@@ -230,7 +230,7 @@ func TestSweepClaimsAStrandedDeliveryOnlyOnce(t *testing.T) {
ctx := context.Background()
s.Engine.ExportSweepWebhookRetries(ctx, s.WebhookID)
s.Engine.ExportRecoverWebhookDeliveries(ctx, s.WebhookID, onTick)
select {
case task := <-s.Engine.ExportDeliveryCh():
@@ -246,7 +246,7 @@ func TestSweepClaimsAStrandedDeliveryOnlyOnce(t *testing.T) {
database.DeliveryStatusPending,
)
s.Engine.ExportSweepWebhookRetries(ctx, s.WebhookID)
s.Engine.ExportRecoverWebhookDeliveries(ctx, s.WebhookID, onTick)
select {
case task := <-s.Engine.ExportDeliveryCh():
@@ -258,9 +258,9 @@ func TestSweepClaimsAStrandedDeliveryOnlyOnce(t *testing.T) {
}
}
// TestSweepSettlesStrandedPendingWithoutResending is the sweep's own
// version of the reconcile: a stranded delivery holding a successful
// result is settled where it stands, and the receiver hears nothing.
// TestSweepSettlesStrandedPendingWithoutResending is the reconcile on a
// tick of the sweep: a stranded delivery holding a successful result is
// settled where it stands, and the receiver hears nothing.
func TestSweepSettlesStrandedPendingWithoutResending(
t *testing.T,
) {
@@ -280,8 +280,8 @@ func TestSweepSettlesStrandedPendingWithoutResending(
rSeedResult(t, s.WebhookDB, d.ID, 1, true)
rAgePending(t, s.WebhookDB, d.ID)
s.Engine.ExportSweepWebhookRetries(
context.Background(), s.WebhookID,
s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, onTick,
)
select {
@@ -312,8 +312,9 @@ func TestSweepSettlesStrandedPendingWithoutResending(
// TestFailedResultWriteLeavesDeliveryRecoverable is the rule the
// targets now follow: a bookkeeping write that fails must not advance
// the status, because pending and retrying are the states the sweeps
// recover and delivered is a claim the database refused to record.
// the status, because pending and retrying are the states the recovery
// routine recovers and delivered is a claim the database refused to
// record.
func TestFailedResultWriteLeavesDeliveryRecoverable(
t *testing.T,
) {
internal/delivery/recovery_test.go
+340
View File
@@ -0,0 +1,340 @@
package delivery_test
import (
"context"
"log/slog"
"net/http"
"testing"
"time"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/database/databasetest"
"sneak.berlin/go/webhooker/internal/delivery"
)
// These tests cover the recovery routine, which the engine runs once
// when it starts and then on every tick of the sweep
// (https://git.eeqj.de/sneak/webhooker/issues/542). Every case runs it
// both ways, each on a webhook of its own.
// atStart and onTick are the two values of the recovery routine's
// atStart parameter: its run when the engine starts, and a tick of the
// sweep.
const (
atStart = true
onTick = false
)
// recoveryRun names one of the two runs of the recovery routine.
type recoveryRun struct {
name string
atStart bool
}
// recoveryRuns returns the routine's run at start and a tick.
func recoveryRuns() []recoveryRun {
return []recoveryRun{
{"at start", atStart},
{"on a tick", onTick},
}
}
// recoverySetup gives a fresh webhook its main-database row and an http
// target with retries, and returns the setup and the target's ID. No
// worker runs, so whatever the routine queues stays in the channels for
// the test to read, and nothing is sent to the target.
func recoverySetup(t *testing.T) (iSetup, string) {
t.Helper()
s := newISetup(t)
targetID := uuid.New().String()
iCreateWebhook(t, s.MainDB, s.WebhookID, "recovery")
iCreateTarget(
t, s.MainDB, targetID, s.WebhookID, "recovery",
database.TargetTypeHTTP,
iHTTPConfig("http://example.com/hook"), 20,
)
return s, targetID
}
// seedRecoveryDelivery seeds a delivery at status with the given
// recorded attempts, failed but for the last when lastSucceeded is set,
// all recorded now.
func seedRecoveryDelivery(
t *testing.T,
s iSetup,
targetID string,
status database.DeliveryStatus,
attempts int,
lastSucceeded bool,
) string {
t.Helper()
event := iSeedEvent(t, s.WebhookDB, s.WebhookID, `{"recovery":true}`)
d := iSeedDelivery(t, s.WebhookDB, event.ID, targetID, status)
for n := 1; n <= attempts; n++ {
rSeedResult(
t, s.WebhookDB, d.ID, n, lastSucceeded && n == attempts,
)
}
return d.ID
}
// backdateAttempts moves every recorded attempt of a delivery an hour
// into the past, long past the backoff after its first attempt.
func backdateAttempts(t *testing.T, s iSetup, deliveryID string) {
t.Helper()
require.NoError(t, s.WebhookDB.Model(&database.DeliveryResult{}).
Where("delivery_id = ?", deliveryID).
UpdateColumn("created_at", s.WebhookDB.NowFunc().Add(-time.Hour)).
Error)
}
// requireQueued waits for a task on ch. A retrying delivery reaches the
// retry channel through a timer, even one due now, so it is waited for.
func requireQueued(
t *testing.T, ch <-chan delivery.Task,
) delivery.Task {
t.Helper()
select {
case task := <-ch:
return task
case <-time.After(5 * time.Second):
t.Fatal("the recovery routine queued nothing")
return delivery.Task{}
}
}
// assertNothingQueued asserts the engine queues nothing, giving a retry
// timer that is due now the moment it needs to fire.
func assertNothingQueued(t *testing.T, e *delivery.Engine) {
t.Helper()
select {
case task := <-e.ExportDeliveryCh():
t.Errorf("queued pending delivery %s", task.DeliveryID)
case task := <-e.ExportRetryCh():
t.Errorf("queued retrying delivery %s", task.DeliveryID)
case <-time.After(200 * time.Millisecond):
}
}
// TestRecovery_RetryingBackoffPassed: a retrying delivery whose backoff
// is over is queued for its next attempt at once.
func TestRecovery_RetryingBackoffPassed(t *testing.T) {
t.Parallel()
for _, run := range recoveryRuns() {
t.Run(run.name, func(t *testing.T) {
t.Parallel()
s, targetID := recoverySetup(t)
id := seedRecoveryDelivery(
t, s, targetID, database.DeliveryStatusRetrying, 1, false,
)
backdateAttempts(t, s, id)
s.Engine.ExportRecoverDeliveries(context.Background(), run.atStart)
task := requireQueued(t, s.Engine.ExportRetryCh())
assert.Equal(t, id, task.DeliveryID)
assert.Equal(t, 2, task.AttemptNum)
iAssertStatus(t, s.WebhookDB, id, database.DeliveryStatusRetrying)
})
}
}
// TestRecovery_RetryingBackoffNotPassed: a retrying delivery whose last
// attempt was just now waits out the rest of its backoff, ten attempts
// in, eight and a half minutes. It is not sent now, and the engine holds
// it, so a later tick leaves it to the timer.
func TestRecovery_RetryingBackoffNotPassed(t *testing.T) {
t.Parallel()
for _, run := range recoveryRuns() {
t.Run(run.name, func(t *testing.T) {
t.Parallel()
s, targetID := recoverySetup(t)
id := seedRecoveryDelivery(
t, s, targetID, database.DeliveryStatusRetrying, 10, false,
)
s.Engine.ExportRecoverDeliveries(context.Background(), run.atStart)
assertNothingQueued(t, s.Engine)
iAssertStatus(t, s.WebhookDB, id, database.DeliveryStatusRetrying)
assert.Equal(t, 1, s.Engine.ExportInflightHeld(),
"no retry timer holds the delivery",
)
})
}
}
// TestRecovery_PendingOlderThanTheAgeBound: a pending delivery untouched
// for longer than the sweep's age bound is sent by either run.
func TestRecovery_PendingOlderThanTheAgeBound(t *testing.T) {
t.Parallel()
for _, run := range recoveryRuns() {
t.Run(run.name, func(t *testing.T) {
t.Parallel()
s, targetID := recoverySetup(t)
id := seedRecoveryDelivery(
t, s, targetID, database.DeliveryStatusPending, 0, false,
)
rAgePending(t, s.WebhookDB, id)
s.Engine.ExportRecoverDeliveries(context.Background(), run.atStart)
task := requireQueued(t, s.Engine.ExportDeliveryCh())
assert.Equal(t, id, task.DeliveryID)
assert.Equal(t, 1, task.AttemptNum)
})
}
}
// TestRecovery_PendingYoungerThanTheAgeBound: a pending delivery touched
// a moment ago, as one a stop cut off is, is sent at start, which is how
// it reaches its target on the next start
// (https://git.eeqj.de/sneak/webhooker/issues/527). A tick leaves it.
func TestRecovery_PendingYoungerThanTheAgeBound(t *testing.T) {
t.Parallel()
for _, run := range recoveryRuns() {
t.Run(run.name, func(t *testing.T) {
t.Parallel()
s, targetID := recoverySetup(t)
id := seedRecoveryDelivery(
t, s, targetID, database.DeliveryStatusPending, 0, false,
)
s.Engine.ExportRecoverDeliveries(context.Background(), run.atStart)
if run.atStart {
task := requireQueued(t, s.Engine.ExportDeliveryCh())
assert.Equal(t, id, task.DeliveryID)
return
}
assertNothingQueued(t, s.Engine)
iAssertStatus(t, s.WebhookDB, id, database.DeliveryStatusPending)
assert.Zero(t, s.Engine.ExportInflightHeld())
})
}
}
// TestRecovery_AlreadySucceeded: a pending or retrying delivery whose
// success is recorded but whose status write was lost is marked
// delivered where it stands, and not sent again.
func TestRecovery_AlreadySucceeded(t *testing.T) {
t.Parallel()
for _, run := range recoveryRuns() {
t.Run(run.name, func(t *testing.T) {
t.Parallel()
s, targetID := recoverySetup(t)
pending := seedRecoveryDelivery(
t, s, targetID, database.DeliveryStatusPending, 1, true,
)
rAgePending(t, s.WebhookDB, pending)
retrying := seedRecoveryDelivery(
t, s, targetID, database.DeliveryStatusRetrying, 2, true,
)
backdateAttempts(t, s, retrying)
s.Engine.ExportRecoverDeliveries(context.Background(), run.atStart)
assertNothingQueued(t, s.Engine)
for id, attempts := range map[string]int{pending: 1, retrying: 2} {
iAssertStatus(
t, s.WebhookDB, id, database.DeliveryStatusDelivered,
)
assert.Len(t, iResults(t, s.WebhookDB, id), attempts,
"marking the delivery delivered recorded an attempt",
)
}
})
}
}
// TestRecovery_MissingDatabase: a tick leaves a webhook whose database
// file is missing alone, and the run at start opens it, which creates an
// empty one and reports the loss (see
// TestRecoverDeliveries_ReportsAMissingWebhookDatabase).
func TestRecovery_MissingDatabase(t *testing.T) {
t.Parallel()
mainDB := iMainDB(t)
webhookID := uuid.New().String()
iCreateWebhook(t, mainDB.DB(), webhookID, "missing-database")
dbMgr := databasetest.NewWebhookDBManager(t, t.TempDir())
t.Cleanup(func() { _ = dbMgr.CloseAll() })
engine := delivery.NewTestEngineWithDB(
mainDB, dbMgr,
slog.New(slog.DiscardHandler),
&http.Client{Timeout: 5 * time.Second}, 1,
)
engine.ExportRecoverDeliveries(context.Background(), onTick)
assert.False(t, dbMgr.DBExists(webhookID),
"a tick created the missing database",
)
engine.ExportRecoverDeliveries(context.Background(), atStart)
assert.True(t, dbMgr.DBExists(webhookID),
"the run at start did not open the missing database",
)
}
// TestRecovery_LeavesADeliveryTheEngineHolds: a delivery the engine is
// working on, has queued, or holds a retry timer for is left alone by
// either run, pending and retrying alike, however old.
func TestRecovery_LeavesADeliveryTheEngineHolds(t *testing.T) {
t.Parallel()
for _, run := range recoveryRuns() {
t.Run(run.name, func(t *testing.T) {
t.Parallel()
s, targetID := recoverySetup(t)
pending := seedRecoveryDelivery(
t, s, targetID, database.DeliveryStatusPending, 0, false,
)
rAgePending(t, s.WebhookDB, pending)
retrying := seedRecoveryDelivery(
t, s, targetID, database.DeliveryStatusRetrying, 1, false,
)
backdateAttempts(t, s, retrying)
require.True(t, s.Engine.ExportRetainDelivery(pending))
require.True(t, s.Engine.ExportRetainDelivery(retrying))
s.Engine.ExportRecoverDeliveries(context.Background(), run.atStart)
assertNothingQueued(t, s.Engine)
iAssertStatus(t, s.WebhookDB, pending, database.DeliveryStatusPending)
iAssertStatus(t, s.WebhookDB, retrying, database.DeliveryStatusRetrying)
assert.Equal(t, 2, s.Engine.ExportInflightHeld())
})
}
}
internal/delivery/target.go
+9 -18
View File
@@ -13,8 +13,8 @@ import (
// The engine provides one to each target so a target can own
// its retries durably: it records the attempt, marks the
// delivery retrying, and asks the Scheduler to deliver the
// next attempt after delay — exactly what the engine does for
// its own restart recovery.
// next attempt after delay — exactly what the engine's
// recovery routine does for a retrying delivery.
type Scheduler interface {
ScheduleRetry(task Task, delay time.Duration)
}
@@ -45,29 +45,20 @@ type Target interface {
}
// rescheduler is implemented by targets that own durable
// retries. The engine's restart recovery and periodic sweep
// use it to let the target recompute the schedule for an
// orphaned retrying delivery, keeping the retry schedule
// target-owned. Fire-and-forget targets do not implement it
// and their (never-occurring) retrying deliveries are
// skipped.
// retries. The engine's recovery routine uses it to let the
// target recompute the schedule for an orphaned retrying
// delivery, keeping the retry schedule target-owned.
// Fire-and-forget targets do not implement it, and a retrying
// delivery found for one is terminally failed.
type rescheduler interface {
// remainingBackoff returns how long to wait before the
// next attempt of a recovered retrying delivery.
// next attempt of a recovered retrying delivery: zero once
// its backoff has passed.
remainingBackoff(
webhookDB *gorm.DB,
deliveryID string,
attemptNum int,
) time.Duration
// backoffElapsed reports whether the backoff window for
// the last attempt has already passed, so the periodic
// sweep can re-enqueue the delivery now.
backoffElapsed(
webhookDB *gorm.DB,
deliveryID string,
attemptNum int,
) bool
}
// attemptResult is the outcome of a single delivery attempt,
internal/delivery/target_http.go
+4 -26
View File
@@ -84,8 +84,8 @@ func (c *httpCore) deliver(
// engine is stopping, which cancels the context its request is made
// with. Such an attempt is not recorded: no DeliveryResult, no circuit
// breaker outcome, no status change, no count in the attempt metrics.
// The delivery stays pending or retrying, and startup recovery sends
// it again on the next start.
// The delivery stays pending or retrying, and the recovery routine
// sends it again on the next start.
//
// A target's own timeout leaves that context alone, so its attempt is
// still recorded as the failure it is.
@@ -334,7 +334,8 @@ func (c *httpCore) publishCircuitState(
// remainingBackoff returns how long remains of the backoff
// window for the last attempt of a recovered retrying
// delivery. It implements rescheduler.
// delivery: zero once it has passed, or when the last attempt
// cannot be read. It implements rescheduler.
func (c *httpCore) remainingBackoff(
webhookDB *gorm.DB,
deliveryID string,
@@ -357,29 +358,6 @@ func (c *httpCore) remainingBackoff(
return max(remaining, 0)
}
// backoffElapsed reports whether the backoff window for the
// last attempt of a retrying delivery has passed. It
// implements rescheduler.
func (c *httpCore) backoffElapsed(
webhookDB *gorm.DB,
deliveryID string,
attemptNum int,
) bool {
var lastResult database.DeliveryResult
err := webhookDB.
Where("delivery_id = ?", deliveryID).
Order("created_at DESC").
First(&lastResult).Error
if err != nil {
return true
}
backoff := Backoff(attemptNum)
return time.Since(lastResult.CreatedAt) >= backoff
}
// Backoff is how long an http or slack target with retries waits after
// a delivery's failed attempt attemptNum before trying it again.
func Backoff(attemptNum int) time.Duration {
internal/delivery/terminal_state_test.go
+38 -37
View File
@@ -178,7 +178,7 @@ func TestRecoverSingleRetry_TargetDeleted(t *testing.T) {
)
s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID,
context.Background(), s.WebhookID, atStart,
)
iAssertStatus(
@@ -201,7 +201,7 @@ func TestRecoverSingleRetry_TargetDeleted(t *testing.T) {
)
}
func TestSweepSingleRetry_TargetDeleted(t *testing.T) {
func TestRecoverSingleRetry_OnTick_TargetDeleted(t *testing.T) {
t.Parallel()
s := newISetup(t)
@@ -218,11 +218,11 @@ func TestSweepSingleRetry_TargetDeleted(t *testing.T) {
// Twice, because the bug was an error the sweep repeated every
// minute for the life of the database: the second sweep must
// find nothing left to do.
s.Engine.ExportSweepWebhookRetries(
context.Background(), s.WebhookID,
s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, onTick,
)
s.Engine.ExportSweepWebhookRetries(
context.Background(), s.WebhookID,
s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, onTick,
)
iAssertStatus(
@@ -239,10 +239,11 @@ func TestSweepSingleRetry_TargetDeleted(t *testing.T) {
assert.Zero(t, s.Engine.ExportInflightHeld())
}
// TestSweepSingleRetry_TargetNeverExisted covers the other half of the
// soft-delete distinction: an id with no row at all, deleted or
// otherwise, must not be reported as something the operator deleted.
func TestSweepSingleRetry_TargetNeverExisted(t *testing.T) {
// TestRecoverSingleRetry_OnTick_TargetNeverExisted covers the other
// half of the soft-delete distinction: an id with no row at all,
// deleted or otherwise, must not be reported as something the operator
// deleted.
func TestRecoverSingleRetry_OnTick_TargetNeverExisted(t *testing.T) {
t.Parallel()
s := newISetup(t)
@@ -264,8 +265,8 @@ func TestSweepSingleRetry_TargetNeverExisted(t *testing.T) {
iSeedFailedResult(t, s.WebhookDB, d.ID)
s.Engine.ExportSweepWebhookRetries(
context.Background(), s.WebhookID,
s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, onTick,
)
iAssertStatus(
@@ -303,8 +304,8 @@ func TestFailMissingTarget_WritesNoTargetRow(
database.DeliveryStatusRetrying,
)
s.Engine.ExportSweepWebhookRetries(
context.Background(), s.WebhookID,
s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, onTick,
)
iAssertStatus(
@@ -546,7 +547,7 @@ func TestRecoverSingleRetry_TargetUnreadable_LeavesDeliveryAlone(
require.NoError(t, sqlDB.Close())
s.Engine.ExportRecoverRetryingDeliveries(
s.WebhookDB, s.WebhookID,
context.Background(), s.WebhookDB, s.WebhookID,
)
iAssertStatus(
@@ -564,7 +565,7 @@ func TestRecoverSingleRetry_TargetUnreadable_LeavesDeliveryAlone(
// --- 4. A pending delivery whose target is gone ---
func TestRecoverPending_TargetDeleted(t *testing.T) {
func TestRecoverPendingDeliveries_TargetDeleted(t *testing.T) {
t.Parallel()
s := newISetup(t)
@@ -577,7 +578,7 @@ func TestRecoverPending_TargetDeleted(t *testing.T) {
)
s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID,
context.Background(), s.WebhookID, atStart,
)
iAssertStatus(
@@ -600,10 +601,10 @@ func TestRecoverPending_TargetDeleted(t *testing.T) {
)
}
// TestRecoverPending_TargetDeleted_LeavesAnOwnedDeliveryAlone: the
// terminal write takes ownership like every other recovery write, so a
// delivery the engine still holds is not failed underneath its worker.
func TestRecoverPending_TargetDeleted_LeavesAnOwnedDeliveryAlone(
// TestRecoverPendingDeliveries_TargetDeleted_LeavesAnOwnedDeliveryAlone:
// the terminal write takes ownership like every other recovery write, so
// a delivery the engine still holds is not failed underneath its worker.
func TestRecoverPendingDeliveries_TargetDeleted_LeavesAnOwnedDeliveryAlone(
t *testing.T,
) {
t.Parallel()
@@ -620,7 +621,7 @@ func TestRecoverPending_TargetDeleted_LeavesAnOwnedDeliveryAlone(
require.True(t, s.Engine.ExportRetainDelivery(deliveryID))
s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID,
context.Background(), s.WebhookID, atStart,
)
iAssertStatus(
@@ -675,18 +676,18 @@ func TestFailMissingTarget_LeavesASettledDeliveryAlone(
assert.Zero(t, s.Engine.ExportInflightHeld())
}
// TestSweepPending_TargetDeleted sweeps twice over a batch that also
// holds a healthy stranded delivery. The one whose target is gone is
// failed once and then left alone; the healthy one is queued by the
// first sweep and not again by the second.
func TestSweepPending_TargetDeleted(t *testing.T) {
// TestRecoverPendingDeliveries_OnTick_TargetDeleted sweeps twice over a
// batch that also holds a healthy stranded delivery. The one whose
// target is gone is failed once and then left alone; the healthy one is
// queued by the first sweep and not again by the second.
func TestRecoverPendingDeliveries_OnTick_TargetDeleted(t *testing.T) {
t.Parallel()
liveTargetID := uuid.New().String()
s := fSweepSetup(t, liveTargetID, "still-there")
deliveryID := tSeedDeletedTarget(
t, s, "gone-on-pending-sweep", "http://example.com/hook",
t, s, "gone-while-pending-on-tick", "http://example.com/hook",
database.DeliveryStatusPending,
)
rAgePending(t, s.WebhookDB, deliveryID)
@@ -703,7 +704,7 @@ func TestSweepPending_TargetDeleted(t *testing.T) {
ctx := context.Background()
s.Engine.ExportSweepWebhookRetries(ctx, s.WebhookID)
s.Engine.ExportRecoverWebhookDeliveries(ctx, s.WebhookID, onTick)
tasks := fDrain(s.Engine)
require.Len(t, tasks, 1,
@@ -711,7 +712,7 @@ func TestSweepPending_TargetDeleted(t *testing.T) {
)
assert.Equal(t, healthy.ID, tasks[0].DeliveryID)
s.Engine.ExportSweepWebhookRetries(ctx, s.WebhookID)
s.Engine.ExportRecoverWebhookDeliveries(ctx, s.WebhookID, onTick)
assert.Empty(t, fDrain(s.Engine),
"the second sweep queued a delivery again",
@@ -724,7 +725,7 @@ func TestSweepPending_TargetDeleted(t *testing.T) {
last := tLastResult(t, s, deliveryID, 2)
assert.Contains(t, last.Error, "gone-on-pending-sweep")
assert.Contains(t, last.Error, "gone-while-pending-on-tick")
assert.Contains(t, last.Error, "was deleted")
}
@@ -775,11 +776,11 @@ func TestSendRecoveredDeliveries_TargetMissingFromMap(
)
}
// TestRecoverPending_TargetUnreadable_LeavesDeliveryAlone: a failed
// read of the main database is not a deleted target. Restart recovery
// holds every pending delivery of the webhook in one batch, so failing
// on this would fail all of them.
func TestRecoverPending_TargetUnreadable_LeavesDeliveryAlone(
// TestRecoverPendingDeliveries_TargetUnreadable_LeavesDeliveryAlone: a
// failed read of the main database is not a deleted target. At start the
// recovery routine holds every pending delivery of the webhook in one
// batch, so failing on this would fail all of them.
func TestRecoverPendingDeliveries_TargetUnreadable_LeavesDeliveryAlone(
t *testing.T,
) {
t.Parallel()
@@ -807,7 +808,7 @@ func TestRecoverPending_TargetUnreadable_LeavesDeliveryAlone(
require.NoError(t, sqlDB.Close())
s.Engine.ExportRecoverPendingDeliveries(
context.Background(), s.WebhookDB, s.WebhookID,
context.Background(), s.WebhookDB, s.WebhookID, atStart,
)
iAssertStatus(