kvcache-ai / kvcache-ai/Mooncake
[RFC]: Decouple Mooncake Store business logic from metrics
- Dominant language
- C++
- Stars
- 6.6k
- Forks
- 1.2k
- Avg merge
- 3d 5h
- Merged PRs (30d)
- 312
Description
## Summary
Mooncake Store currently uses observability gauges as inputs to several business decisions. This reverses the intended dependency direction:
```text
current: business mutation -> metric bookkeeping -> metric read -> business decision
proposed: business mutation -> authoritative domain state -> business decision
\-> metrics projection
```
Metrics should be a lossy, replaceable projection of runtime state. Disabling, resetting, restoring, or changing the metrics implementation must not change eviction, promotion, or replica-placement behavior.
This RFC proposes to move all business reads to authoritative internal state, keep metrics write-only from business code, and derive storage gauges from domain snapshots.
## Motivation
Depending on metrics for correctness is counterintuitive and fragile:
- metric gauges duplicate state already owned by allocators and segment managers;
- every allocate/deallocate, mount/unmount, restore, CXL, and NoF path must update both copies correctly;
- the global ratio reads allocated and capacity gauges separately, so it is not a coherent snapshot;
- reset/rebuild logic can temporarily make a healthy, full cluster appear empty;
- a missed increment/decrement changes eviction and promotion behavior rather than only degrading observability;
- singleton metric state leaks across tests and encourages tests to use telemetry as a business-state oracle;
- future metrics backends, compile-time switches, sampling, or asynchronous export become correctness-sensitive.
The rule should be:
> Business logic may emit metrics, but must not read metrics. Metrics may read/project business state, but must never be the source of truth.
## Current coupling audit
The audit covered production code in Mooncake Store, Transfer Engine, TENT, PG, and EP. Tests, benchmarks, admin summaries, and RPCs whose explicit purpose is to expose metrics are classified separately.
### True control-flow dependencies
| Business behavior | Current input | Effect |
| --- | --- | --- |
| DRAM eviction trigger and eviction target | `MasterMetricManager::get_global_mem_used_ratio()` in `MasterService::EvictionThreadFunc` | Decides whether to evict and computes `BatchEvict` target/lower bound. Runs every 10 ms. |
| NoF eviction trigger and eviction target | `MasterMetricManager::get_global_nof_used_ratio()` in `MasterService::EvictionThreadFunc` | Decides whether to evict and computes `NoFBatchEvict` target/lower bound. |
| L2-to-L1 promotion admission | `MasterMetricManager::get_global_mem_used_ratio()` in `MasterService::TryPushPromotionQueue` | Rejects promotion at the high watermark and changes retry-candidate state. |
These three reads are the correctness coupling this RFC must remove.
The promotion dependency originated in RFC #2031 and landed in PR #2071. This RFC does not change promotion policy; it changes the source of the watermark signal.
### Metrics-named domain dependency
`SsdFreeRatioFirstAllocationStrategy` consumes `SsdMetricsProvider` to rank segments. The provider is implemented by `ScopedLocalDiskSegmentAccess` and reads `LocalDiskSegment::ssd_total_capacity_bytes` plus `ssd_used_bytes`.
This path affects replica placement, but it does **not** read Prometheus/ylt metric instruments. It already reads internal state; the abstraction is misleadingly named. It should be renamed to a domain concept such as `SsdUsageProvider` or `LocalDiskUsageView`, with explicit state invariants.
This path landed in PR #2450 and is complementary to the true metric-instrument coupling above.
### Observability-only reads
The following are valid and are not business dependencies:
- admin HTTP metric serialization and summaries;
- cache-statistics RPCs whose contract is explicitly to return statistics;
- metric unit tests and observability assertions;
- Transfer Engine/TENT/client paths that only record counters, histograms, or timing fields.
No production business decision outside Mooncake Store was found to read an exported metric value.
## Goals
1. Make DRAM eviction, NoF eviction, promotion admission, and SSD-aware placement depend only on internal domain state.
2. Establish one authoritative owner for used bytes and capacity per storage tier.
3. Preserve existing eviction/promotion/placement policy and thresholds.
4. Preserve Prometheus metric names, labels, and externally visible semantics where they are already correct.
5. Make metrics optional for correctness: a no-op, reset, delayed, or unavailable metrics sink must not affect behavior.
6. Make the dependency direction enforceable through APIs, not only comments.
## Non-goals
1. Do not redesign eviction algorithms or promotion policy.
2. Do not change user-facing configuration or watermark values.
3. Do not redesign cumulative request/error counters.
4. Do not make distributed SSD capacity perfectly real-time; client-reported capacity remains the state source for that feature.
5. Do not require a broad metrics-library replacement.
Contributor guide
Research direction
Start with MasterService::EvictionThreadFunc and TryPushPromotionQueue, tracing their reads of MasterMetricManager::get_global_mem_used_ratio() and get_global_nof_used_ratio(). Then inspect SsdFreeRatioFirstAllocationStrategy, SsdMetricsProvider, and ScopedLocalDiskSegmentAccess to separate domain state from metric naming. Done means eviction, promotion, and placement use authoritative state while metric names and externally visible semantics remain unchanged.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend, distributed-systems, observability-sre
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100