Forward port #14057: fix backup-epoch livelock that strands old TLog generations

[saintstack]

Forward port of #14057 (release-7.4), a nightlies BackupOldAndNewRestore failure. Same change, translated for main's layout (backupworker/BackupWorker.cpp, logrouter/LogRouter.cpp, logsystem/LogSystem.cpp) and native coroutines. Main has all three gaps unchanged: the LogRouter counter bug at LogRouter.cpp:430, no progress seed in uploadData, and a silent return in monitorBackupProgress.

Here is the 7.4 branch commit message:

A backup worker displaced before its first upload cycle leaves its epoch with
no progress record. monitorBackupProgress then retries forever, so the epoch
never completes and permanently pins oldestBackupEpoch. That blocks
generation purging, and since finalUpdate requires an empty oldTLogData, a
single stranded generation keeps the cluster out of FULLY_RECOVERED. In
simulation the test simply never finishes.

Fix: seed a progress record when uploadData starts. The seeded value is
savedVersion (= startVersion - 1), so it advertises no data.

Also included: a SevWarnAlways in LogRouter that fired on every successful
peek (noPrimaryPeekLocation = present ? 0 : ++n; if (!(n % 4)) — 0 % 4 == 0),
plus traces at the three points where this wedge was previously silent.

Not fixed: recoverAt is still not persisted to cstate, which is a second,
independent prerequisite for old-generation tracking. Enabling
RECORD_RECOVER_AT_IN_CSTATE activates an ordering assertion at
TagPartitionedLogSystem.actor.cpp:358 -- newState.oldTLogData[j].recoverAt < oldestGenerationRecoverAtVersion won't. -- that has never executed, and it fails
immediately (KillRegionCycle.toml seed 90374059). Will address separately — the
wedge remains reachable; this change only reduces how often it is entered.

Review note: saveProgress does not check stopped, so a worker displaced
inside this first commit window now blocks teardown on a commit that needs the
new recovery to complete. Previously it exited locally. Deliberately not coded
around — the alternatives either change saveProgress for all callers or drop
the record this fix exists to write.

20260918-021822-stack_forward_port-3f64643f10834290 compressed=True data_size=39591687 duration=4300037 ended=100000 fail_fast=10 max_runs=100000 pass=100000 priority=100 remaining=0 runtime=0:28:21 sanity=False started=100000 stopped=20260918-024643 submitted=20260918-021822 timeout=5400 username=stack_forward_port

[spraza]

Looks good. I'll let @neethuhaneesha have a look too since it's related to backup worker.

[neethuhaneesha]

In the description it was mentioned "monitorBackupProgress then retries forever, so the epoch never completes and permanently pins oldestBackupEpoch',
But looks like this will not runs for old epochs.

I can understand the some old generation are not able to purge because there is no error monitoring control. But, I quite didn't get how the fix is help in purging old generations. We can discuss offline.

[foundationdb-ci]

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

• Commit ID: 7606416
• Duration 0:32:52
• 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]

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

• Commit ID: 7606416
• Duration 0:42:16
• 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]

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

• Commit ID: 7606416
• Duration 0:50:57
• 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]

Result of foundationdb-pr-clang on Linux RHEL 9

• Commit ID: 7606416
• Duration 1:03:35
• 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]

Result of foundationdb-pr on Linux RHEL 9

• Commit ID: 7606416
• Duration 1:11:29
• 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]

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

• Commit ID: 7606416
• Duration 1:12:36
• 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)

[saintstack]

In the description it was mentioned "monitorBackupProgress then retries forever, so the epoch never completes and permanently pins oldestBackupEpoch',
But looks like this will not runs for old epochs.

Thanks for the review @neethuhaneesha. You are right. monitorBackupProgress only looks at current. The PR description needs to do better job describing the problem:

A recovery can't finish while any old TLog generation is still retained, and a
generation stays retained while a backup worker for its epoch is still outstanding.

A backup worker records its progress only once per 10s upload cycle, and the first
pass through that cycle has nothing to write yet. If a recovery displaces the worker
before the second pass, its epoch is left with no progress record at all.

That is worse than a record saying zero. A tag with no record is re-recruited
unconditionally at the next recovery, whereas a tag that has one is dropped as soon
as the record shows it caught up (BackupProgress.cpp:138-139 vs :57-58). So with
no record the epoch is never retired by recovery — only by a worker that survives a
full upload cycle.

In the failing run, recoveries arrived every ~9.6s against that 10s cycle, so no
worker ever survived one. Epoch 6 was re-recruited 28 times and never completed, 26
generations piled up, and the run died on TracedTooManyLines.

Fix

Write one progress record as soon as uploadData starts, before the first delay
(BackupWorker.cpp:855). Displacement now leaves a record behind instead of nothing.

