feat: expand Grafana dashboard with comprehensive Master metrics panels by liangxu2000 · Pull Request #2944 · kvcache-ai/Mooncake

liangxu2000 · 2026-07-16T02:46:40Z

Description

Replace the minimal dashboard (1 panel, non-working metric name) with a
comprehensive 40-panel Grafana dashboard. All PromQL queries now reference
actual metrics exposed by MasterMetricManager.

Module

Type of Change

How Has This Been Tested?

Deployed on a running master with metrics port 9009. Verified Prometheus
scrapes /metrics correctly, all 40 panels render in Grafana with valid
data. Manually cross-checked metric names against master_metric_manager.cpp.

Test commands:

# Start master with metrics
./mooncake_master --metrics_port=9009 --enable_metric_reporting=true

# Start monitoring stack
cd monitoring && bash run.sh

# Open Grafana at http://localhost:3000 (admin/admin)

Test results:

Unit tests pass
Integration tests pass (if applicable)
Manual testing done (describe below)

Manual testing: launched master + Prometheus + Grafana, confirmed all panels
render. No behavioral code changes, only dashboard JSON.

Checklist

I have performed a self-review of my own code
I have formatted my code using ./scripts/code_format.sh
I have run pre-commit run --all-files and all hooks pass
I have updated the documentation (if applicable)
I have added tests to prove my changes are effective
For changes >500 LOC: I have filed an RFC issue

AI Assistance Disclosure

No AI tools were used
AI tools were used (specify below)

AI (CodeBuddy) assisted with expanding dashboard panels and verifying PromQL
queries against master_metric_manager.cpp. Human reviewed every metric name
and validated all 40 panels against a running master instance.

gemini-code-assist

Code Review

This pull request significantly expands the Mooncake Master Grafana dashboard, replacing a single RPC requests graph with a comprehensive set of panels covering cluster overview, core operations QPS, segment lifecycles, evictions, cache hits, heartbeat health, and snapshot operations. Feedback on these changes highlights several incorrect PromQL queries where counter metrics are missing their required _total suffixes and other queries incorrectly retain trailing underscores from C++ private member variable names instead of using the actual registered Prometheus metric names.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-07-16T02:48:34Z

+        { "expr": "rate(master_attempted_evictions_mem[5m])", "legendFormat": "MEM Attempts", "refId": "A" },
+        { "expr": "rate(master_successful_evictions_mem[5m])", "legendFormat": "MEM Success", "refId": "B" },
+        { "expr": "rate(master_attempted_evictions_nof[5m])", "legendFormat": "NoF Attempts", "refId": "C" },
+        { "expr": "rate(master_successful_evictions_nof[5m])", "legendFormat": "NoF Success", "refId": "D" }


These PromQL queries are missing the _total suffix. Since mem_eviction_attempts_, mem_eviction_success_, nof_eviction_attempts_, and nof_eviction_success_ are registered as counters in MasterMetricManager, Prometheus automatically appends the _total suffix to their exposed metric names.

Suggested change

{ "expr": "rate(master_attempted_evictions_mem[5m])", "legendFormat": "MEM Attempts", "refId": "A" },

{ "expr": "rate(master_successful_evictions_mem[5m])", "legendFormat": "MEM Success", "refId": "B" },

{ "expr": "rate(master_attempted_evictions_nof[5m])", "legendFormat": "NoF Attempts", "refId": "C" },

{ "expr": "rate(master_successful_evictions_nof[5m])", "legendFormat": "NoF Success", "refId": "D" }

{ "expr": "rate(master_attempted_evictions_mem_total[5m])", "legendFormat": "MEM Attempts", "refId": "A" },

{ "expr": "rate(master_successful_evictions_mem_total[5m])", "legendFormat": "MEM Success", "refId": "B" },

{ "expr": "rate(master_attempted_evictions_nof_total[5m])", "legendFormat": "NoF Attempts", "refId": "C" },

{ "expr": "rate(master_successful_evictions_nof_total[5m])", "legendFormat": "NoF Success", "refId": "D" }

gemini-code-assist · 2026-07-16T02:48:34Z

+        { "expr": "rate(master_evicted_key_count[5m])", "legendFormat": "Total Keys", "refId": "A" },
+        { "expr": "rate(master_evicted_key_count_mem[5m])", "legendFormat": "MEM Keys", "refId": "B" },
+        { "expr": "rate(master_evicted_key_count_nof[5m])", "legendFormat": "NoF Keys", "refId": "C" }


These eviction key count metrics are registered as counters in MasterMetricManager and are missing the _total suffix in their PromQL queries.

