Back up a range that contains the test data in two bulkload tests by saintstack · Pull Request #14322 · apple/foundationdb

saintstack · 2026-10-19T22:36:26Z

BackupS3BlobCorrectness builds its backup ranges from random alphanumeric endpoints when backupRangesCount > 0:

rangeEndpoints.insert(deterministicRandom()->randomAlphaNumeric(
    deterministicRandom()->randomInt(1, backupRangeLengthMax + 1)));

The Cycle workload writes its keys through doubleToTestKey, which is format("%016llx", …) (fdbserver/core/WorkloadKeys.cpp) — 16 characters of lowercase hex. A random alphanumeric endpoint pair usually sorts entirely outside that keyspace: a measured draw was ["kSd", "s"), which contains no Cycle key at all.

So those runs backed up a range holding none of the test's data, and "backup and restore succeeded" asserted almost nothing. Worse, it is nondeterministic — whether a run covers data depends on the draw, and the draw moves with the simulated topology, so it shifts with unrelated configuration changes.

Two tests used that mode:

BackupS3BlobBulkLoadRestoreKeyRange — whose whole point is that a --keys-scoped bulkload round trip restores the requested range and nothing else. Its data half was vacuous whenever the draw missed.
BackupS3BlobBulkLoadIncrementalRejected — which inherited it; its refusal probes do not depend on data.

Every other BackupS3Blob* test uses backupRangesCount = -1 (normalKeys) and was never affected.

Fix

1. backupRangePrefix backs up exactly one prefix range, pointed at a prefix the Cycle workload writes under. The range stays narrow, which matters: the out-of-range sentinels that detect a restore writing outside the requested range need an outside to occupy. Confirmed still working — 3 sentinels planted.

2. Key the restore-range skip off the actual range count. shouldSkipRestoreRanges dropped one restore range when backupRangesCount > 1, but that is the option, not the number of ranges built. A single-range configuration that does not set the option inherits its default of 5, so the skip fired and removed the only range, leaving restore() to fail in its ranges.empty() check with a bare restore_error and no message. This commit is required for the first one; without it roughly 30% of runs (shouldSkipRestoreRanges fires at p = 0.3) would fail.

Behaviour is unchanged for every existing configuration: backupRangesCount = -1 yields 1 range (old -1 > 1 false, new 1 > 1 false) and = 5 yields 5 (both true). The two differ only when the option and the real count disagree, which is only the new prefix mode.

3. BS3BCW_Ranges now records the chosen ranges. It previously carried no details, so the single fact that decides whether a run covers any data was invisible in a trace. That is why this went unnoticed.

Testing

check	result
local sim: `BackupS3BlobCorrectness` seeds 101, 202	pass, 0 `Severity="40"`
local sim: `MultiBackupS3BlobCorrectness` seed 101	pass, 0 `Severity="40"`
`BackupS3BlobBulkLoadRestoreKeyRange` (RocksDB build)	pass — `BackupRange0="d/ - d0"`, backup/restore range counts 1/1, 3 out-of-range sentinels, 0 `BS3BCW_RestoreWroteOutsideBackupRanges`, bulkload used, 1 bulkdump data file
Joshua general ensemble on this sha	100000 / 100000 passed, 45:20 wall

BackupS3BlobCorrectness builds its backup ranges from random alphanumeric endpoints when backupRangesCount is positive. Cycle writes 16-character lowercase hex keys via doubleToTestKey (fdbserver/core/WorkloadKeys.cpp), so a random endpoint pair usually sorts entirely outside the keyspace the workload writes: a measured draw was ["kSd", "s"), which contains no Cycle key at all. Whether a run covered any data was therefore decided by the draw, and the draw moves with the simulated topology, so it changed with unrelated configuration. Two tests used that mode: BackupS3BlobBulkLoadRestoreKeyRange, whose point is that a --keys-scoped bulkload round trip restores the requested range and nothing else, and BackupS3BlobBulkLoadIncrementalRejected, which inherited it. In the first the data half of the assertion was vacuous whenever the draw missed, which is most of the time. Every other BackupS3Blob* test uses backupRangesCount = -1, covering normalKeys, and is unaffected. Add backupRangePrefix, which backs up exactly one prefix range, and point it at a prefix the Cycle workload writes under. The range stays narrow, so the out-of-range sentinels that detect a restore writing outside the requested range still have an outside to occupy, and the sentinels confirm it: 3 are still planted. Also record the chosen ranges on BS3BCW_Ranges, which previously carried no details. Which ranges a run picked determines whether it covers any data, and that was not visible in a trace. Verified on the rangefile path, which runs without RocksDB: BackupRange0="d/ - d0", 3 out-of-range sentinels planted, range files written, restore loads them, test passes on two seeds. The two bulkload tomls themselves need a RocksDB build to run.

