ConduitIO / ConduitIO/conduit

lifecycle: the recovery-backoff window holds a dead run for up to 10 minutes while Stop admits StatusRecovering

Open
#2,810 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.