[store] feat: add secondary storage usage monitor #976

yejj710 · 2025-10-28T09:44:31Z

background

Mooncake has already supported tiered caching, but currently only memory usage is tracked by mooncake_master

new feature

recording of SSD persistent storage usage
configuration for maximum available SSD capacity

gemini-code-assist · 2025-10-28T09:45:00Z

Summary of Changes

Hello @yejj710, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request extends Mooncake's storage monitoring capabilities beyond memory to include SSD persistent storage. It introduces a new configuration option for defining the total SSD capacity and implements the necessary logic within the MasterService and MasterMetricManager to accurately track allocated SSD space and report its usage. This enhancement provides a more comprehensive view of resource utilization in tiered caching environments.

Highlights

SSD Storage Usage Monitoring: Introduced new metrics and mechanisms to track the usage of SSD persistent storage, complementing the existing memory usage tracking in Mooncake's tiered caching system.
Configurable SSD Capacity: Added a new configuration parameter, global_file_segment_size, allowing users to define the maximum available capacity for SSD storage. This parameter has a default value of 500 GiB.
Metric Manager Enhancements: The MasterMetricManager now includes dedicated metrics for allocated file size and total file capacity on SSD, along with methods to increment, decrement, and retrieve these values, as well as calculate the global file usage ratio.
Dynamic Allocation Tracking: Integrated the new SSD usage tracking into the MasterService's PutStart and PutRevoke operations, ensuring that allocated_file_size is updated correctly when disk replicas are created or removed.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces monitoring for secondary storage (SSD), which is a great addition for tiered caching. The changes for configuration and metric collection are mostly well-implemented. However, I've found a critical issue in PutRevoke that could lead to a service crash, and a significant logic omission where allocated_file_size is not decremented upon object removal, which will cause the metric to be inaccurate over time. I've also included a few medium-severity suggestions to improve code readability and correctness. Please review the comments for details.

xiaguan · 2025-10-29T02:20:42Z

mooncake-store/include/types.h

 static constexpr int64_t DEFAULT_CLIENT_LIVE_TTL_SEC = 10;  // in seconds
 static const std::string DEFAULT_CLUSTER_ID = "mooncake_cluster";
 static const std::string DEFAULT_ROOT_FS_DIR = "";
+static const uint64_t DEFAULT_GLOBAL_FILE_SEGMENT_SIZE = 536870912000;  // 500 GiB


This would confuse the user. How about setting it to u64::MAX and showing "infinite"? Since we don't limit DFS usage right now.

good idea! i will fix it later.

stmatengss · 2025-10-29T02:29:06Z

mooncake-store/src/master_metric_manager.cpp

    };

    // Serialize Gauges
    serialize_metric(allocated_size_);


Rename as mem_allocated_size? On the other hand, should we have a total_allocated_size metric?

for question 1: i have renamd memory related metrics already;
for question 2: I don't see concrete scenario for total_allocated_size. It would be more appropriate to extend the functionality when specific scenarios emerge in the future.

stmatengss · 2025-10-29T02:31:15Z

The current PR fails the Build CI Test. Please resolve the issues. @yejj710

