Re-recruit an old epoch's backup worker when it dies by saintstack · Pull Request #14240 · apple/foundationdb

saintstack · 2026-10-07T02:55:16Z

#13712 stopped old-epoch backup worker failures from triggering a recovery, to break a recovery loop. Recovery is the only code that recruits a backup worker, so a worker that died while draining an old generation's tail was never replaced. In a healthy cluster no recovery follows: oldestBackupEpoch stays pinned, TLog retention grows without bound, the backup never becomes restorable, and FULLY_RECOVERED is never reached again.

This PR has the cluster controller replace a dead old-epoch backup worker directly, without starting a recovery. The replacement resumes from the dead worker's last saved version and takes over its slot.

Changes

Re-recruitment. monitorOldEpochBackupWorker watches each old-epoch worker. On failure it checks durable progress: it releases the slot if progress covers the range or no backup needs it, and otherwise recruits a replacement from savedVersion + 1 and swaps it into the slot in place (LogSystem::replaceBackupWorker). Retries back off up to 30s; it never gives up, since abandoning the range would re-pin the generation. Both Backup V2 variants share one templated monitor.
Knob. CC_RERECRUIT_BACKUP_WORKER_ENABLED (default true) restores the wait-for-next-recovery behaviour without a code change.
Replaced-but-alive workers. A worker declared dead may still be running. It now exits once ServerDBInfo drops it from its slot, and the monitor republishes registration after every replace or release so that happens promptly. If it finishes anyway, its done request releases whichever replacement holds the slot. A draining worker that finds its range popped exits rather than publish a log file claiming versions it lacks; it logs SevError unless durable progress accounts for the pop.
pop() is a no-op for an epoch that was retired while the worker still ran.

Testing

slow/BackupOldEpochWorkerFailure.toml forces recoveries during a partitioned-log backup, then restores and runs Cycle's check over the restored data. Three buggify injections drive the paths simulation never reaches on its own: a kill before the worker's first durable commit, a controller that replaces a worker that is still running, and a worker that learns of its replacement late.

100k simulation runs of the BackupOldEpochWorkerFailure test:

build	runs	failures
without re-recruitment	47,207	10 (pinned generation)
this PR	99,985	1 (`TracedTooManyLines` under heavy injected disk faults; no new injection active)

Code probes from the second run: 11,171 kills, 19,280 re-recruitments, 9,880 replacements of a still-running worker, 9,599 fences, 758 late fences, 0 stalls, 0 failed recruitments. Before the pop() change the late-fence injection hit ASSERT_WE_THINK(backupEpoch == oldestBackupEpoch) 10 times in 36,003 runs; after it, 0 in 99,985.

Caveats

The range-partitioned instantiation compiles but no test runs it.
The popped-range exit is not reached in this test: the backed-up tail drains in a single pass, so a predecessor never pops part of it. Its probe is marked rare.
BackupData::progressSavedDurably exists only for the pre-commit kill.
The monitor also runs on normal completion: a worker that exits cleanly fires waitFailureClient, and the monitor releases the slot from durable progress. This advances oldestBackupEpoch sooner than waiting for the next recovery.

tclinkenbeard-oai

Generated by Codex.

What problem is this PR trying to solve? (ELI10)

After recovery, FoundationDB may leave a backup helper copying the last changes from an older set of transaction logs. If that helper dies, nothing replaces it until another recovery happens.

Illustrative example: the database is healthy again, but one helper dies halfway through its remaining work. The database keeps the old logs because the backup still needs them. Without another recovery, that unfinished work can prevent the backup from becoming restorable and keep the cluster from reaching FULLY_RECOVERED.

How does this PR solve the problem? (ELI10 and review walkthrough)

The controller watches these helpers and tries to replace a failed helper directly. It reads the helper’s saved progress to decide whether to resume copying or release its place in the old log set.

Start a monitor after recruiting each old-epoch worker. The monitor belongs to the current recovery, so its lifetime must end when that recovery is replaced.
Check durable progress and whether any backup still needs the range. Completed or unnecessary work releases its slot; unfinished work resumes after the last durably saved version.
Recruit and install the replacement. Replacing the slot in place keeps the old logs retained and handles a replacement that reports completion before installation.

