- Use nondeterministicRandom() in MockS3Server to avoid diverging simulation state
- Add faultInjection=false to BulkLoad tests to prevent timeout failures
- Add extraMachineCountDC=3 for non-overlapping teams in HA configs --
makes it so can allow HA on these tests (Had been off up to this).
Add two new simulation tests for BulkLoad restore integration:
1. BackupS3BlobBulkLoadRestoreWithChaos.toml - Tests BulkDump/BulkLoad under
aggressive S3 fault injection (15% error rate, 20% throttling, 10% delays,
0.5% corruption) to validate resilience and retry mechanisms.
2. BackupS3BlobBulkLoadRestoreMultiRange.toml - Tests BulkDump/BulkLoad with
multiple non-contiguous backup ranges (backupRangesCount=5) to validate
correct handling of multiple SST file sets and range boundaries.
Both tests use snapshotMode=1 (BULKDUMP) and useRangeFileRestore=false
(BulkLoad) to exercise the new SST-file-based backup/restore path.
Fix BulkDump/BulkLoad race conditions and MockS3 concurrent writes
- Fix race where failing BulkDump task incorrectly resets mode to 0
- Fix MockS3 concurrent write collision using unique temp filenames
- Add encryption metadata write verification
- Add singleRegion config option to prevent HA in tests
- Enable HA (multi-region) testing for BulkLoad tests
Add chaos support to BackupS3BlobCorrectness test
Implements chaos injection for the BackupS3BlobCorrectness workload following
the pattern from S3ClientWorkload and BulkDumping chaos implementations.
Features:
- New BackupS3BlobCorrectnessWithChaos.toml test with four variants:
Stable (no chaos), LightChaos, MediumChaos, HeavyChaos
- Extended BackupS3BlobCorrectness workload with chaos parameters:
enableChaos, errorRate, throttleRate, delayRate, corruptionRate, maxDelay
- Conditionally uses MockS3ServerChaos when chaos is enabled
Bug Fix:
- Remove TraceEvent calls from lazy persistence initialization paths
- TraceEvent can access GlobalConfig which may not be initialized yet
- Fixes crash: Bad pointer dereference in GlobalConfig::get() during early S3 access
This completes chaos coverage for all S3 operations in FoundationDB:
- S3ClientWorkload: Direct S3 client operations
- BulkDumping: Bulk data dump/load via S3
- BackupS3BlobCorrectness: Backup and restore via S3
Changes:
- fdbserver/workloads/BackupS3BlobCorrectness.actor.cpp: chaos support
- tests/slow/BackupS3BlobCorrectnessWithChaos.toml: new test configuration
- tests/CMakeLists.txt: register new test
- fdbserver/MockS3Server.actor.cpp: remove early TraceEvent calls
- Add chaos injection support to BulkDumping workload
- Added MockS3ServerChaos include to BulkDumping.actor.cpp
- Added chaos configuration options (errorRate, throttleRate, delayRate, corruptionRate, maxDelay)
- Enhanced _setup() to support both regular and chaos MockS3 servers
- Added detection for already-registered servers to avoid conflicts
- Configure chaos rates per-test to allow progressive intensity testing
- Created BulkDumpingS3WithChaos.toml test suite with 4 test scenarios:
* Stable (0% chaos) - baseline
* Light chaos (5-10%) - minor disruptions
* Medium chaos (15-20%) - moderate disruptions
* Heavy chaos (30-40%) - severe disruptions
This mirrors the S3ClientWorkloadWithChaos design and allows testing
BulkDumping resilience to S3 failures in simulation.
The validateBulkLoadJobHistory function incorrectly asserted that every
job in history had the same jobId, when it should have been searching
for a specific job. This caused crashes when multiple jobs existed in
history from cancellations/retries during chaos testing.
* Fix bulk load hangs with empty ranges and improve chaos test resilience
Empty Range Handling Fixes:
1. BulkLoadUtil.actor.cpp (bulkLoadDownloadTaskFileSets):
- Fixed root cause where empty ranges were skipped entirely with 'continue',
never added to localFileSets, causing FetchKeys to hang forever
- Now creates empty BulkLoadFileSet entries for empty ranges to track them
2. BulkLoadUtil.actor.cpp (bulkLoadTransportBlobstore_impl):
- Added hasDataFile() check before attempting S3 downloads
- Skip data file download gracefully for empty ranges
3. storageserver.actor.cpp (tryGetRangeForBulkLoad):
- Added hasDataFile() check to prevent calling getDataFileFullPath() on empty fileSets
- Added empty result signaling: when all ranges are empty, send empty RangeResult
with end_of_stream() to properly complete FetchKeys operations
4. storageserver.actor.cpp (bulk load SST ingestion):
- Added hasDataFile() checks before accessing data file paths during SST processing
5. storageserver.actor.cpp (restoreDurableState):
- Improved bulk load recovery robustness by using intersectingRanges() instead
of exact range match during storage server recovery
- Handle range boundary mismatches that can occur due to splits/chaos
- Added comprehensive error logging for debugging recovery issues
Performance and Retry Improvements:
1. Added BULKLOAD_DOWNLOAD_RETRY_DELAY knob (2.0s for both simulation and production)
2. Added BULKLOAD_DOWNLOAD_MAX_RETRIES knob (20 for both simulation and production)
- Total retry window: 20 retries × 2s = 40 seconds maximum
3. Reduced BLOBSTORE_MAX_DELAY_RETRYABLE_ERROR from 60s to 20s in production
4. Added retry limits to prevent infinite retry loops in bulk load downloads
* Implement MockS3ServerChaos: S3 Error Injection for Testing
Add chaos injection system for MockS3Server following AsyncFileChaos pattern.
Tests S3BlobStore client resilience against realistic S3 failures.
- MockS3ServerChaos wrapper with configurable fault injection
- S3FaultInjector with error/throttle/delay/corruption rates
- Chaos support integrated into S3ClientWorkload
- Simplified URL parsing and improved error handling
- Multipart upload idempotency and BUGGIFY fix
- ChaosMetrics tracking for S3 events
- Comprehensive test suite with multiple chaos levels
Design doc: design/mocks3server_chaos_design.md
Tests: tests/slow/S3ClientWorkloadWithChaos.toml
Currently adds light/medium/heavy chaos to the simple s3client
test. Will follow-on with similar for bulkload via s3 and for
backup via s3.
* Mostly around registration of http server across test runs inside a workflow
* Formatting
* Missing state qualifier
* * fdbclient/S3BlobStore.actor.cpp
Fix bug where we are accumulating headers across retries.
* fdbserver/MockS3Server.actor.cpp
* fdbserver/MockS3ServerChaos.actor.cpp
* fdbserver/include/fdbserver/MockS3Server.h
Add check if we should persist mocks3 data.
* tests/slow/S3ClientWorkloadWithChaos.toml
Use one mocks3, the one that does chaos for all loads.
* Reduce expected duration wait
* Ameliorate test timing out by turning down the aggressiveness and
having connection failures run for the start of the test rather
than all through it; the backup/restore was struggling to complete
and could timeout before doing so.
Also address case where we were not being s3-like.
* fdbserver/MockS3Server.actor.cpp
Backup can have two different clients uploading the same file. This is
legit in s3; last file wins. Accommodate (file system was logging a
SevError when second process went looking for .part file removed by
first).
* fdbserver/tester.actor.cpp
Allow setting how long connection failures run for.
* fdbserver/workloads/BackupS3BlobCorrectness.actor.cpp
Return void rather than hang around -- was covering actual
issues.
* tests/fast/BackupS3BlobCorrectness.toml
Have the test run less virulently.
* Be more radical; turn off the chaos-making. We will be adding it back in a subsequent, controlled PR
* 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>
* Fix MockS3Server canBeSet() assertion failure. Seen on mac.
- Fix UnsentPacketQueue pointer overwrite issue in MockS3Server; was
overwriting existing response->data.content pointer.
- Replace 'new UnsentPacketQueue()' with 'discardAll()' to use existing content queue
- Remove conditional reference counting that could cause memory corruption
- Simplify clone() method to always return new instance
- Add null check on globals and aggressive clearing of currentProcess.
* ASSERT global id >= 0.
Comment on when currentProcess is nullptr
(Address review feedback)
---------
Co-authored-by: michael 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
* Add HTTP range header support to support partial content requests.
Made storage global rather than instance-based.
Made safe against process context switching mid-insert.
Enabled tests and made them test s3-ness (bucket in path, etc.)
Added range tests.
* Add a MockS3Server. Register it at 127.0.0.1:8080 on the simulated
network (simulated network intercepts requests for 127.0.0.1:8080
and does appropriate forwarding to s3 handler). Add two workloads,
one for s3client against 's3' and another that swaps in s3 as
backend for the BulkDumping test.
The Mock S3 Server is mostly generated (claude-sonnet-4) code. It
supports:
- Basic GET/PUT/DELETE/HEAD object operations
- Multipart uploads (initiate, upload parts, complete, abort)
- Object tagging (put/get tags)
- In-memory storage with deterministic behavior
- S3-compatible XML responses
* fdbserver/workloads/BulkDumping.actor.cpp
Change this workload so it reads the transport to use
from configuration file so can be used to test
file-based and s3 bulk loading. If the transport is
blobstore/s3, start up the mocks3server in _setup.
* fdbserver/workloads/S3ClientWorkload.actor.cpp
Simple workload to exercise s3client. Starts up the
mocks3server in _setup.
* fdbclient/S3BlobStore.actor.cpp
Check BUGGIFY before messing w/ status codes.
* fdbrpc/HTTP.actor.cpp
Add defines and log if failed parse of status line.
* Disable global buggify if the toml file turns off buggify
* Remove disable of fault injection if BUGGIFY set
* Remove an irrelevant configuration
michael stack