/home/runner/work/Mooncake/Mooncake/mooncake-store/src/master_metric_manager.cpp:25:7: error: class ‘mooncake::MasterMetricManager’ does not have any field named ‘total_file_capacity_’
   25 |       total_file_capacity_("master_total_file_capacity_bytes",
      |       ^~~~~~~~~~~~~~~~~~~~
/home/runner/work/Mooncake/Mooncake/mooncake-store/src/master_metric_manager.cpp:27:7: error: class ‘mooncake::MasterMetricManager’ does not have any field named ‘allocated_file_size_’
   27 |       allocated_file_size_("master_allocated_file_size_bytes",
      |       ^~~~~~~~~~~~~~~~~~~~

stmatengss · 2025-10-29T09:09:32Z

mooncake-store/src/master_metric_manager.cpp

    // --- Get current values ---
-    int64_t allocated = allocated_size_.value();
-    int64_t capacity = total_capacity_.value();
+    int64_t allocated = mem_allocated_size_.value();


Rename it as mem_allocated to keep it readable

Thanks. This actually helped me identify a bug in the percentage utilization alerts

stmatengss · 2025-10-29T09:11:04Z

mooncake-store/src/master_service.cpp

        std::string file_path = ResolvePath(key);
        replicas.emplace_back(file_path, total_length,
                              ReplicaStatus::PROCESSING);
+        MasterMetricManager::instance().inc_allocated_file_size(total_length);


Please make sure this is the only path of file allocation.

ok, I've double-checked the code. no problem

xiaguan

If you have some time, you could also update the docs that show up on our website.

If you don’t have much time, just let me know—the PR is ready to merge either way.

xiaguan · 2025-10-31T02:06:14Z

doc/en/mooncake-store.md

 Note: When enabling this feature, the user must ensure that the DFS-mounted directory (`root_fs_dir=/path/to/dir`) is valid and consistent across all client hosts. If some clients have invalid or incorrect mount paths, it may cause abnormal behavior in Mooncake Store.

+#### Persistent Storage Space Configuration
+Mooncake provides configurable DFS available space. Users can specify `--global_file_segment_size=1048576` when starting the master, indicating a maximum usable space of 1MB on DFS.  


Just let users know it's just a metric or something—we didn't evict anything on DFS right now.

ok, I will add more descriptive information

xiaguan · 2025-10-31T02:08:14Z

mooncake-store/src/master_service.cpp

        return tl::make_unexpected(ErrorCode::INVALID_WRITE);
    }
+    // When disk replica is enabled, update allocated_file_size
+    if (use_disk_replica_ && replica_type == ReplicaType::DISK) {


How about we do it in ObjectMetadata's constructor and destructor? That's more RAII and the code looks cleaner.

move it to destructor sounds better, i will slove it today

stmatengss · 2025-10-31T07:50:36Z

@yejj710 Please confirm these issues are resolved so this PR can be merged before today's release.

yejj710 · 2025-10-31T10:24:02Z

@yejj710 Please confirm these issues are resolved so this PR can be merged before today's release.

I have resolved these issues.

stmatengss · 2025-11-01T16:08:32Z

Cant pass CI, plz fix it. @yejj710

stmatengss

Please make the build success.

yejj710 · 2025-11-02T10:20:38Z

Please make the build success.

done. please review it again @stmatengss

stmatengss · 2025-11-03T02:40:12Z

@yejj710 Is it ready for merging?

yejj710 · 2025-11-03T02:55:20Z

@yejj710 Is it ready for merging?

yes ~ @stmatengss

gemini-code-assist bot reviewed Oct 28, 2025

View reviewed changes

xiaguan reviewed Oct 29, 2025

View reviewed changes

stmatengss reviewed Oct 29, 2025

View reviewed changes

stmatengss requested review from stmatengss and xiaguan October 29, 2025 15:25

yejj710 changed the title ~~[store] feat: add secondary storage usage monitor~~ [WIP][store] feat: add secondary storage usage monitor Oct 30, 2025

yejj710 changed the title ~~[WIP][store] feat: add secondary storage usage monitor~~ [store] feat: add secondary storage usage monitor Oct 30, 2025

xiaguan reviewed Oct 31, 2025

View reviewed changes

xiaguan approved these changes Nov 1, 2025

View reviewed changes

stmatengss approved these changes Nov 1, 2025

View reviewed changes

stmatengss requested changes Nov 1, 2025

View reviewed changes

yejj710 added 11 commits November 2, 2025 21:55

add secondary storage usage monitor

d1b4b05

remove useless code

6f3476e

code format & rename memory related metrics

1c1732d

fix some bug

72c4b9b

use int64_t define file capacity & add init total_file_capacity

9a3378d

use int64_t to define file_capacity

72eacd1

rename memory usage related functions

b7494f3

fix 2 bugs

6b4ca2d

add docs

0a06fdd

fix code format

800ac44

add more docs

f445548

yejj710 added 2 commits November 2, 2025 21:55

use RAII way to update allocated_file_size

b870c38

fix ci

48c484d

yejj710 force-pushed the monitor4nfs branch from 476c34e to 48c484d Compare November 2, 2025 13:55

fix CI compile problem

25a33d8

stmatengss approved these changes Nov 3, 2025

View reviewed changes

stmatengss merged commit 14aea87 into kvcache-ai:main Nov 3, 2025
11 checks passed

[store] feat: add secondary storage usage monitor #976

[store] feat: add secondary storage usage monitor #976

Uh oh!

Conversation

yejj710 commented Oct 28, 2025

background

new feature

Uh oh!

gemini-code-assist bot commented Oct 28, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stmatengss commented Oct 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xiaguan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stmatengss commented Oct 31, 2025

Uh oh!

yejj710 commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stmatengss commented Nov 1, 2025

Uh oh!

stmatengss left a comment

Choose a reason for hiding this comment

Uh oh!

yejj710 commented Nov 2, 2025

Uh oh!

stmatengss commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yejj710 commented Nov 3, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yejj710 commented Oct 31, 2025 •

edited

Loading

stmatengss commented Nov 3, 2025 •

edited

Loading