That is the intended repair for the example above. The ownership handoff and retry termination still have the problems below.

What is it trying to do?

Repair failed old-epoch backup workers without restarting transaction-system recovery, for both partitioned and range-partitioned Backup V2.

Is it correct?

Not yet. The inclusive end-version checks, durable-progress resume calculation, slot retention, completion-before-install handling, and recovery-scoped cancellation look consistent with the surrounding code. Existing request and durable-record formats are unchanged.

However, the replacement can race a still-live predecessor, and retries can permanently stop while work remains.

Reviewed head 1f073f14a712bf47ae624851bb7593e2ee9258a8 against base 65c8a2c4b291820ef1e26101e164c9a5a3546c91. No builds, tests, or simulation were run. At review time, current-head checks were passing or pending.

Are there bugs?

[P1] Prevent the predecessor from popping data after the replacement’s progress snapshot. The new request retains the same recovery epoch and uses the earlier progress snapshot. Failure detection can suspect a live worker during a controller-to-worker partition, while checkRemoved() only stops it after a newer recovery. It can therefore commit and pop additional mutations after the monitor reads progress.

For example, the monitor reads saved version 100, then the original saves through 149 and pops through that boundary. The replacement starts at 101 and encounters missing mutations. The partitioned worker logs this but continues—its assertion is ASSERT(true). For an existing backup, the file’s advertised beginning remains the requested start. A subsequent file can claim coverage of versions 101–200 while omitting 101–148. Restore filtering can discard the original valid smaller file in favor of that incomplete containing file, losing mutations during restore.

Fence the predecessor’s ability to advance/pop before choosing the resume point, or explicitly stop and reconcile a replacement that encounters popped data before publishing a file. Updating the worker list alone does not stop a partitioned predecessor or its buffered uploads.
[P2] Keep a repair path after ten attempts. The attempt cap returns permanently, leaving the dead slot and old generation pinned. Successful replacements and durable forward progress do not reset the count. If a transient fault clears after the tenth failure, the healthy cluster still cannot repair the range without another recovery—the original failure mode. The configured delay defaults to only 0.4 seconds, so this need not represent a prolonged outage. Retain a retry path with capped backoff and suppressed warnings instead of abandoning the range.

Are there omissions?

The new failure injection kills the worker outright before durable progress. It does not cover a still-live predecessor advancing during replacement, or eventual repair after more than ten failures.

Add coverage for those behaviors and verify restored contents. Despite its restoreAfter option, the selected Backup workload performs backup operations only; this test has no restore workload. It also selects only partitioned-log backup, leaving range-partitioned replacement unexercised.

Are there better ways of doing things?

Keep the repair outside transaction-system recovery, but make the worker handoff safe against failure suspicion and use bounded retry frequency instead of a finite lifetime attempt budget.

Should this CL be LGTMd?

Not yet. Fix the concurrent-worker/popped-data race and preserve eventual repair after transient repeated failures, with regression coverage for those paths.