Suggested change

{ "expr": "rate(master_evicted_key_count[5m])", "legendFormat": "Total Keys", "refId": "A" },

{ "expr": "rate(master_evicted_key_count_mem[5m])", "legendFormat": "MEM Keys", "refId": "B" },

{ "expr": "rate(master_evicted_key_count_nof[5m])", "legendFormat": "NoF Keys", "refId": "C" }

{ "expr": "rate(master_evicted_key_count_total[5m])", "legendFormat": "Total Keys", "refId": "A" },

{ "expr": "rate(master_evicted_key_count_mem_total[5m])", "legendFormat": "MEM Keys", "refId": "B" },

{ "expr": "rate(master_evicted_key_count_nof_total[5m])", "legendFormat": "NoF Keys", "refId": "C" }

gemini-code-assist · 2026-07-16T02:48:34Z

+        { "expr": "rate(master_evicted_size_bytes[5m])", "legendFormat": "Total Bytes", "refId": "A" },
+        { "expr": "rate(master_evicted_size_bytes_mem[5m])", "legendFormat": "MEM Bytes", "refId": "B" },
+        { "expr": "rate(master_evicted_size_bytes_nof[5m])", "legendFormat": "NoF Bytes", "refId": "C" }


These eviction size metrics are registered as counters in MasterMetricManager and are missing the _total suffix in their PromQL queries.

Suggested change

{ "expr": "rate(master_evicted_size_bytes[5m])", "legendFormat": "Total Bytes", "refId": "A" },

{ "expr": "rate(master_evicted_size_bytes_mem[5m])", "legendFormat": "MEM Bytes", "refId": "B" },

{ "expr": "rate(master_evicted_size_bytes_nof[5m])", "legendFormat": "NoF Bytes", "refId": "C" }

{ "expr": "rate(master_evicted_size_bytes_total[5m])", "legendFormat": "Total Bytes", "refId": "A" },

{ "expr": "rate(master_evicted_size_bytes_mem_total[5m])", "legendFormat": "MEM Bytes", "refId": "B" },

{ "expr": "rate(master_evicted_size_bytes_nof_total[5m])", "legendFormat": "NoF Bytes", "refId": "C" }

gemini-code-assist · 2026-07-16T02:48:34Z

+        { "expr": "rate(mem_cache_hit_nums_[5m])", "legendFormat": "MEM Hits", "refId": "A" },
+        { "expr": "rate(file_cache_hit_nums_[5m])", "legendFormat": "SSD Hits", "refId": "B" }


These PromQL queries contain trailing underscores and are missing the _total suffix. They correspond to the C++ private member variables mem_cache_hit_nums_ and file_cache_hit_nums_ in MasterMetricManager. Since they are registered as counters, their actual Prometheus metric names are mem_cache_hit_nums_total and file_cache_hit_nums_total.

Suggested change

{ "expr": "rate(mem_cache_hit_nums_[5m])", "legendFormat": "MEM Hits", "refId": "A" },

{ "expr": "rate(file_cache_hit_nums_[5m])", "legendFormat": "SSD Hits", "refId": "B" }

{ "expr": "rate(mem_cache_hit_nums_total[5m])", "legendFormat": "MEM Hits", "refId": "A" },

{ "expr": "rate(file_cache_hit_nums_total[5m])", "legendFormat": "SSD Hits", "refId": "B" }

gemini-code-assist · 2026-07-16T02:48:34Z

+        { "expr": "mem_cache_nums_", "legendFormat": "MEM Objects", "refId": "A" },
+        { "expr": "file_cache_nums_", "legendFormat": "SSD Objects", "refId": "B" }


These PromQL queries contain trailing underscores that match the C++ private member variables mem_cache_nums_ and file_cache_nums_. Since these are registered as gauges, their actual Prometheus metric names do not have trailing underscores.

Suggested change

{ "expr": "mem_cache_nums_", "legendFormat": "MEM Objects", "refId": "A" },

{ "expr": "file_cache_nums_", "legendFormat": "SSD Objects", "refId": "B" }

{ "expr": "mem_cache_nums", "legendFormat": "MEM Objects", "refId": "A" },

{ "expr": "file_cache_nums", "legendFormat": "SSD Objects", "refId": "B" }

gemini-code-assist · 2026-07-16T02:48:34Z

