kvcache-ai / kvcache-ai/Mooncake
[RFC]: Parallelize long-running Mooncake Store unit tests
- Dominant language
- C++
- Stars
- 6.6k
- Forks
- 1.2k
- Avg merge
- 3d 5h
- Merged PRs (30d)
- 312
Description
## Status
Draft / implementation in review.
This RFC replaces the oversized prototype PR #2896 with six smaller draft PRs that have an explicit review and merge topology.
## Motivation
Several Mooncake Store unit-test executables contain many semantically unrelated cases and run for a long time as one scheduling unit. Splitting those cases by behavior gives CTest more useful parallel work and makes failures easier to locate, but creating one executable per source file increases compile/link overhead.
The proposal therefore separates two concerns:
1. Keep source files organized by semantic behavior.
2. Use a small CMake helper to combine compatible files into a bounded number of Unity-built test targets.
No hard-coded shard index or test-name routing table is introduced.
## Invariants
- Preserve all 296 source-defined GTest cases.
- Changes to test-only fixture behavior are acceptable when needed for isolation.
- Production behavior must not change.
- Each domain split must remain independently reviewable.
- The final target-balancing change must be isolated in its own commit/PR layer.
## PR topology
```text
+--> #2900 DummyClient + offload --------+
+--> #2901 SSD + snapshot SSD -----------+
RFC #2898 / main --------+--> #2902 promotion --------------------+--> #2905 Unity/target balance
+--> #2903 MasterService core -----------+
+--> #2904 snapshot MasterService -------+
```
The five domain PRs are all based directly on the same `main` commit and can be reviewed in parallel. #2905 contains those five commits plus one final CMake-only commit; review that final commit after the domain PRs, then let its diff shrink as #2900–#2904 merge.
## Tracking
| Order | PR | Scope | Source tests | Review dependency |
| --- | --- | --- | ---: | --- |
| 1 | #2900 | DummyClient + offload | 17 | RFC only |
| 2 | #2901 | SSD + snapshot SSD | 26 | RFC only |
| 3 | #2902 | Promotion | 40 | RFC only |
| 4 | #2903 | MasterService core | 144 | RFC only |
| 5 | #2904 | Snapshot MasterService | 69 | RFC only |
| 6 | #2905 | Unity build and target balancing | 296 total | #2900–#2904 |
## CMake design
`add_store_test()` remains the single registration primitive. The final PR adds `add_store_unity_test()`, which delegates to it and enables CMake `UNITY_BUILD` for compatible semantic source groups.
The result is:
- 30 independently registered targets in the five domain PRs, useful for isolated review and validation.
- 21 balanced targets in the final integrated layout.
- Semantic directories and filenames remain intact.
- No numeric shard constants or hard-coded test-case mapping in CMake.
## Validation
Five independent branches:
- All targeted builds passed.
- 30/30 CTest targets passed.
- Source-defined test count: 17 + 26 + 40 + 144 + 69 = 296.
- Four-way group wall times: 32.69s, 43.32s, 36.63s, 85.97s, and 89.85s.
Final integration branch:
- 21/21 targets built on current `main`.
- 21/21 CTest targets passed with `ctest -j4` in 204.53s.
- Isolated prototype measurement: 148.05s target build and 203.81s test time.
- A contended cold build measured 490.52s after four concurrent worktree builds; it is retained as a host-load datapoint and is not used as a regression comparison.
## Acceptance criteria
- [x] 296 source-defined tests preserved.
- [x] Semantic source layout replaces the long monoliths.
- [x] Final test wall time is at most 210s at four-way concurrency.
- [x] Isolated changed-target build is at most 160s.
- [x] Combined isolated build + test time is at most 360s.
- [x] Domain PRs can be reviewed in parallel.
- [x] Target balancing is isolated for focused review.
## Rollout
1. Review #2900–#2904 in parallel.
2. Merge accepted domain PRs in any order, resolving only localized CMake registration overlap if needed.
3. Rebase #2905 as those PRs land.
4. Review the final CMake-only target-balancing commit.
5. Mark the drafts ready only after human review of every changed line and successful CI.
## Superseded prototype
#2896 demonstrated the full approach but is too large to review effectively. It is superseded by this RFC and #2900–#2905.
Contributor guide
Research direction
Start by reviewing the RFC's PR topology, especially #2900–#2905, and inspect the existing add_store_test() registration primitive. The final work centers on add_store_unity_test(), CMake UNITY_BUILD, and CTest validation. Done means preserving all 296 source-defined tests while reaching the stated 21-target and timing acceptance criteria.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cmake, cpp
- Domain
- build-system, testing-qa
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100