shouldSkipRestoreRanges dropped one restore range when backupRangesCount was greater than 1, but that is the option, not the number of ranges actually built. A single-range configuration that does not set the option inherits its default of 5, so the skip fired and removed the only range, leaving restore() to fail with a bare restore_error from its ranges.empty() check -- with no message, since that path prints nothing.

nos-a · 2026-10-20T18:43:22Z

@@ -109,8 +110,11 @@ timeout = 7200
    performRestore = true
    # One random key range, so the backup covers a proper subrange of normalKeys. The bulk path accepts


Remove this comment?

nos-a · 2026-10-20T18:44:14Z

-    backupRangeLengthMax = 10
+    # One narrow range, but aligned with the data: Cycle writes 16-char lowercase hex keys
+    # (doubleToTestKey), which a random alphanumeric endpoint pair almost never overlaps, so the
+    # backup used to cover no data at all -- and whether it did was random per run. The


nit: I'd avoid putting history of past issues in code/config comments (prefer pr description or commit message)

nos-a · 2026-10-20T18:45:21Z


    [[test.workload]]
    testName = 'Cycle'
+    keyPrefix = 'd/'


This keyPrefix on Cycle and backupRangePrefix have to match, and nothing checks it. A cheap guard would be to assert after setup that the backup range holds at least one key, so a mismatch can't bring back a vacuous test. Unfortunately there's no way in TOML to have a single property shared between two sections.

Added assert in start rather than setup because workload setups run concurrently (Cycle data doesn't gets a chance to show up)

nos-a · 2026-10-20T18:47:21Z

@@ -120,6 +120,9 @@ struct BackupS3BlobCorrectnessWorkload : TestWorkload {
 	double backupStartAt, restoreStartAfterBackupFinished, stopDifferentialAfter;
 	Key backupTag;
 	int backupRangesCount, backupRangeLengthMax;


After this change no test uses the random-range mode (the other six tomls use -1, these two the prefix). Could the default of backupRangesCount become -1 (so setting neither is safe), plus an assertion that a prefix and a positive count aren't both set? Ideally I'd remove the random mode and the restore-range skipping if we don't need it anymore.

Did your latter suggestion... removed random mode.

No test uses the random-range mode any more, so drop it along with the restore-range skipping that only applied to it. Fail the workload at start when a backupRangePrefix range holds no key, so a mismatch with the data workload's key prefix cannot bring back a vacuous test.

foundationdb-ci · 2026-10-22T00:25:25Z

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

Commit ID: dc0c4a6
Duration 0:24: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)

foundationdb-ci · 2026-10-22T00:36:09Z

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

Commit ID: dc0c4a6
Duration 0:35:21
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-22T00:48:13Z

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

Commit ID: dc0c4a6
Duration 0:47:26
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-22T00:51:36Z

Result of foundationdb-pr-clang on Linux RHEL 9

Commit ID: dc0c4a6
Duration 0:50:44
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-22T00:56:13Z

Result of foundationdb-pr on Linux RHEL 9

Commit ID: dc0c4a6
Duration 0:55:25
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-22T01:23:59Z

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

Commit ID: dc0c4a6
Duration 1:23:08
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-22T01:49:12Z

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

Commit ID: dc0c4a6
Duration 1:48:13
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)

michael stack added 2 commits October 18, 2026 01:24

This comment has been minimized.

Sign in to view

saintstack mentioned this pull request Oct 20, 2026

Replay partitioned mutation logs on a bulkload restore #14324

Closed

This comment has been minimized.

Sign in to view

saintstack mentioned this pull request Oct 20, 2026

Replay partitioned mutation logs on a bulkload restore #14327

Open

This comment has been minimized.

Sign in to view

saintstack requested a review from nos-a October 20, 2026 16:05

nos-a reviewed Oct 20, 2026

View reviewed changes

nos-a requested changes Oct 20, 2026

View reviewed changes

saintstack requested a review from nos-a October 22, 2026 00:00

nos-a approved these changes Oct 22, 2026

View reviewed changes

		@@ -109,8 +110,11 @@ timeout = 7200
		performRestore = true
		# One random key range, so the backup covers a proper subrange of normalKeys. The bulk path accepts

Conversation

saintstack commented Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fix

Testing

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

foundationdb-ci commented Oct 22, 2026

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

Uh oh!

foundationdb-ci commented Oct 22, 2026

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

Uh oh!

foundationdb-ci commented Oct 22, 2026

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

Uh oh!

foundationdb-ci commented Oct 22, 2026

Result of foundationdb-pr-clang on Linux RHEL 9

Uh oh!

foundationdb-ci commented Oct 22, 2026

Result of foundationdb-pr on Linux RHEL 9

Uh oh!

foundationdb-ci commented Oct 22, 2026

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

Uh oh!

foundationdb-ci commented Oct 22, 2026

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

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

saintstack commented Oct 19, 2026 •

edited

Loading