Records merge by max per (epoch, tag), so the recorded version never goes backwards.
The next recruitment for that epoch begins at max(record + 1, epoch begin) rather
than unconditionally at the epoch's begin version, so whatever a displaced worker had
established is not re-done. Once every tag of an epoch has a record covering the
epoch's last version, nothing is recruited for that epoch, oldestBackupEpoch moves
past it, and the generation can be dropped.

The recorded value can't overstate what's in the backup container: it is the worker's
savedVersion, the highest version it has actually uploaded (startVersion - 1
before it uploads anything), which is the same number it would have written one cycle
later. An epoch that genuinely still holds mutations a backup needs therefore still
requires a worker to upload them.

Also

• A LogRouter warning fired on success rather than failure (0 % 4 == 0).
• Traces at three points where this wedge was previously silent.

Not fixed

recoverAt is still not persisted to cstate. RECORD_RECOVER_AT_IN_CSTATE defaults
off, so every generation read back has recoverAt == 0 and newEpoch's bail-out
disables old-generation recovery tracking on each recovery — TRACK_TLOG_RECOVERY is
on but self-disabling. That is a separate defect, not a second blocker here:
measured, enabling the knob left retention unchanged. Enabling it by default also
tripped an ordering assertion (LogSystem.cpp:520) on KillRegionCycle.toml seed
90374059, so it needs its own change.

This reduces how often a generation is stranded; it does not remove the failure mode.
Residual runs still show recovery stuck on a single retained generation
(OldTLogs=1 FinalUpdate=0) even where oldestBackupEpoch advanced normally.

Review note

The new write happens at worker startup. If a recovery displaces the worker while
that commit is in flight, the commit can't complete until a transaction system exists
again, and saveProgress retries without bound — so the worker's teardown waits on
it. Previously there was no write at that point and teardown was immediate. Checking
stopped wouldn't help: the worker isn't stopped yet when the commit starts.

Left unbounded deliberately. A timeout() would cap teardown and still land the
record in the common case — happy to add that if you'd prefer.

Testing

Joshua 20260918-021822-stack_forward_port-3f64643f10834290: 100,000 runs, 0
failures — but a 115-test ensemble, so this test received only a fraction of them.
The mechanism is traced; the effect size is not measured.

I just updated the comments in the PR too to be clearer on what is going on.

Here are line numbers to help review....

A tag whose record reaches epochEnd - 1 is dropped from the recruit set (epochEnd is endVersion in the below method) from BackupProgress:

48 void BackupProgress::updateTagVersions(std::map<Tag, Version>* tagVersions,
 49                                        std::set<Tag>* tags,
 50                                        const std::map<Tag, Version>& progress,
 51                                        Version endVersion,
 52                                        Version adjustedBeginVersion,
 53                                        LogEpoch epoch) {
 54   for (const auto& [tag, savedVersion] : progress) {
 55     // If tag is not in "tags", it means the old epoch has more tags than
 56     // new epoch's tags. Just ignore the tag here.
 57     auto n = tags->erase(tag);
 58     if (n > 0 && savedVersion < endVersion - 1) {
 59       const Version beginVersion =
 60           (savedVersion + 1 > adjustedBeginVersion) ? (savedVersion + 1) : adjustedBeginVersion;
 61       tagVersions->insert({ tag, beginVersion });
 62       TraceEvent("BackupVersionRange", dbgid)
 63           .detail("OldEpoch", epoch)
 64           .detail("Tag", tag.toString())
 65           .detail("BeginVersion", savedVersion + 1)
 66           .detail("AdjustedBeginVersion", beginVersion)
 67           .detail("EndVersion", endVersion);
 68     }
 69   }
 70 }

A tag not in 'progress' -- has no progress -- makes it out of BackupProgress::updateTagVersions and then gets unconditionally re-added at the epoch's begin version later in BackupProgress::getUnfinishedBackup (:137-138). An epoch with no record can never be retired by recovery — only by a worker that survives long enough to write one.

But in this test we are trapped in recoveries that are triggering every 9.6 seconds... Backup worker can't land an update before 14 seconds have passed (the 4second pause at start then after the first 10second update cadence elapses).

This fix updates progress record once for the worker when uploadData starts, before the loop's first delay. shouldBackupWorkerExitEarly runs onBackupChanges before uploadData is launched. For an old-epoch worker that has saved nothing yet that advances savedVersion to minVersion, the lowest version any active backup still needs (BackupWorker.cpp:409-415).

Now the tag has a version.

The erase quoted above ' 57 auto n = tags->erase(tag);' now works.

See what you think.

[foundationdb-ci]

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

• Commit ID: 7606416
• Duration 1:49:24
• 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)

[saintstack]

Same comment as for 7.4 version of this PR:

Making this a draft. #14240 might take care of this case. At least, the test there uses the circumstance this PR tries to fix as the test scenario to prove #14240 works for the most extreme (this) case.