[Store] Initialize file-per-key storage with eviction disabled

[feichai0017]

Description

Fixes #2981.

• Treat file-per-key offloading as unavailable until metadata has been scanned, and enforce the same state in BatchOffload().
• Refresh the mounted segment after recovery so the master receives the final eviction-disabled state before heartbeats start.

Module

• Mooncake Store (mooncake-store)

Type of Change

• Bug fix

How Has This Been Tested?

Test commands:

pre-commit run --files mooncake-store/src/file_storage.cpp mooncake-store/src/storage_backend.cpp mooncake-store/tests/file_storage_test.cpp mooncake-store/tests/storage_backend_test.cpp
cmake --build build-focused --target storage_backend_test file_storage_test
./build-focused/mooncake-store/tests/storage_backend_test --gtest_brief=1
./build-focused/mooncake-store/tests/file_storage_test --gtest_brief=1

Test results (Ubuntu):

• Unit tests pass (storage_backend_test: 72/72; file_storage_test: 24/24)
• Integration tests pass (file-per-key initialization with eviction disabled)
• Manual testing done

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 (touched-file hooks pass)
• I have updated the documentation (not applicable)
• I have added tests to prove my changes are effective
• For changes >500 LOC: I have filed an RFC issue (not applicable; under 500 LOC)

AI Assistance Disclosure

• No AI tools were used
• AI tools were used: Codex assisted with investigation, implementation, testing, and review.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[gemini-code-assist]

Code Review

This pull request updates the offloading initialization and state handling in the storage backend. Specifically, it refreshes the local offloading state in FileStorage::Init() after the metadata scan, returns false instead of an internal error in IsEnableOffloading() when metadata has not yet been scanned, and adds a check in BatchOffload to reject offloading if it is disabled. Additionally, corresponding unit tests have been updated and added to verify initialization behavior when eviction is disabled. There are no review comments, and I have no feedback to provide.

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.

[codecov-commenter]

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

Codecov Report

❌ Patch coverage is 82.85714% with 12 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
mooncake-store/src/file_storage.cpp	44.44%	10 Missing ⚠️
mooncake-store/tests/storage_backend_test.cpp	90.00%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[he-yufeng]

The direction is right: treating "metadata not scanned yet" as a hard INTERNAL_ERROR was too aggressive for a backend that only knows its quota after the initial scan, so mounting conservatively-disabled and refreshing after the scan in Init() is a cleaner startup contract. Two things worth a look before merge.

1. IsEnableOffloading() now collapses two distinct states into false. Before, "not scanned yet" surfaced as an error, distinct from a genuine "offloading is disabled". After this change both return false, so a caller can no longer tell "not ready yet, retry later" from "the operator turned offloading off". For the two callers here that's fine — BatchOffload rejects either way, and Init() calls it only after ScanMeta has set meta_scanned_ — but the method is virtual with several overrides and is also read on the heartbeat path (file_storage.cpp:326, which still logs an error branch). Worth confirming no consumer relied on the error to mean "startup incomplete" rather than "disabled by config", since that signal is now gone.

2. KEYS_ULTRA_LIMIT is a confusing code for "disabled". When offloading is off, BatchOffload returns ErrorCode::KEYS_ULTRA_LIMIT, which reads as "the key/size cap was hit" — a capacity condition — rather than "offloading isn't enabled". A caller (or someone reading logs) could easily misdiagnose a disabled backend as a full one. If there's an OFFLOAD_DISABLED-style code (or room to add one) it would make the rejection self-explanatory; at minimum a one-line comment at the return site would help.

Neither blocks the fix. #1 is the one I'd most like confirmed, since the lost distinction is easy to depend on implicitly on the heartbeat/startup path.

[feichai0017]

@he-yufeng Thanks for flagging both points. 7b3f646 now reports the pre-scan state as UNABLE_OFFLOADING, which FileStorage::Init() treats as transient while mounting conservatively disabled. After the scan, false remains a quota condition and BatchOffload() returns KEYS_ULTRA_LIMIT; both states are covered by tests.