+      "gridPos": { "h": 4, "w": 6, "x": 6, "y": 91 },
+      "id": 705,
+      "options": { "colorMode": "background", "graphMode": "none", "justifyMode": "auto", "orientation": "auto", "reduceOptions": { "calcs": ["lastNotNull"], "fields": "", "values": false }, "textMode": "auto" },
+      "targets": [{ "expr": "100 * rate(valid_get_nums_[5m]) / (rate(total_get_nums_[5m]) + 1)", "legendFormat": "", "refId": "A" }],


This PromQL query contains trailing underscores and is missing the _total suffix for the counter metrics.

Suggested change

"targets": [{ "expr": "100 * rate(valid_get_nums_[5m]) / (rate(total_get_nums_[5m]) + 1)", "legendFormat": "", "refId": "A" }],

"targets": [{ "expr": "100 * rate(valid_get_nums_total[5m]) / (rate(total_get_nums_total[5m]) + 1)", "legendFormat": "", "refId": "A" }],

gemini-code-assist · 2026-07-16T02:48:34Z

+        { "expr": "rate(valid_get_nums_[5m])", "legendFormat": "Valid Gets", "refId": "A" },
+        { "expr": "rate(total_get_nums_[5m])", "legendFormat": "Total Gets", "refId": "B" }


These PromQL queries contain trailing underscores and are missing the _total suffix for the counter metrics.

Suggested change

{ "expr": "rate(valid_get_nums_[5m])", "legendFormat": "Valid Gets", "refId": "A" },

{ "expr": "rate(total_get_nums_[5m])", "legendFormat": "Total Gets", "refId": "B" }

{ "expr": "rate(valid_get_nums_total[5m])", "legendFormat": "Valid Gets", "refId": "A" },

{ "expr": "rate(total_get_nums_total[5m])", "legendFormat": "Total Gets", "refId": "B" }

gemini-code-assist · 2026-07-16T02:48:34Z

+      "gridPos": { "h": 4, "w": 4, "x": 0, "y": 109 },
+      "id": 901,
+      "options": { "colorMode": "value", "graphMode": "area", "justifyMode": "auto", "orientation": "auto", "reduceOptions": { "calcs": ["lastNotNull"], "fields": "", "values": false }, "textMode": "auto" },
+      "targets": [{ "expr": "master_snapshot_success", "legendFormat": "", "refId": "A" }],


The snapshot success metric is registered as a counter in MasterMetricManager and is missing the _total suffix in its PromQL query.

Suggested change

"targets": [{ "expr": "master_snapshot_success", "legendFormat": "", "refId": "A" }],

"targets": [{ "expr": "master_snapshot_success_total", "legendFormat": "", "refId": "A" }],

liangxu2000 · 2026-07-16T03:22:44Z

Thanks for the detailed review! After investigation, I believe these suggestions are based on a misconception about the ylt::metric::counter_t library. The library does not auto-append the _total suffix to counter metric names — the name passed to the constructor is exposed verbatim by Prometheus.

Evidence that the library does not auto-append `_total`

In mooncake-store/src/master_metric_manager.cpp, RPC counters are already registered with an explicit _total suffix:

put_start_requests_("master_put_start_requests_total", "Total number of PutStart requests received"),
remove_requests_("master_remove_requests_total", "Total number of Remove requests received"),
// ... dozens more RPC counters

If the library auto-appended _total, these would be exposed as master_put_start_requests_total_total (double suffix), which would conflict with the Prometheus output. Since these RPC metrics work correctly in the dashboard, the library does not auto-append.

Counter metrics without `_total` suffix are valid

Several counters in the source code are intentionally registered without _total — they match the exact strings passed to the constructor:

Source code string	Dashboard query	Correct?
`"master_attempted_evictions_mem"`	`master_attempted_evictions_mem`	✅
`"master_evicted_key_count"`	`master_evicted_key_count`	✅
`"master_snapshot_success"`	`master_snapshot_success`	✅
`"master_put_start_discard_cnt"`	`master_put_start_discard_cnt`	✅

Verified against a running mooncake_master — curl http://localhost:9009/metrics returns:

# HELP master_evicted_key_count Total number of keys evicted
# TYPE master_evicted_key_count counter
master_evicted_key_count 0
# HELP master_put_start_discard_cnt Total number of discarded PutStart operations
# TYPE master_put_start_discard_cnt counter
master_put_start_discard_cnt 0
# HELP master_snapshot_success Total number of successful snapshot operations
# TYPE master_snapshot_success counter
master_snapshot_success 2493

