This parallel restore feature has been slated for removal for at least a year. This PR is closely based on earlier PR #12107.
This blog post explains some of the problems with the parallel restore feature: https://medium.com/@jingyuzhou/why-foundationdb-restore-is-slow-and-what-can-be-done-about-it-e73a821fdd33
As far as large feature removal changes go, this one is very straightforward, with most relevant files and test cases simply being deleted. There is one knob rename where storageserver.actor.cpp was using a knob with FASTRESTORE in the name. Other than that, changes to shared files mainly involve removing fastrestore-specific CLI and role support.
In progress:
20260330-222511-gglass-5ee0142213471b70 compressed=True data_size=35343375 duration=4611964 ended=100000 fail=1 fail_fast=1000 max_runs=100000 pass=99999 priority=100 remaining=0 runtime=0:58:23 sanity=False started=100000 stopped=20260330-232334 submitted=20260330-222511 timeout=5400 username=gglass
The one failure was in SwizzledCycleTest.toml with too many lines of output and a timeout. I kind of suspect it's unrelated but haven't looked further.
* Remove parallel restore feature. This is based on earlier PR 12107. Compiles but untested.
* AI generated commit:
⏺ The fix restores a single if block that was accidentally deleted when removing the FASTRESTORE_TOOL code:
if (!restoreSystemKeys && !restoreUserKeys && backupKeys.empty()) {
addDefaultBackupRanges(backupKeys);
}
When no explicit key ranges are specified on the command line and neither --user-data nor --system-metadata flags are set, this populates backupKeys with the default backup ranges
(essentially all user data). Without it, backupKeys stays empty and hits the ASSERT(!backupRanges.empty()) in submitBackup().
* Remove a believed-to-be-dead code path, and update .gitignore
* Remove duplicate definition of restoreRequestDoneKey
* RocksDB build improvements: version config file and fdbserver --version display
- Add cmake/RocksDBVersion.cmake for centralized RocksDB version configuration
- Support both release versions (ROCKSDB_VERSION) and git commit hashes (ROCKSDB_GIT_HASH)
- Auto-generate FDBRocksDBVersion.h at cmake configure time from template
- Display RocksDB version in fdbserver --version output
- Remove source-controlled FDBRocksDBVersion.h (now generated into build directory)
* reconcile with commit b1fbbd87
* Add --reseed-time option to reset random seed during simulation
Adds a new command line option --reseed-time that allows resetting the
deterministic random seed at a random time during simulation. When
provided with a time value (e.g., 100), the simulator will reset the
random seed to a truly random value at a random time between [0, time].
This feature enables testing different execution paths from the same
initial simulation state while maintaining determinism up to the reset
point.
Changes:
- Add resetSeed() method to IRandom interface with default no-op implementation
- Implement resetSeed() in DeterministicRandom class to reinitialize RNG
- Add --reseed-time command line option to fdbserver
- Add reseedTime parameter to simulationSetupAndRun()
- Add reseedRandomAtTime() actor to schedule seed reset
- Use platform::getRandomSeed() for truly random new seed value
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* TestHarness2: Add support for running tests from argument file
This commit adds the ability to run specific tests with predefined
parameters by passing an argument file to TestHarness2. This is useful
for reproducing specific test failures or running tests with exact
configurations.
Changes:
- Added --test-args-file option to config.py
- Implemented parse_test_args_file() to parse argument files containing
test parameters in the format:
'-f fast/CycleTest.toml -s 315315 -b off --reseed-time 100'
- Modified TestRun to accept and forward extra_args to fdbserver
- Modified TestRunner.run_tests() to:
* Accept extra_args and override_buggify parameters
* Skip test statistics tracking when running from args file
* Forward all unrecognized arguments to fdbserver command line
- Added example_test_args.txt demonstrating the format
The track_stats parameter prevents assertion failures when the specified
test is not in the test_picker's dictionary (which happens when running
a specific test that may not match the configured filters).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* Joshua: Support test_args.txt file for reproducing specific test runs
Modified correctnessTest.sh to automatically detect and use test_args.txt
file if present in the working directory. This enables easy reproduction of
specific test failures in Joshua by simply including a test_args.txt file
with the test parameters.
When test_args.txt exists, the script passes --test-args-file option to
TestHarness2, which will run the specific test with the exact parameters
specified in the file instead of randomly selecting a test.
Example test_args.txt content:
-f tests/fast/CycleTest.toml -s 315315 -b off --reseed-time 100
This allows developers to reproduce simulation test failures by copying
the exact test parameters from Joshua logs.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* CMake: Optionally include test_args.txt in correctness package
Modified create_correctness_package() to automatically include test_args.txt
in the correctness tarball if it exists in the source directory at CMake
configuration time.
This allows developers to create reproducible test packages by:
1. Creating test_args.txt in the source root with specific test parameters
(e.g., '-f tests/fast/CycleTest.toml -s 315315 -b off --reseed-time 100')
2. Running cmake to reconfigure
3. Running ninja package_tests to build the package
When the package is extracted and run in Joshua, correctnessTest.sh will
automatically detect test_args.txt and run the specific test instead of
randomly selecting one.
If test_args.txt doesn't exist, the package is built normally without it,
maintaining backward compatibility.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* TestHarness2: Disable determinism check when using test_args_file
Modified run_tests() to skip the unseed mismatch (determinism) check when
running tests from test_args.txt file. The determinism check now requires
track_stats=True, which means it only runs during normal random test
selection, not when running specific tests via --test-args-file.
Rationale:
- When reproducing a specific test failure via test_args.txt, users want to
run that exact test configuration once, not have it randomly run twice
- The determinism check adds overhead and complexity that isn't needed for
reproduction scenarios
- The check uses config.random which would make reproduction less predictable
This change makes test_args.txt behavior more focused and predictable for
debugging and reproduction workflows.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* TestHarness2: Include ResettingRandomSeed events in test output
Added a handler for ResettingRandomSeed trace events to include them in
the test summary output. This makes random seed reset operations visible
in Joshua logs and test results.
The handler captures:
- NewSeed: The new random seed value
- Time: Simulation time when the reset occurred
This is particularly useful when debugging tests that use --reseed-time
option, as it provides visibility into when and how the random seed was
changed during the test run.
Example output:
<ResettingRandomSeed NewSeed="67890" Time="100.5"/>
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* Use non-deterministic random for reseed time
To truly randomize the seed reset time.
* clang-format fix
* Address review comments
---------
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Delete encryption at rest in accordance with plans circulated in PR #12400.
Development of this PR was mostly straightforward. Several issues of note:
Upgrade tests which use Redwood want to use the old "encrypt by 0xFF XOR" test-only encoding/encryption algorithm. I wanted to delete that in main and did so. Upgrade tests have been modified not to use Redwood (storage engine 3, mentioned in various storageEngineExcludeTypes test options updates). 7.1 did not define storageEngineExcludeTypes. After some discussion we have decided to delete 7.1-based upgrade tests. 7.3- and 7.4-based upgrade tests remain in place.
Cross-version compatibility (such as it is, I assume in client library startup) remains super easy to break when deleting code. Tips: (A) removing unnecessary arguments to functions is bread and butter code editing when removing code, but if you find yourself removing arguments from a serializer() invocation, you might be breaking a protocol. (B) run ctest -R upgrade early and often.
An implication of these changes is that we are going to rely on the community to make us aware of any {upgrade+Redwood}-specific bugs. In other words, we are declining to continue to go out of our way to test functionality that we have no plans to use.
On the plus side this reclaims 15,000 more lines of code that we don't have to look at or think about, including in common areas such as commit proxy, backups, and generic storage server code.
Testing:
20260129-230241-gglass-15694f5f80af6932 compressed=True data_size=34905446 duration=4335357 ended=100000 fail_fast=1000 max_runs=100000 pass=100000 priority=100 remaining=0 runtime=6:21:47 sanity=False started=100000 stopped=20260130-052428 submitted=20260129-230241 timeout=5400 username=gglass
20260202-214159-gglass-69b90c779cf8ec68 compressed=True data_size=35007141 duration=4612639 ended=100000 fail_fast=1000 max_runs=100000 pass=100000 priority=100 remaining=0 runtime=3:00:25 sanity=False started=100000 stopped=20260203-004224 submitted=20260202-214159 timeout=5400 username=gglass
* Checkpoint file removals and code edits for removing encryption at rest. Have not tried to compile this yet.
* Checkpoint some improvements. Still does not compile.
* Merging with upstream changes in copyright updates resurrected previously deleted files, so delete them again
* Checkpoint incremental progress towards getting this to compile
* Checkpoint more intermediate changes getting encryption at rest deleted. Still doesnt compile but getting closer.
* Changes sufficient to get things to compile with removal of encryption at rest. NOT TESTED.
* Delete encryptModes from toml files run by current fdbserver binaries; restarting tests using <= 7.4 binaries do need encryptModes to say disabled
* Avoid using MAX_ENCODING_VALUE for random purposes for which other solutions are more clear and generally better
* Stop using XOREncryption_TestOnly because that no longer works. Also I see no need to support it as it requires unneeded interfaces and test fixtures in order to actually work.
* Remove more page encryption stuff, and address some TODO(gglass) comments.
* debugging redwood failures
* Fix some misc simulation failures. Notably, disable storage engine type 3 (redwood) on upgrade tests, as it writes databases with encoding type 1 which is no longer supported
* Add a comment discussion deprecation options for fields in persistent metadata, and explain why we are merely renaming the member as deprecated and nothing anything else.
* Remove 7.1 upgrade tests. These tests enable Redwood and write databases with "0xFF XOR encryption" style encoding (encoding 1), which is removed in main.
* Improve comments
* Obligatory f3f commit. Format The Effin Source Files
* Delete more unneeded encryption stuff
* Put back about 1% of deleted code in a desperate attempt to unbreak broken protocol compatibility
* formatting
* Remove mentions of encryption at rest in backup related APIs
* Address misc review comments. Remove --encrypt-files backup option.
* Joshua: Capture test config for crashed tests
Implements multiple fallback layers to ensure crashed tests always output
XML with reproduction info instead of empty strings.
Changes:
- fdbserver: Log TestConfiguring event before simulation starts
- TestHarness2/run.py: Parse traces + handle UnicodeDecodeError
- TestHarness2/app.py: Finally block ensures XML output
- Joshua scripts: Generate fallback XML if Python crashes
Fixes crashes from binary data in output and ensures test config is
always available for local reproduction of Joshua failures.
* Suppress emission of binary characters (This looks to be an old issue surfaced by changes in previous commit)
* Address review comments and allow that older binaries without this
fix could emit binary characters -- warn rather than fail in this
case.
* Add MockS3 persistence
Add comprehensive persistence to MockS3Server for crash recovery and post-test
analysis.
Key Changes:
- MockS3 persistence: Objects/multipart uploads persist to simfdb/mocks3/:
simfdb/mocks3/objects/<bucket>/<object>.{data,meta.json}
simfdb/mocks3/multipart/<uploadId>.{state.json,part.N}
- Crash resilience: State restored on restart, survives process kills
- Virtual function guards: Prevent pure virtual crashes in MockS3RequestHandler
- JSON with rapidjson: Replace manual string building with Document API
- Actor workaround: Document 'state dummy' pattern for early returns
- Simulator: Allow mocks3 directory in simfdb validation
- Test scripts: Centralize PRESERVE_TEST_DATA cleanup logic
- Atomic writes prevent corruption, sorted loading ensures determinism
* Simplify createParentDirectories to use platform API
Replaced manual loop with single call to platform::createDirectory(),
which already handles recursive creation and EEXIST errors properly.
This is cleaner, more robust, and leverages well-tested platform code.
* Remove arbitrary 100MB file size limit from readFileContent
Removed hardcoded 100MB limit for MockS3 metadata file reads.
Metadata should be tiny (<1KB), and if it's unreasonably large,
that's a bug we should surface, not silently ignore with an
arbitrary threshold.
* Simplify deletePersistedFile by removing redundant fileExists check
Removed fileExists() check from deletePersistedFile wrapper since
IAsyncFileSystem::deleteFile() already handles missing files gracefully.
Kept the wrapper for centralized trace events and error handling across
10+ call sites in MockS3 persistence code.
* Replace magic numbers with named constants for file suffixes
Defined constants for MockS3 persistence file extensions:
- OBJECT_DATA_SUFFIX = ".data" (length 5)
- OBJECT_META_SUFFIX = ".meta.json" (length 10)
- MULTIPART_STATE_SUFFIX = ".state.json" (length 11)
Replaced all hardcoded length checks (10, 11) and string literals
throughout the file. This makes the code more maintainable and
self-documenting.
* Fix 'magic numbers'.. use defines
* Remove unused OBJECT_DATA_SUFFIX_LEN constant
Removed OBJECT_DATA_SUFFIX_LEN which was triggering -Werror in CI/CD.
This constant was never used in length checks (we only concatenate
the suffix, never measure it).
* Fix MockS3 blocking during multipart cleanup
Added periodic yielding (every 10 parts) in deletePersistedMultipart
to prevent blocking other MockS3 requests during large multipart upload
cleanup. Without this, deleting 110+ parts would take 4-5 seconds of
continuous file operations, causing S3 client requests to timeout with
operation_cancelled errors.
Fixes BulkDumpingS3 test failures where bucket existence checks would
get cancelled while MockS3 was busy deleting multipart files.
* Increase yield frequency in multipart cleanup (every 5 parts with 10ms delay)
Changed from yielding every 10 parts with delay(0) to every 5 parts
with delay(0.01). The previous fix wasn't aggressive enough - 110 parts
were still taking 10+ seconds to delete, causing HTTP request timeouts.
With this change:
- Yield twice as often (every 5 parts instead of 10)
- Add 10ms actual delay (not just event loop yield)
- Gives HTTP requests real time to be processed
Fixes persistent BulkDumpingS3 test failures.
* Increase multipart cleanup yield frequency (every 2 parts, 20ms delay)
Changed from yielding every 5 parts with 10ms delay to every 2 parts
with 20ms delay. The previous fix still allowed deletions to take 11+
seconds for 110 parts, causing HTTP timeouts at 6.5 seconds.
New behavior for 110-part uploads:
- Yields ~55 times (vs 22 previously)
- Total delay time: ~1.1s (vs ~220ms)
- Much more breathing room for concurrent HTTP requests
Fixes BulkDumpingS3 test failures with seed 1268929493.
* Refactor: Extract common cleanup_with_preserve_check function
Consolidates repeated test data preservation logic from three test scripts
into a single function in tests_common.sh.
Before: Each test had duplicate code checking PRESERVE_TEST_DATA and
conditionally shutting down servers.
After: All tests call cleanup_with_preserve_check() which handles both
the preservation check and server shutdown.
Changes:
- Added cleanup_with_preserve_check() to tests_common.sh
- Updated s3_backup_test.sh to use common function
- Updated s3client_test.sh to use common function
- Updated bulkload_test.sh to use common function
No functional change, just reduces duplication.
* Improve ACTOR state variable initialization comment
Expanded the comment explaining why state variables must be declared
before early returns in ACTOR functions. The previous comment was too
brief and didn't explain the underlying mechanism.
New comment explains:
- How the actor compiler creates internal Promise<T> objects
- What 'sendable' means (canBeSet() must return true)
- Why Promise initialization depends on state variable declarations
- What canBeSet() assertion failure you get if violated
- Clear correct vs wrong pattern examples
This addresses reviewer feedback asking what 'properly initialize
the actor's Promise object' means.
* Clarify ACTOR state variable initialization patterns
Expanded comment to show three valid patterns for state variable
initialization, addressing reviewer feedback about declaration vs
initialization.
Key additions:
- Pattern 2 shows declaring + initializing together (simpler)
- Pattern 1 shows declaring early, initializing later (conditional)
- Pattern 3 shows dummy variable when real vars come later
- Added 'CHOOSING A PATTERN' section with guidance
- Added NOTE about default constructor requirement
Clarifies that the reviewer's suggestion (declaring + initializing
together) is valid and often simpler. The declare-then-assign pattern
is mainly useful for conditional initialization or when constructor
might throw.
* Shorten ACTOR state variable initialization comment
Reduced from 66 lines to 27 lines while keeping essential information:
- What the issue is (canBeSet() crash)
- Why it happens (Promise init code generation)
- How to fix it (declare state var before early return)
- Correct vs wrong example
Removed verbose background explanations and multiple pattern variations.
* Clarify that state variable declaration alone is sufficient
Updated comment to emphasize the reviewer's point that just declaring
the state variable (without initialization) triggers Promise init.
Changes:
- Added 'Declaration alone is sufficient - initialization can happen later'
- Changed example to show declaration at top, initialization later
- Added NOTE for types without default constructors
This addresses jzhou77's feedback that declaring and initializing later
has the same effect as declaring+initializing together.
* Clarify ACTOR state variable comment with technical details
Rewrote comment to directly address reviewer feedback:
1. Explains what 'properly initialize Promise' means: The actor compiler
generates a member initialization list (': member(value)') which ensures
the Actor<T> base class and its Promise are initialized before any code runs.
2. Clarifies why TIMING matters: Compiler must see state variable before early
return to generate the init list. Declaring it after is 'too late - compiler
didn't see it early enough'.
3. Removed the NOTE about default constructors (less critical).
Based on proof from generated code showing state variables trigger member
initialization list generation.
* Add note about default constructor requirement for state variables
Addresses jzhou77's feedback: declaring 'state SomeType variable;' requires
a default constructor. If the type doesn't have one, you must initialize at
declaration: 'state MyType x(params);'
* With fault injection, a bucket exists can be cancelled. Allow. Don't log severity=40 for something that is going to happen
---------
Co-authored-by: stack <stack@duboce.com>
* Use MockS3Server instead of seaweedfs for ctest.
Avoid download of seaweedfs and the 25second startup.
Preserve the try-another-port if expected is occupied.
Preserve use of s3 if available.
* Call describeBackup w/ invalidVersion so metadata is written when blobstore url
Trevor Clinkenbeard