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 #1136] [Usage]: Question about `master_exist_key_failures_total` metric behavior #378

New issue

Closed

opened 2026-10-29 21:13:35 +00:00 by bsakaguchi · 8 comments

bsakaguchi commented

2026-10-29 21:13:35 +00:00

Owner

Originally created by @tianlang-wq on GitHub (Nov 28, 2025).
Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/1136

Describe your usage question

Summary

I need clarification on when the master_exist_key_failures_total metric is supposed to be triggered. After analyzing the code, I found that the current implementation rarely increments this counter.

Current Behavior Analysis

Code Analysis

Looking at the ExistKey implementation in mooncake-store/src/master_service.cpp:

auto MasterService::ExistKey(const std::string& key)
    -> tl::expected<bool, ErrorCode> {
    MetadataAccessor accessor(this, key);
    if (!accessor.Exists()) {
        VLOG(1) << "key=" << key << ", info=object_not_found";
        return false;  // Returns false for non-existent keys
    }

    auto& metadata = accessor.Get();
    for (const auto& replica : metadata.replicas) {
        if (replica.status() == ReplicaStatus::COMPLETE) {
            metadata.GrantLease(default_kv_lease_ttl_, default_kv_soft_pin_ttl_);
            return true;  // Returns true when complete replica found
        }
    }

    return false;  // Returns false when no complete replica found
}

Issue

The function only returns true/false (success cases) and never returns an error code like other operations do. The metric master_exist_key_failures_total is incremented only when the RPC call fails (returns an error), but ExistKey never returns errors in normal operation.

Current Metric Logic

In rpc_service.cpp:

tl::expected<bool, ErrorCode> WrappedMasterService::ExistKey(
    const std::string& key) {
    return execute_rpc(
        "ExistKey", [&] { return master_service_.ExistKey(key); },
        [&](auto& timer) { timer.LogRequest("key=", key); },
        [] { MasterMetricManager::instance().inc_exist_key_requests(); },
        [] { MasterMetricManager::instance().inc_exist_key_failures(); });
}

The failure metric is only incremented when !result.has_value() is true, but ExistKey always returns a value.

Questions

