kvcache-ai/Mooncake

Fork 0

mirror of https://github.com/kvcache-ai/Mooncake.git synced 2026-11-05 01:17:55 +00:00

[GH-ISSUE #3158] [RFC]: Decouple Mooncake Store business logic from metrics #813

New issue

Open

opened 2026-10-29 21:19:48 +00:00 by bsakaguchi · 4 comments

bsakaguchi commented

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

Owner

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

Summary

Mooncake Store currently uses observability gauges as inputs to several business decisions. This reverses the intended dependency direction:

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

Make DRAM eviction, NoF eviction, promotion admission, and SSD-aware placement depend only on internal domain state.
Establish one authoritative owner for used bytes and capacity per storage tier.
Preserve existing eviction/promotion/placement policy and thresholds.
Preserve Prometheus metric names, labels, and externally visible semantics where they are already correct.
Make metrics optional for correctness: a no-op, reset, delayed, or unavailable metrics sink must not affect behavior.
Make the dependency direction enforceable through APIs, not only comments.

Non-goals

Do not redesign eviction algorithms or promotion policy.
Do not change user-facing configuration or watermark values.
Do not redesign cumulative request/error counters.
Do not make distributed SSD capacity perfectly real-time; client-reported capacity remains the state source for that feature.
Do not require a broad metrics-library replacement.

Originally created by @Aionw on GitHub (Jul 28, 2026). Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/3158 ## 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.

bsakaguchi added the

RFC

label

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

bsakaguchi commented

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

Author

Owner

@github-actions[bot] commented on GitHub (Jul 28, 2026):

Thanks for opening this issue, @Aionw!

Field	Value
Issue	#3158
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 (Jul 28, 2026): Thanks for opening this issue, @Aionw! | Field | Value | |-------|-------| | **Issue** | #3158 | | **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:19:49 +00:00

Author

Owner

@Hubert-Zhu commented on GitHub (Aug 2, 2026):

Hi @Aionw.

I am new to the this project and this RFC sounds very reasonable and have a well-definited scope. I am wondering if I can pick up this as my first onboarding task.

I did some POC with vibe-coded some draft PRs. But, before offically implementation, I have some qustions.

About design decision:
I am trying to understand the recovery flow. For a domain-owned O(1) usage state in SegmentManager, do we need to add some HA/snapshot restore change to rebuild that state? Otherwise, I am not sure if it may get into the bookkeeping problem.

About workflow/ how to contribute:
This is clear a mid-scoped change. Do we usually do 1 big PR or stack of small PRs?

@Hubert-Zhu commented on GitHub (Aug 2, 2026): Hi @Aionw. I am new to the this project and this RFC sounds very reasonable and have a well-definited scope. I am wondering if I can pick up this as my first onboarding task. I did some POC with vibe-coded some draft PRs. But, before offically implementation, I have some qustions. About design decision: I am trying to understand the recovery flow. For a domain-owned O(1) usage state in SegmentManager, do we need to add some HA/snapshot restore change to rebuild that state? Otherwise, I am not sure if it may get into the bookkeeping problem. About workflow/ how to contribute: This is clear a mid-scoped change. Do we usually do 1 big PR or stack of small PRs?

bsakaguchi commented

2026-10-29 21:19:49 +00:00

Author

Owner

@Hubert-Zhu commented on GitHub (Aug 3, 2026):

Synced with @Aionw through Slack. I will set the target date to Aug/10 for this task.

@Hubert-Zhu commented on GitHub (Aug 3, 2026): Synced with @Aionw through Slack. I will set the target date to Aug/10 for this task.

bsakaguchi commented

2026-10-29 21:19:49 +00:00

Author

Owner

@Hubert-Zhu commented on GitHub (Aug 10, 2026):

https://github.com/Hubert-Zhu/Mooncake/pull/4. still need a bit more time to bake.

@Hubert-Zhu commented on GitHub (Aug 10, 2026): https://github.com/Hubert-Zhu/Mooncake/pull/4. still need a bit more time to bake.