These are counter type metrics (per # TYPE line) without _total suffix, confirming the naming is intentional and not a bug.

Regarding trailing underscores

Metrics like mem_cache_nums_, file_cache_nums_, mem_cache_hit_nums_ are the actual names registered in the source code — for example, mem_cache_nums_("mem_cache_nums_", "Current number of cached values in the memory pool"). The trailing underscore is intentional and the dashboard queries match the source verbatim.

liangxu2000 · 2026-07-16T06:41:54Z

Ping @stmatengss for review — you originally authored this dashboard in #1335. The bot's _total suffix suggestions have been rebutted with source code evidence and runtime curl output. Would appreciate your review when you have time.

Lin-z-w · 2026-07-17T08:03:52Z

+      "gridPos": { "h": 4, "w": 4, "x": 4, "y": 5 },
+      "id": 106,
+      "options": { "colorMode": "background", "graphMode": "none", "justifyMode": "auto", "orientation": "auto", "reduceOptions": { "calcs": ["lastNotNull"], "fields": "", "values": false }, "textMode": "auto" },
+      "targets": [{ "expr": "100 * master_nof_allocated_bytes / master_total_nof_capacity_bytes", "legendFormat": "", "refId": "A" }],


These NoF gauges are registered in MasterMetricManager, but they are never serialized by MasterMetricManager::serialize_metrics(), so they do not appear in the real Master's /metrics response.

The same issue affects 19 metric names used by this dashboard: the NoF capacity gauges, NoF mount/unmount/remount counters, per-tier eviction counters, and batch item counters. As a result, five panels are completely empty and two eviction panels are only partially populated when connected to a real Master.

Please either add the missing metrics to serialize_metrics() or remove/replace these queries so every panel references metrics that are actually exported.

Lin-z-w · 2026-07-17T08:04:50Z

+      "gridPos": { "h": 4, "w": 6, "x": 0, "y": 91 },
+      "id": 704,
+      "options": { "colorMode": "background", "graphMode": "none", "justifyMode": "auto", "orientation": "auto", "reduceOptions": { "calcs": ["lastNotNull"], "fields": "", "values": false }, "textMode": "auto" },
+      "targets": [{ "expr": "100 * rate(mem_cache_hit_nums_[5m]) / (rate(mem_cache_hit_nums_[5m]) + rate(file_cache_hit_nums_[5m]) + 1)", "legendFormat": "", "refId": "A" }],


Adding a literal 1 to a rate denominator materially biases the result whenever traffic is not much greater than 1 request/s. For example, rates of 0.4 and 0.6 should produce a 40% MEM ratio, but this expression reports 20%.

Please handle division by zero without changing valid non-zero ratios, for example with clamp_min(denominator, 1e-9) or an explicit zero-traffic fallback. The same issue is present in the Valid Get Rate expression on line 668.

Lin-z-w · 2026-07-17T08:05:54Z

+      "id": 803,
+      "options": { "legend": { "calcs": ["mean", "lastNotNull"], "displayMode": "table", "placement": "bottom" }, "tooltip": { "mode": "multi", "sort": "desc" } },
+      "targets": [
+        { "expr": "rate(master_nof_heartbeat_probe_latency_ms_bucket[5m])", "legendFormat": "le={{le}}ms", "refId": "A" }


rate(..._bucket[5m]) returns the number of observations per second accumulated under each le boundary; it does not return latency in milliseconds. Displaying these values with the ms unit therefore mislabels the y-axis and makes the cumulative bucket series difficult to interpret.

Please use histogram_quantile() to display meaningful latency percentiles, for example:

histogram_quantile(0.95, sum by (le) (rate(master_nof_heartbeat_probe_latency_ms_bucket[5m])))

Alternatively, if raw bucket rates are intentional, change the unit and title accordingly. The Snapshot Duration panel on line 785 has the same issue.

1. Serialize missing metrics in MasterMetricManager::serialize_metrics(): - NoF capacity gauges (nof_allocated_size_, nof_total_capacity_, nof_allocated_size_per_segment_, nof_total_capacity_per_segment_) - NoF segment mount/unmount/remount counters and their failures - Tiered eviction counters (mem_eviction_*, nof_eviction_*) - Batch operation items/partial_successes/failed_items counters 2. Fix division-by-constant in percentage panels (id 704, 705): Replace '+ 1' with clamp_min(denominator, 1e-9) to avoid ratio distortion at low traffic. 3. Fix histogram bucket rate misuse as latency (id 803, 903): Replace rate(..._bucket[5m]) with histogram_quantile(0.95, sum by (le) (rate(..._bucket[5m]))) to show true latency percentiles.

…stency Panel 1201 had the same histogram bucket misuse issue as panels 803/903. Replace bare 'master_value_size_bytes_bucket' with histogram_quantile(0.95, sum by (le) (rate(..._bucket[5m]))). Update unit from 'short' to 'bytes' and title to reflect p95 value size.

Lin-z-w · 2026-07-17T10:08:59Z

+    },
+    {
+      "collapsed": false,
+      "gridPos": { "h": 1, "w": 24, "x": 0, "y": 9 },


This row starts at y=9, but panel 108 also starts at y=9 with h=8, so it occupies rows 9 through 16 while this row spans the full dashboard width.

Grafana resolves the overlap by moving one of the elements during layout. In a local render, the "NoF SSD Storage" panel was placed under the RPC QPS row instead of remaining in the Cluster Overview section.

Please move this row to y=17 and shift the following panels accordingly, or otherwise assign non-overlapping grid positions.

Lin-z-w · 2026-07-17T10:09:13Z

+  "annotations": { "list": [ { "builtIn": 1, "datasource": "-- Grafana --", "enable": true, "hide": true, "iconColor": "rgba(0, 211, 255, 1)", "name": "Annotations & Alerts", "type": "dashboard" } ] },
  "editable": true,
  "gnetId": null,
+  "refresh": "500ms",


500ms is below Grafana's default min_refresh_interval of 5 seconds. Grafana clamps this value to 5 seconds and logs a warning, so the committed dashboard does not actually refresh at the configured interval under the provided monitoring setup.

Please set this to 5s, or explicitly provision and document a lower min_refresh_interval if sub-second refresh is required.

gongshanchong · 2026-07-17T11:00:32Z

When will this branch be merged? This feature is really great

codecov-commenter · 2026-07-17T11:49:49Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

stmatengss · 2026-07-18T04:43:38Z

Good work! Very appreciated

feat: expand Grafana dashboard with comprehensive Master metrics panels

8e7e905

gemini-code-assist Bot reviewed Jul 16, 2026

View reviewed changes

github-actions Bot added the run-ci label Jul 16, 2026

liangxu2000 closed this Jul 16, 2026

liangxu2000 reopened this Jul 16, 2026

ykwd requested a review from Lin-z-w July 17, 2026 06:35

Lin-z-w self-assigned this Jul 17, 2026

Lin-z-w reviewed Jul 17, 2026

View reviewed changes

liangxu2000 requested review from XucSh, YiXR, stmatengss and ykwd as code owners July 17, 2026 08:51

github-actions Bot added the Store label Jul 17, 2026

Lin-z-w reviewed Jul 17, 2026

View reviewed changes

		{ "expr": "rate(mem_cache_hit_nums_[5m])", "legendFormat": "MEM Hits", "refId": "A" },
		{ "expr": "rate(file_cache_hit_nums_[5m])", "legendFormat": "SSD Hits", "refId": "B" }

		{ "expr": "mem_cache_nums_", "legendFormat": "MEM Objects", "refId": "A" },
		{ "expr": "file_cache_nums_", "legendFormat": "SSD Objects", "refId": "B" }

	"targets": [{ "expr": "100 * rate(valid_get_nums_[5m]) / (rate(total_get_nums_[5m]) + 1)", "legendFormat": "", "refId": "A" }],
	"targets": [{ "expr": "100 * rate(valid_get_nums_total[5m]) / (rate(total_get_nums_total[5m]) + 1)", "legendFormat": "", "refId": "A" }],

		{ "expr": "rate(valid_get_nums_[5m])", "legendFormat": "Valid Gets", "refId": "A" },
		{ "expr": "rate(total_get_nums_[5m])", "legendFormat": "Total Gets", "refId": "B" }

	"targets": [{ "expr": "master_snapshot_success", "legendFormat": "", "refId": "A" }],
	"targets": [{ "expr": "master_snapshot_success_total", "legendFormat": "", "refId": "A" }],

Uh oh!

Conversation

liangxu2000 commented Jul 16, 2026

Description

Module

Type of Change

How Has This Been Tested?

Checklist

AI Assistance Disclosure

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

liangxu2000 commented Jul 16, 2026

Evidence that the library does not auto-append _total

Counter metrics without _total suffix are valid

Regarding trailing underscores

Uh oh!

liangxu2000 commented Jul 16, 2026

Uh oh!

Lin-z-w Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

Lin-z-w Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

Lin-z-w Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

Lin-z-w Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

Lin-z-w Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

gongshanchong commented Jul 17, 2026

Uh oh!

codecov-commenter commented Jul 17, 2026

Codecov Report

Uh oh!

stmatengss commented Jul 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Evidence that the library does not auto-append `_total`

Counter metrics without `_total` suffix are valid