Design Intent: Is this the intended behavior? Should ExistKey only fail on system-level errors, not on key misses?
Metric Purpose: What is the intended purpose of master_exist_key_failures_total? Should it track:
- System-level failures only (current behavior)
- Key cache misses (when key doesn't exist)
- Both system failures and cache misses
Consistency: Other operations like GetReplicaList return errors like OBJECT_NOT_FOUND when keys don't exist, but ExistKey returns false. Is this inconsistency intentional?
Monitoring Use Case: How should users interpret this metric for monitoring? Currently it will almost always be 0, which might be confusing.

Context

This question arose while setting up monitoring for a Mooncake deployment. The master_exist_key_failures_total metric is always 0, even during periods of high cache miss rates, making it unclear how to properly monitor cache performance.

Environment

Mooncake version: Latest main branch
Deployment: Distributed KV store with memory and file storage backends
Monitoring: Prometheus metrics collection

Would appreciate clarification on the intended behavior and best practices for monitoring cache hit/miss rates.

Before submitting a new issue...

Make sure you already searched for relevant issues and read the documentation

Originally created by @tianlang-wq on GitHub (Nov 28, 2025). Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/1136 ### Describe your usage question ## Summary I need clarification on when the `master_exist_key_failures_total` metric is supposed to be triggered. After analyzing the code, I found that the current implementation rarely increments this counter. ## Current Behavior Analysis ### Code Analysis Looking at the `ExistKey` implementation in `mooncake-store/src/master_service.cpp`: ```cpp auto MasterService::ExistKey(const std::string& key) -> tl::expected<bool, ErrorCode> { MetadataAccessor accessor(this, key); if (!accessor.Exists()) { VLOG(1) << "key=" << key << ", info=object_not_found"; return false; // Returns false for non-existent keys } auto& metadata = accessor.Get(); for (const auto& replica : metadata.replicas) { if (replica.status() == ReplicaStatus::COMPLETE) { metadata.GrantLease(default_kv_lease_ttl_, default_kv_soft_pin_ttl_); return true; // Returns true when complete replica found } } return false; // Returns false when no complete replica found } ``` ### Issue The function only returns `true`/`false` (success cases) and never returns an error code like other operations do. The metric `master_exist_key_failures_total` is incremented only when the RPC call fails (returns an error), but `ExistKey` never returns errors in normal operation. ### Current Metric Logic In `rpc_service.cpp`: ```cpp tl::expected<bool, ErrorCode> WrappedMasterService::ExistKey( const std::string& key) { return execute_rpc( "ExistKey", [&] { return master_service_.ExistKey(key); }, [&](auto& timer) { timer.LogRequest("key=", key); }, [] { MasterMetricManager::instance().inc_exist_key_requests(); }, [] { MasterMetricManager::instance().inc_exist_key_failures(); }); } ``` The failure metric is only incremented when `!result.has_value()` is true, but `ExistKey` always returns a value. ## Questions 1. **Design Intent**: Is this the intended behavior? Should `ExistKey` only fail on system-level errors, not on key misses? 2. **Metric Purpose**: What is the intended purpose of `master_exist_key_failures_total`? Should it track: - System-level failures only (current behavior) - Key cache misses (when key doesn't exist) - Both system failures and cache misses 3. **Consistency**: Other operations like `GetReplicaList` return errors like `OBJECT_NOT_FOUND` when keys don't exist, but `ExistKey` returns `false`. Is this inconsistency intentional? 4. **Monitoring Use Case**: How should users interpret this metric for monitoring? Currently it will almost always be 0, which might be confusing. ## Context This question arose while setting up monitoring for a Mooncake deployment. The `master_exist_key_failures_total` metric is always 0, even during periods of high cache miss rates, making it unclear how to properly monitor cache performance. ## Environment - Mooncake version: Latest main branch - Deployment: Distributed KV store with memory and file storage backends - Monitoring: Prometheus metrics collection Would appreciate clarification on the intended behavior and best practices for monitoring cache hit/miss rates. ### Before submitting a new issue... - [ ] Make sure you already searched for relevant issues and read the [documentation](https://kvcache-ai.github.io/Mooncake/)

bsakaguchi

2026-10-29 21:13:35 +00:00

closed this issue
added the
enhancement
label

bsakaguchi commented

2026-10-29 21:13:36 +00:00

Author

Owner

@ykwd commented on GitHub (Nov 28, 2025):

This is a very good question.

There are two methods to compute the cache hit rate:

First:
cache hit rate = successful get request number / successful put request number.
For example, if I put 100 key–value pairs into the cache, and they are used 80 times in total, then the cache hit rate is 80%.

Second:
cache hit rate = successful get request number / total get request number.
For example, if I issue 100 get requests, and 80 of them successfully retrieve the item, then the cache hit rate is 80%.

The first method was introduced in this PR: https://github.com/kvcache-ai/Mooncake/pull/1020

However, after thinking about it, the first method is actually closer to a cache utilization rate. The second method is the real definition of cache hit rate.

The problem is that in our upper-layer application, the implementation may call exist before both get and put. As a result, it will only get objects where exist = true, and only put objects where exist = false. Because of this behavior, calculating the hit rate using the second method becomes somewhat difficult.

@ykwd commented on GitHub (Nov 28, 2025): This is a very good question. There are two methods to compute the cache hit rate: **First:** cache hit rate = successful get request number / successful put request number. For example, if I put 100 key–value pairs into the cache, and they are used 80 times in total, then the cache hit rate is 80%. **Second:** cache hit rate = successful get request number / total get request number. For example, if I issue 100 get requests, and 80 of them successfully retrieve the item, then the cache hit rate is 80%. The first method was introduced in this PR: [https://github.com/kvcache-ai/Mooncake/pull/1020](https://github.com/kvcache-ai/Mooncake/pull/1020) However, after thinking about it, the first method is actually closer to a *cache utilization rate*. The second method is the real definition of *cache hit rate*. The problem is that in our upper-layer application, the implementation may call `exist` before both `get` and `put`. As a result, it will only `get` objects where `exist = true`, and only `put` objects where `exist = false`. Because of this behavior, calculating the hit rate using the second method becomes somewhat difficult.

bsakaguchi commented

2026-10-29 21:13:36 +00:00

Author

Owner

@tianlang-wq commented on GitHub (Nov 28, 2025):

If master_exist_key_failures_total is used to determine cache hit rate, and now there's a better method #1020, will this metric be deprecated in the future?

@tianlang-wq commented on GitHub (Nov 28, 2025): If master_exist_key_failures_total is used to determine cache hit rate, and now there's a better method #1020, will this metric be deprecated in the future?

bsakaguchi commented

2026-10-29 21:13:36 +00:00

Author

Owner

@ykwd commented on GitHub (Nov 28, 2025):

I don't think master_exist_key_failures_total is used to determine cache hit rate

@ykwd commented on GitHub (Nov 28, 2025): I don't think master_exist_key_failures_total is used to determine cache hit rate

bsakaguchi commented

2026-10-29 21:13:36 +00:00

Author

Owner

@tianlang-wq commented on GitHub (Nov 28, 2025):

Thank you for your answer. So, what is the purpose of master_exist_key_failures_total? Based on current code analysis, it appears this metric consistently remains at 0.

@tianlang-wq commented on GitHub (Nov 28, 2025): Thank you for your answer. So, what is the purpose of master_exist_key_failures_total? Based on current code analysis, it appears this metric consistently remains at 0.

bsakaguchi commented

2026-10-29 21:13:36 +00:00

Author

Owner

@tianlang-wq commented on GitHub (Nov 28, 2025):

If it calls exist 10 times, with 8 hits and 2 misses, then the 2 misses should correspond to master_exist_key_failures_total based on the code. Then, 8/10 would indeed be the hit rate. Is my interpretation correct?

@tianlang-wq commented on GitHub (Nov 28, 2025): If it calls exist 10 times, with 8 hits and 2 misses, then the 2 misses should correspond to master_exist_key_failures_total based on the code. Then, 8/10 would indeed be the hit rate. Is my interpretation correct?

bsakaguchi commented

2026-10-29 21:13:37 +00:00

Author

Owner

@tianlang-wq commented on GitHub (Nov 28, 2025):

However, the issue I've discovered currently is that master_exist_key_failures_total remains consistently at 0, regardless of the state. This should be a bug in the logic, right?

@tianlang-wq commented on GitHub (Nov 28, 2025): However, the issue I've discovered currently is that master_exist_key_failures_total remains consistently at 0, regardless of the state. This should be a bug in the logic, right?

bsakaguchi commented

2026-10-29 21:13:37 +00:00

Author

Owner

@ykwd commented on GitHub (Nov 28, 2025):

Thank you for your answer. So, what is the purpose of master_exist_key_failures_total? Based on current code analysis, it appears this metric consistently remains at 0.

Yes. In the current implementation, the exist RPC will always succeed on the master’s side. Returning either true or false is expected behavior, so this metric doesn’t really provide meaningful information at the moment.

@ykwd commented on GitHub (Nov 28, 2025): > Thank you for your answer. So, what is the purpose of master_exist_key_failures_total? Based on current code analysis, it appears this metric consistently remains at 0. Yes. In the current implementation, the `exist` RPC will always succeed on the master’s side. Returning either `true` or `false` is expected behavior, so this metric doesn’t really provide meaningful information at the moment.

bsakaguchi commented

2026-10-29 21:13:37 +00:00

Author

Owner

@tianlang-wq commented on GitHub (Nov 28, 2025):

Thank you for your answer.

@tianlang-wq commented on GitHub (Nov 28, 2025): Thank you for your answer.