apple#13712 removed old-epoch backup worker failure from the conditions that trigger a recovery, to stop a recovery loop. Recovery is the only code that ever recruits a backup worker, so after that change a draining old-epoch worker that died was never replaced. Its range simply waited for the next recovery, and in an otherwise healthy cluster no recovery came. The generation is then pinned indefinitely: oldestBackupEpoch never advances, so TLog retention grows without bound, the backup never becomes restorable, and FULLY_RECOVERED is permanently out of reach. Recruit the replacement in place from the cluster controller instead, so the repair does not reintroduce the loop apple#13712 removed. The cluster controller already holds what is needed to recruit one worker; no recovery is forced and no new recovery trigger is added. monitorOldEpochBackupWorker watches each old-epoch worker with waitFailureClient and, on failure, recruits a single replacement for that range and swaps it into the log set in place. Two conditions bound it. It consults getMinBackupVersion first, so a range no backup needs is released rather than re-recruited forever; without that a range no backup wants is re-recruited indefinitely, measured at 13,059 replacements for one range. And attempts are rate limited on both the failure and the success path, so a replacement that is recruited and then stops answering cannot spin at RPC speed. The attempt cap is a constexpr rather than a knob. Nothing is needed to stop monitors accumulating across recoveries. clusterWatchDatabase drops the per-recovery actorCollection that owns them, so a monitor cannot observe a generation later than the one that recruited it, and the next recovery starts fresh monitors for whatever is still undrained. Both Backup V2 variants are covered. They recruit old-epoch workers with the same shape and share one backupWorkers list per log set, so the monitor is templated over the request type rather than duplicated; the variants differ only in which progress keyspace to read and which endpoint to recruit through. Backup V1 is unaffected, since FileBackupAgent only enables backup workers for the partitioned and range-partitioned log types. Nothing covered this case, so add slow/BackupOldEpochWorkerFailure.toml with a buggify-gated injection that kills a draining old-epoch worker between its first pull and its first durable progress commit. BackupData::progressSavedDurably tracks that window; savedVersion cannot, because onBackupChanges raises it in memory before anything is committed. The test provokes recoveries by changing the resolver count rather than by killAll, so the adversary that causes the failure is not also killing the processes that would repair it -- with killAll the replacement workers become new victims and no repair can win.

…r a transient fault A replacement reuses the dead worker's recruitedEpoch, and checkRemoved only displaces a worker once recoveryCount advances, so a predecessor merely suspected dead keeps running and can pop past the snapshot the monitor recruited from. The replacement then peeked popped data and only logged it -- ASSERT(true) is a no-op -- and for an existing backup the file's begin version is max(savedVersion, startVersion), not the first real mutation. It would advertise coverage it lacks, and filterDuplicates prefers a containing file over the smaller valid one it supersedes, so the gap restores as empty. Exit instead. The attempt budget returned permanently once exhausted, with no reset on progress and a flat 0.4s brake, so ten attempts burned in four seconds and left the generation pinned -- the failure this monitor exists to repair. Replaced with backoff capped at 30s that resets on durable progress, warning once at the old threshold. Adds a probe on the exit and a restore phase with a Cycle check, since nothing validated restored contents. The buggify that recruits behind durable progress does not fire yet: over 40 seeds the drain commits at startVersion - 1 then endVersion, so savedVersion is never strictly inside the window. It needs a drain of more than two upload passes.

…replaced workers CC_RERECRUIT_BACKUP_WORKER_ENABLED (default true) turns off in-place re-recruitment and its simulation kill, restoring the wait-for-next-recovery behaviour without a code change. A replaced worker that is still alive now exits once ServerDBInfo drops it from its slot, and the monitor republishes registration after every replace or release so that happens promptly. A done request from a replaced worker releases whichever replacement holds the slot. A draining worker that finds its range popped logs SevError again unless durable progress accounts for the pop. Simulation replaces still-running workers and delays their fencing, under buggify, so the fence and the chained release are exercised.

foundationdb-ci · 2026-10-11T03:01:59Z

Result of foundationdb-pr-clang-ide on Linux RHEL 9

Commit ID: f9eaf1f
Duration 0:20:42
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-10-11T03:26:16Z

Result of foundationdb-pr-clang on Linux RHEL 9

Commit ID: f9eaf1f
Duration 0:45:01
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-10-11T03:29:17Z

Result of foundationdb-pr-clang-arm on Linux RHEL 9

Commit ID: f9eaf1f
Duration 0:48:02
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-10-11T03:34:00Z

Result of foundationdb-pr-macos-m1 on macOS 14.x

Commit ID: f9eaf1f
Duration 0:52:42
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-10-11T03:35:47Z

Result of foundationdb-pr on Linux RHEL 9

Commit ID: f9eaf1f
Duration 0:54:28
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2026-10-11T04:18:46Z

Result of foundationdb-pr-cluster-tests on Linux RHEL 9

Commit ID: f9eaf1f
Duration 1:37:30
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)
Cluster Test Logs zip file of the test logs (available for 30 days)

foundationdb-ci · 2026-10-11T08:12:14Z