[GH-ISSUE #3574] [Store][MasterService Tests] Migrate storage-tier RPC scenarios #918

New issue

Closed

opened 2026-10-29 21:20:53 +00:00 by bsakaguchi · 2 comments

bsakaguchi commented

2026-10-29 21:20:53 +00:00

Owner

Originally created by @Aionw on GitHub (Aug 21, 2026).
Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/3574

Parent: #3098\nDepends on: #3573\n\nMigrate client-visible storage-tier behavior using the typed DSL.\n\nScope:\n- NoF and DFS scenarios;\n- SSD and local-disk mount and unmount behavior;\n- offload heartbeat and completion;\n- capacity reporting and disk-replica eviction;\n- keep backend-specific setup and invariants in integration tests.\n\nThis PR should consume the storage DSL extension established by the foundation PR.

Originally created by @Aionw on GitHub (Aug 21, 2026). Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/3574 Parent: #3098\nDepends on: #3573\n\nMigrate client-visible storage-tier behavior using the typed DSL.\n\nScope:\n- NoF and DFS scenarios;\n- SSD and local-disk mount and unmount behavior;\n- offload heartbeat and completion;\n- capacity reporting and disk-replica eviction;\n- keep backend-specific setup and invariants in integration tests.\n\nThis PR should consume the storage DSL extension established by the foundation PR.

bsakaguchi closed this issue

2026-10-29 21:20:53 +00:00

bsakaguchi commented

2026-10-29 21:20:54 +00:00

Author

Owner

@github-actions[bot] commented on GitHub (Aug 21, 2026):

Thanks for opening this issue, @Aionw!

Field	Value
Issue	#3574
GitHub user ID	`41376987`
Reporter	@Aionw

A maintainer will triage this when possible. To help us respond faster, please include:

Mooncake version or commit SHA
Environment (OS, CUDA/driver, RDMA stack if relevant)
Steps to reproduce and expected vs. actual behavior

Useful links: Documentation · Contributing guide

This message was posted automatically by the issue bot.

@github-actions[bot] commented on GitHub (Aug 21, 2026): Thanks for opening this issue, @Aionw! | Field | Value | |-------|-------| | **Issue** | #3574 | | **GitHub user ID** | `41376987` | | **Reporter** | @Aionw | A maintainer will triage this when possible. To help us respond faster, please include: - Mooncake version or commit SHA - Environment (OS, CUDA/driver, RDMA stack if relevant) - Steps to reproduce and expected vs. actual behavior Useful links: [Documentation](https://kvcache-ai.github.io/Mooncake/) · [Contributing guide](https://github.com/kvcache-ai/Mooncake/blob/main/CONTRIBUTING.md) > This message was posted automatically by the issue bot.

bsakaguchi commented

2026-10-29 21:20:54 +00:00

Author

Owner

@rishabhsinha17 commented on GitHub (Sep 2, 2026):

Claiming this one (campaign sibling of #3572 -> #3656 merged, #3573 -> #3702 open). The new scenario files will be independent of #3702's, so either can land first; the monolith edits need a trivial rebase whichever comes second.

Scope check against the landed DSL (#3510 plus Eventually() from #3748):

Already expressible, migrating with no new vocabulary:

DFS: DfsPutEndAllAndUpsertTopologyAreAtomic and the four DfsEvictionSplitsAcceptedAndRejectedCandidates cases (DfsReplicas, OfType(ALL), EvictDfs, EvictMemory, HasDfsReplicas; the DFS env stays in the test body since it is read at service construction).
SSD dual-replica (root_fs_dir): PutEndBothReplica, PutRevoke{Disk,Memory,Both}Replica, RemoveKey, plus the EvictDiskReplica_* cases once the action below exists.

Needs vocabulary the foundation deliberately deferred ("introduced together with its first migration consumer and contract coverage"). Proposed, all thin wrappers over exported RPCs, each with contract tests:

MountLocalDisk(actor) / .OffloadingDisabled() and UnmountLocalDisk(actor)
ReportSsdCapacity(actor, bytes)
OffloadHeartbeat(actor).ExpectTasks({...}) / .ExpectNoTasks() / .Disabled()
CompleteOffload({...}).By(actor).OnNode(node) (NotifyOffloadSuccess registering COMPLETE local-disk replicas sized from the objects' PutStart)
EvictDiskReplica(key).By(actor).OfType(...)

Consumers: OffloadObjectHeartbeat (monolith), the local-disk half of master_service_ssd_test.cpp (unmount idempotency, stops offload work, drops only its own replicas, re-adoption, refused NotifyOffloadSuccess after unmount, usage-tracking release on Remove and BatchReplicaClear, SSD-free-ratio placement after offload, EvictDiskReplica releasing usage), and the offload-queue admission cases in offload_on_evict_test.cpp that are pure RPC flows (PutEnd queueing per combo, upsert preempting or being rejected by a queued or in-flight offload).

Staying focused, with reasons in the PR: metric-probing SSD tests (calculate_cache_stats, file_cache_nums), the PushOffloadingQueue private hook, the perf comparison, PutStartExpires (timing twin of the monolith's PutStartExpiringTest), the eviction-pressure loops and ConcurrentMountLocalDiskSegment / LocalDiskUnmountInterleavingTest (#3575 territory), and the NoF soft-pin tests (private deadline probe).

One question, the USE_NOF put-end/revoke tests: NoFNode is in the DSL, but MountNoFSegment returns UNAVAILABLE_IN_CURRENT_MODE without USE_NOF, and neither nightly nor my local build sets it (SPDK). I can move them under the same #ifdef USE_NOF guard, but I would be shipping scenarios I cannot execute. My default is to leave them in the monolith and say so in the PR; tell me if you prefer the guarded migration.

If you would rather see the local-disk/offload vocabulary as its own foundation-style PR first, or kept out of the DSL entirely, say so and I will split.

@rishabhsinha17 commented on GitHub (Sep 2, 2026): Claiming this one (campaign sibling of #3572 -> #3656 merged, #3573 -> #3702 open). The new scenario files will be independent of #3702's, so either can land first; the monolith edits need a trivial rebase whichever comes second. Scope check against the landed DSL (#3510 plus `Eventually()` from #3748): Already expressible, migrating with no new vocabulary: - DFS: `DfsPutEndAllAndUpsertTopologyAreAtomic` and the four `DfsEvictionSplitsAcceptedAndRejectedCandidates` cases (`DfsReplicas`, `OfType(ALL)`, `EvictDfs`, `EvictMemory`, `HasDfsReplicas`; the DFS env stays in the test body since it is read at service construction). - SSD dual-replica (`root_fs_dir`): `PutEndBothReplica`, `PutRevoke{Disk,Memory,Both}Replica`, `RemoveKey`, plus the `EvictDiskReplica_*` cases once the action below exists. Needs vocabulary the foundation deliberately deferred ("introduced together with its first migration consumer and contract coverage"). Proposed, all thin wrappers over exported RPCs, each with contract tests: - `MountLocalDisk(actor)` / `.OffloadingDisabled()` and `UnmountLocalDisk(actor)` - `ReportSsdCapacity(actor, bytes)` - `OffloadHeartbeat(actor).ExpectTasks({...})` / `.ExpectNoTasks()` / `.Disabled()` - `CompleteOffload({...}).By(actor).OnNode(node)` (NotifyOffloadSuccess registering COMPLETE local-disk replicas sized from the objects' PutStart) - `EvictDiskReplica(key).By(actor).OfType(...)` Consumers: `OffloadObjectHeartbeat` (monolith), the local-disk half of `master_service_ssd_test.cpp` (unmount idempotency, stops offload work, drops only its own replicas, re-adoption, refused NotifyOffloadSuccess after unmount, usage-tracking release on Remove and BatchReplicaClear, SSD-free-ratio placement after offload, EvictDiskReplica releasing usage), and the offload-queue admission cases in `offload_on_evict_test.cpp` that are pure RPC flows (PutEnd queueing per combo, upsert preempting or being rejected by a queued or in-flight offload). Staying focused, with reasons in the PR: metric-probing SSD tests (`calculate_cache_stats`, `file_cache_nums`), the `PushOffloadingQueue` private hook, the perf comparison, `PutStartExpires` (timing twin of the monolith's `PutStartExpiringTest`), the eviction-pressure loops and `ConcurrentMountLocalDiskSegment` / `LocalDiskUnmountInterleavingTest` (#3575 territory), and the NoF soft-pin tests (private deadline probe). One question, the `USE_NOF` put-end/revoke tests: `NoFNode` is in the DSL, but `MountNoFSegment` returns `UNAVAILABLE_IN_CURRENT_MODE` without `USE_NOF`, and neither nightly nor my local build sets it (SPDK). I can move them under the same `#ifdef USE_NOF` guard, but I would be shipping scenarios I cannot execute. My default is to leave them in the monolith and say so in the PR; tell me if you prefer the guarded migration. If you would rather see the local-disk/offload vocabulary as its own foundation-style PR first, or kept out of the DSL entirely, say so and I will split.