Do not lose a bulkload task when its data move loses the destination team - #13873
Open
saintstack wants to merge 4 commits into
Open
Do not lose a bulkload task when its data move loses the destination team#13873saintstack wants to merge 4 commits into
saintstack wants to merge 4 commits into
Conversation
added 4 commits
August 14, 2026 17:51
…team A 10B backup/restore validation lost 935,560 key-values (889 MB) while reporting success. A content fingerprint taken before the backup and after the restore disagreed by 0.99x exactly one bulkload task, the trace events named that task, and reading its key range on the restored cluster returned nothing while the range immediately below it returned keys. Two independent problems produced that. A bulkload data move whose destination team becomes unhealthy was treated as permanently failed: the relocator acked the task unretryable, which marks it Error and abandons it. For an ordinary data move that is harmless, since the shard stays put and data distribution decides again later, but a bulkload task's key-values exist only in the dump until an attempt ingests them, so abandoning the task removes that range from the database with no other copy. An unhealthy destination team is a scheduling condition, not a property of the data. It is now acked retryable, and doBulkLoadTask leaves the persisted state alone and throws, so scheduleBulkLoadTasks re-dispatches the task on its next scan -- the recovery path an abandoned or timed-out task already takes. Re-dispatch has to unpublish the task first. publishTask deliberately refuses a task whose taskId is already published, so a task cannot be triggered twice; without erasing the entry every later dispatch fails as bulkload_task_outdated and the task spins between the scheduler and publishTask without moving data. That was measured before the fix: 7102 dispatch attempts, 3548 of them outdated, zero tasks completed, and a simulation running 9124 seconds where the same seed completes in 23. Re-dispatch is bounded by BulkLoadTaskState::restartCount, which is persisted with the task, advances on every re-trigger from any cause, and is already reported as RestartCount for stalled tasks. Bounding it bounds a task's total thrash rather than destination team failures alone. The limit is a constant in the function, matching the backstop timeout beside it: the existing retry knobs cover other layers -- dump file downloads, retries inside one move-keys call, rebalance -- so reusing one would mislead whoever tuned it next, and the bound exists to stop an endless loop rather than to be tuned per cluster. Second, the restore reported success regardless. monitorBulkLoadJobCompletionWithProgress returned true as soon as the job was no longer running, and a job whose range has been fully walked clears its live metadata even when a task ended in Error, archiving it to history with an Error phase in the same transaction. So "no longer running" meant "finished walking", not "every task succeeded". It now reads the archived phase and fails with restore_bulkload_failed when the job ended in Error. That read needs care. getBulkLoadJobFromHistory reads the system key space and a restore holds the database lock while it runs, so a plain transaction there fails every attempt with database_locked and its retry loop never terminates -- which hung two ensemble runs for their full 3600s timeout before this was found. The helper now sets READ_SYSTEM_KEYS always and LOCK_AWARE on request; its fdbcli caller takes the default and is unchanged. The call site also does not depend on the read succeeding: every task reported done by then, so failing a good restore because the outcome could not be re-read is worse than proceeding, and an unreadable history is reported as SevWarnAlways BulkLoadRestoreJobOutcomeUnknown instead. That weakens the check exactly when it cannot be evaluated, which is a deliberate trade for not hanging. The two halves are load-bearing for each other: giving up on a task after exhausting retries is only safe because a job ending in Error now fails its restore rather than reporting completion.
Reaching that path by chance needs a team to become unhealthy inside the window a bulkload move is in flight, which is too rare to rely on, so BULKLOAD_SIM_INJECT_DEST_TEAM_FAILURES injects a set number of them. It is 0 everywhere else, so no other test changes behaviour. A count rather than a probability: a per-poll probability either never fires on short moves or keeps firing until the task gives up, and neither exercises recovery. The budget is spent on a single task, because the give-up path bounds one task's restartCount and a budget spread across tasks retries each a few times without ever reaching it -- a first attempt that spread 40 injections across the workload's tasks left the give-up path uncovered. Two tests, both driving BulkLoadingWorkload, which compares every restored key-value against what it wrote: BulkLoadingDestTeamFailure injections under the budget, retries win BulkLoadingDestTeamFailureExhausted injections over it, one task gives up Neither needed a workload change. BulkLoadingWorkload already collects error tasks and excludes their ranges before comparing, so a task that legitimately gives up does not fail the run; the assertion is that the load terminates and the surviving ranges are correct. Seed 12345: the first test takes 26s against 23s uninjected with 3 injections and 3 re-dispatches; the second takes 24s with 21 injections, 20 re-dispatches and 1 give-up. Neither logs bulkload_task_outdated or a data difference. Code probes report the injection and re-dispatch paths covered, and the give-up path covered by the second test. Rejected, and worth not repeating: shortening health_poll_time to widen the injection window. At 0.1s the run's tail stopped completing at all, even with no injection. This covers the data distribution half. That a restore whose job ends in Error fails with restore_bulkload_failed rather than reporting success still has no test: it needs a restore-level workload, and BackupS3BlobBulkLoadRestore.toml, which would host it, does not pass on macOS locally.
…ing SevError for it An ensemble run at 178a339824 showed the job-outcome check working -- the history read succeeded and reported JobPhase Error -- but handling that outcome wrong in two ways. It threw restore_bulkload_failed and left the restore retryable, so the task ran again, re-read the same finished job, found the same Error, and threw again: eleven attempts across 1237 simulated seconds until the test hit its 3600s timeout. The condition is terminal by nature, so it now aborts the restore the way an incompatible backup already does in this file -- logError, set the restore state to ABORTED, commit, then throw. It also traced at SevError. A task that could not be loaded is a property of the data and the cluster, not a defect in the code, and a SevError additionally fails any simulation that provokes the condition, so a legitimately detected loss would be indistinguishable from a bug. It is SevWarnAlways now; the aborted restore and the thrown error are the signal. Worth recording what that run also showed, since it is not mine to fix: the job ended in Error because a bulkload task could not find a valid destination team (DDBulkLoadEngineTaskGetTeamFailedToFindValidTeam, ValidTeamSize 0 of 10 with all ten duplicated). tests/fast/BulkLoading.toml documents that exact stall and guards against it with pinned redundancy and extra storage machines; BackupS3BlobBulkLoadRestore.toml appears not to, so it provokes a real task failure. Before this change that produced a restore reporting success with data missing; now it produces a failed restore, which is correct but means that test needs its topology fixed to pass.
These three tests could not supply a destination team disjoint from the source, so bulk-load team selection failed, tasks were marked Error, and the restore was incomplete. An ensemble run shows it plainly: DDBulkLoadEngineTaskGetTeamFailedToFindValidTeam fifty times with TeamSize 10, ValidTeamSize 0 and DuplicatedCount 10 -- every candidate team shared a server with the source -- followed by four tasks marked Error. That is the stall tests/fast/BulkLoading.toml documents at length and guards against with dedicated storage-class machines. These tests set extraMachineCountDC, which adds ordinary machines and does not supply that capacity, so they add extraStorageMachineCountPerDC as well. Per DC, so the multi-region variants keep their region coverage. Until now this went unnoticed because a restore whose bulkload job ended in Error still reported success: the tests were restoring incomplete data and passing. The check added earlier in this branch turns that into a failed restore, which is how the condition surfaced -- ten runs across all three variants, every seed. Verified at the seeds that failed in the ensemble: 493071098 for the plain variant, 3780838088 for MultiRange, 3134547887 for WithChaos. All three now pass, each with zero GetTeamFailedToFindValidTeam, zero tasks marked Error, and the restore completing. The plain variant had never passed on macOS before this.
Contributor
Result of foundationdb-pr-clang-ide on Linux RHEL 9
|
Contributor
Result of foundationdb-pr-clang-arm on Linux RHEL 9
|
Contributor
Result of foundationdb-pr-clang on Linux RHEL 9
|
Contributor
Result of foundationdb-pr on Linux RHEL 9
|
Contributor
Result of foundationdb-pr-macos-m1 on macOS 14.x
|
Contributor
Result of foundationdb-pr-cluster-tests on Linux RHEL 9
|
Contributor
Result of foundationdb-pr-macos on macOS 14.x
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A bulkload restore could lose a task's data and still report success. This fixes that, and adds
simulation coverage for the path that produced it.
Found by a 10B backup/restore validation using the RangeDigest content fingerprint (separate PR).
The fingerprint taken before the backup and after the restore disagreed by 935,560 key-values /
889 MB — 0.99× exactly one bulkload task — while the restore reported
103786/103786complete.Trace events named the single task and its error, and reading that key range on the restored cluster
returned nothing while the range immediately below it returned keys.
The two problems
A recoverable data move failure was treated as permanent. When a bulkload data move's destination
team became unhealthy, the relocator acked the task
unretryableError, which marks itErrorandabandons it. For an ordinary data move that is harmless — the shard stays put and data distribution
decides again later — but a bulkload task's key-values exist only in the dump until an attempt
ingests them, so abandoning the task removes that range from the database with no other copy. An
unhealthy destination team is a scheduling condition, not a property of the data.
It is now acked retryable, and the task is re-dispatched through the path an abandoned or timed-out
task already takes. Two details matter:
publishTaskdeliberately refuses a task whosetaskIdisalready published, so without erasing the entry every later dispatch fails as
bulkload_task_outdatedand the task spins between the scheduler andpublishTaskwithout movingdata. Measured before that was added: 7102 dispatch attempts, 3548 outdated, zero tasks completed,
a simulation running 9124 seconds where the same seed completes in 23.
BulkLoadTaskState::restartCount, which is already persisted with thetask and reported as
RestartCountfor stalled tasks. The bound is a constant in the function,matching the backstop timeout beside it; the existing retry knobs cover other layers (dump file
downloads, retries inside one move-keys call, rebalance), so reusing one would mislead whoever
tuned it next.
The restore reported success regardless.
monitorBulkLoadJobCompletionWithProgressreturned trueas soon as the job was no longer running — but a job whose range has been fully walked clears its
live metadata even when a task ended in
Error, archiving it to history with anErrorphase in thesame transaction. So "no longer running" meant "finished walking", not "every task succeeded". It now
reads the archived phase and fails with
restore_bulkload_failed.The two halves are load-bearing for each other: giving up on a task after exhausting retries is only
safe because a job ending in
Errornow fails its restore rather than reporting completion.Second finding: three existing tests were restoring incomplete data and passing
With the check in place,
BackupS3BlobBulkLoadRestore.tomland its MultiRange and WithChaos variantsbegan failing — 10 runs, every seed. The cause was not this change:
Every candidate team shared a server with the source, so team selection failed at dispatch and
tasks genuinely could not load.
tests/fast/BulkLoading.tomldocuments that stall and guards againstit with
extraStorageMachineCountPerDC; these three set onlyextraMachineCountDC, which addsordinary machines and supplies no storage-class capacity. Before this PR those runs restored
incomplete data and passed. The last commit adds the missing capacity, per DC so the multi-region
variants keep their coverage.
Tests
tests/fast/BulkLoadingDestTeamFailure.tomland…Exhausted.tomldriveBulkLoadingWorkload, whichcompares every restored key-value against what it wrote, while
BULKLOAD_SIM_INJECT_DEST_TEAM_FAILURESinjects a set number of destination-team failures. The knobis 0 everywhere else. A count rather than a probability: a per-poll probability either never fires on
short moves or keeps firing until the task gives up, and neither exercises recovery. The budget is
spent on one task, because the give-up path bounds a single task's
restartCount.The first test keeps injections under the budget so retries win; the second exceeds it so a task
gives up. Neither needed a workload change —
BulkLoadingWorkloadalready excludes ranges whose taskended in
Errorbefore comparing, so the assertion is that the load terminates and surviving rangesare correct.
Code probes confirm the injection, re-dispatch and give-up paths are all covered.
Verification
BackupS3Blob*variants pass at the seeds that previously failed (493071098,3780838088, 3134547887), each with zero
GetTeamFailedToFindValidTeamand the restore completing.Note for review
When the archived job phase cannot be read, the restore proceeds and logs
SevWarnAlways BulkLoadRestoreJobOutcomeUnknownrather than failing. Every task reported done bythat point, so failing a good restore because the outcome could not be re-read seemed worse — but it
does weaken the check exactly when it cannot be evaluated, and a reviewer may prefer failing closed.
20260815-005816-stack_retry-1c455e33f328a2a0 compressed=True data_size=38626587 duration=4106113 ended=100000 fail_fast=10 max_runs=100000 pass=100000 priority=100 remaining=0 runtime=0:29:17 sanity=False started=100000 stopped=20260815-012733 submitted=20260815-005816 timeout=5400 username=stack_retry