lifecycle: the recovery-backoff window holds a dead run for up to 10 minutes while Stop admits StatusRecovering
- Dominant language
- Go
- Stars
- 610
- Forks
- 63
- Avg merge
- 12h 28m
- Merged PRs (30d)
- 57
Description
Found during adversarial review of the #2806 fix. **Pre-existing, and much larger than the window #2806 closes.** Filed so the fix's scoped invariant comment does not read as covering it.
## The window
`StartWithBackoff` sleeps between recovery attempts (`pkg/lifecycle/service.go:262-266`), and during that sleep `runningPipelines[id]` deliberately holds the **dead** pre-recovery run — that is what makes the pointer guard at `:270` able to detect a concurrent restart. Meanwhile the pipeline's status is `StatusRecovering`, and `Stop` admits `StatusRecovering` as well as `StatusRunning` (`:299`).
Window size: `MinDelay`..`MaxDelay` — **1 second to 10 minutes**. #2806's publication window was microseconds.
## Observed
```text
during backoff: status=Recovering, map entry tomb Alive()=false
Stop() during recovery backoff returned: source node is not running
pipeline restarted after a successful Stop: true (status now Running)
```
**The pipeline restarted after a stop was requested.** `StartWithBackoff`'s guard is a bare pointer comparison; nothing anywhere records that a stop was asked for, so the sleep elapses and `Start` runs regardless.
## Why it matters at shutdown (invariant 7)
`StopAll` swallows the error into a log warning (`:366-372`), `runtime` then calls `ls.Wait(exitTimeout)` which resolves instantly off the dead tomb, and shutdown proceeds to quiesce the persister and close the DB — while the backoff timer is still pending. When it fires, `Start` runs with `ctx = context.Background()` (the cleanup goroutine's fresh context, `:911`) against a database the runtime is closing.
## Likely shape of a fix
A stop-requested flag on `runnablePipeline` (or a cancellable backoff context) that `StartWithBackoff` checks after its sleep, so a stop during backoff cancels the restart instead of racing it. Needs its own design pass — the interaction with the recovery-attempt counter and the pointer guard is not obvious.
Tier 1: data path, default engine, invariant 7. Related: #2806, #2809.
Contributor guide
Research direction
Read pkg/lifecycle/service.go around StartWithBackoff (262-270), Stop (299), StopAll (366-372), and the cleanup context at :911. Reproduce the recovery-backoff and shutdown sequence described in the issue, then trace the recovery-attempt counter and pointer guard. Done means a stop requested during backoff prevents the pending restart and shutdown cannot race a database close.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend, data-engineering
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100