[GH-ISSUE #2457] [Store] Count LOCAL_DISK-only GetReplicaList hits in master SSD cache-hit metrics #653

New issue

Closed

opened 2026-10-29 21:17:41 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-29 21:17:41 +00:00

Owner

Originally created by @feichai0017 on GitHub (Jun 13, 2026).
Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/2457

Originally assigned to: @feichai0017 on GitHub.

Problem

Mooncake Store exposes master cache-hit stats such as memory_hits and ssd_hits via MasterMetricManager::calculate_cache_stats().

Currently MasterService::GetReplicaList() increments cache-hit metrics by checking only the first returned Replica::Descriptor:

is_memory_replica() -> inc_mem_cache_hit_nums()
is_disk_replica() -> inc_file_cache_hit_nums()

However SSD offload uses ReplicaType::LOCAL_DISK, represented by LocalDiskDescriptor, which is separate from DiskDescriptor. Replica::Descriptor::is_disk_replica() does not match LocalDiskDescriptor.

This means a LOCAL_DISK-only object can be returned successfully by GetReplicaList() while ssd_hits does not increase. In that case valid_get_nums can increase, but neither memory nor SSD hit counters reflect the successful LOCAL_DISK hit.

Code pointers

mooncake-store/src/master_service.cpp: MasterService::GetReplicaList() classifies hits using replica_list[0].is_memory_replica() / replica_list[0].is_disk_replica().
mooncake-store/include/replica.h: Replica::Descriptor has separate DiskDescriptor and LocalDiskDescriptor variants.
mooncake-store/tests/promotion_on_hit_test.cpp: existing tests create LOCAL_DISK-only metadata via NotifyOffloadSuccess and expect GetReplicaList() to succeed.

Proposed fix

Classify cache-hit metrics from the complete replica list instead of checking only replica_list[0]:

Count a memory hit if any complete MEMORY replica is available.
Otherwise count an SSD hit if any complete DISK or LOCAL_DISK replica is available.
Keep existing valid-get accounting unchanged.
Leave NoF SSD behavior unchanged unless a separate NoF hit metric is introduced later.

Test plan

Add a focused unit test in mooncake-store/tests/master_metrics_test.cpp:

Start MasterService with offload enabled.
Mount a local disk segment.
Create a LOCAL_DISK-only key via NotifyOffloadSuccess.
Call GetReplicaList(key, "default").
Assert CacheHitStat::SSD_HITS increases by one.

Originally created by @feichai0017 on GitHub (Jun 13, 2026). Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/2457 Originally assigned to: @feichai0017 on GitHub. ### Problem Mooncake Store exposes master cache-hit stats such as `memory_hits` and `ssd_hits` via `MasterMetricManager::calculate_cache_stats()`. Currently `MasterService::GetReplicaList()` increments cache-hit metrics by checking only the first returned `Replica::Descriptor`: - `is_memory_replica()` -> `inc_mem_cache_hit_nums()` - `is_disk_replica()` -> `inc_file_cache_hit_nums()` However SSD offload uses `ReplicaType::LOCAL_DISK`, represented by `LocalDiskDescriptor`, which is separate from `DiskDescriptor`. `Replica::Descriptor::is_disk_replica()` does not match `LocalDiskDescriptor`. This means a LOCAL_DISK-only object can be returned successfully by `GetReplicaList()` while `ssd_hits` does not increase. In that case `valid_get_nums` can increase, but neither memory nor SSD hit counters reflect the successful LOCAL_DISK hit. ### Code pointers - `mooncake-store/src/master_service.cpp`: `MasterService::GetReplicaList()` classifies hits using `replica_list[0].is_memory_replica()` / `replica_list[0].is_disk_replica()`. - `mooncake-store/include/replica.h`: `Replica::Descriptor` has separate `DiskDescriptor` and `LocalDiskDescriptor` variants. - `mooncake-store/tests/promotion_on_hit_test.cpp`: existing tests create LOCAL_DISK-only metadata via `NotifyOffloadSuccess` and expect `GetReplicaList()` to succeed. ### Proposed fix Classify cache-hit metrics from the complete replica list instead of checking only `replica_list[0]`: 1. Count a memory hit if any complete MEMORY replica is available. 2. Otherwise count an SSD hit if any complete DISK or LOCAL_DISK replica is available. 3. Keep existing valid-get accounting unchanged. 4. Leave NoF SSD behavior unchanged unless a separate NoF hit metric is introduced later. ### Test plan Add a focused unit test in `mooncake-store/tests/master_metrics_test.cpp`: 1. Start `MasterService` with offload enabled. 2. Mount a local disk segment. 3. Create a LOCAL_DISK-only key via `NotifyOffloadSuccess`. 4. Call `GetReplicaList(key, "default")`. 5. Assert `CacheHitStat::SSD_HITS` increases by one.

bsakaguchi closed this issue

2026-10-29 21:17:42 +00:00

bsakaguchi commented

2026-10-29 21:17:43 +00:00

Author

Owner

@github-actions[bot] commented on GitHub (Jun 13, 2026):

Thanks for opening this issue, @feichai0017!

Field	Value
Issue	#2457
GitHub user ID	`132118805`
Reporter	@feichai0017

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 (Jun 13, 2026): Thanks for opening this issue, @feichai0017! | Field | Value | |-------|-------| | **Issue** | #2457 | | **GitHub user ID** | `132118805` | | **Reporter** | @feichai0017 | 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 referenced this issue

2026-10-29 21:29:48 +00:00

[PR #655] [MERGED] fix(store) : fix disk-backed replicas in size calculation and slice allocation(#653) #1722