Fix MockDD startup with production-only actors#13409
Conversation
tclinkenbeard-oai
left a comment
There was a problem hiding this comment.
Generated by Codex.
What is it trying to do?
This makes MockDD skip DD startup actors that require a real Database context, groups those production-only actors behind one helper, and updates MockDDReadWrite.toml to explicitly disable unsupported shard-location metadata encoding while removing the retired storage_quota_enabled knob.
Is it correct?
Yes, by code inspection. DDMockTxnProcessor::context() is intentionally UNREACHABLE(), and the actors moved behind the non-MockDD branch either call it directly or enter audit code that uses it. The production path still adds the same actor set, while the MockDD path initializes its local bulk-load collection without starting actors outside its contract. The change is startup-only; it adds no serialized or persisted format and introduces no hot-path work.
I did not run builds or tests. At review time, clang-format is passing; the Windows Boost and FoundationDB PR Builder/cluster checks are still pending, with no failing checks visible.
Are there bugs?
I did not find any correctness bugs.
Are there omissions?
None that I think block this. The adjusted MockDD workload is a reasonable regression for the reported startup failure and makes the unsupported metadata-encoding setting deterministic.
Are there better ways of doing things?
Centralizing the boundary in addProductionOnlyDataDistributionActors() is clearer than retaining several scattered if (!isMocked) checks. I do not see a simpler alternative that preserves the production startup set as clearly.
Should this CL be LGTMd?
Yes, LGTM from code inspection. I inspected the two-file patch, MockDD context contract, skipped actors’ context usage, startup future lifetimes, and test knob behavior. I would still wait for the currently pending non-format CI before merge.
Result of foundationdb-pr-clang-ide on Linux RHEL 9
|
Result of foundationdb-pr-clang-arm on Linux RHEL 9
|
Result of foundationdb-pr-macos-m1 on macOS 14.x
|
Result of foundationdb-pr-clang on Linux RHEL 9
|
Result of foundationdb-pr on Linux RHEL 9
|
Result of foundationdb-pr-cluster-tests on Linux RHEL 9
|
Result of foundationdb-pr-macos on macOS 14.x
|
Result of foundationdb-pr-clang-ide on Linux RHEL 9
|
Result of foundationdb-pr-clang-ide on Linux RHEL 9
|
Result of foundationdb-pr-macos-m1 on macOS 14.x
|
Result of foundationdb-pr-clang-arm on Linux RHEL 9
|
Result of foundationdb-pr-clang-arm on Linux RHEL 9
|
Result of foundationdb-pr-macos on macOS 14.x
|
Result of foundationdb-pr-clang on Linux RHEL 9
|
Result of foundationdb-pr-macos-m1 on macOS 14.x
|
Result of foundationdb-pr-clang on Linux RHEL 9
|
Result of foundationdb-pr on Linux RHEL 9
|
Result of foundationdb-pr-cluster-tests on Linux RHEL 9
|
Result of foundationdb-pr on Linux RHEL 9
|
Result of foundationdb-pr-cluster-tests on Linux RHEL 9
|
Result of foundationdb-pr-macos on macOS 14.x
|
This PR fixes a
tests/fast/MockDDReadWrite.tomltest failure.Mock data distribution uses
DDMockTxnProcessor, whosecontext()method is intentionally unreachable becauseMockDDdoes not have a realDatabasecontext.dataDistribution()nevertheless started several production-only actors forMockDD:monitorBackupPartitionRequiredmonitorBulkLoadModeAndSpawnActorsbulkDumpCoreThese actors require a real database context. A
MockDDcorrectness run therefore aborted when backup-partition monitoring calledDDMockTxnProcessor::context(). After guarding that actor, deterministic replay exposed the same issue in dynamic bulk-load monitoring.MockDDReadWrite.tomlalso retained the removedstorage_quota_enabledknob and allowed shard-location metadata encoding to be randomly enabled, even though that path is unsupported byMockDD.