[Store] Treat new LRU buckets as recently admitted

[feichai0017]

Description

Fixes #2976.

• Record a newly committed bucket's admission time in the LRU index.
• Keep failed commits out of the index and cover eviction ordering against an older, recently read bucket.

Module

• Mooncake Store (mooncake-store)

Type of Change

• Bug fix

How Has This Been Tested?

Test commands:

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

Test results (Ubuntu):

• Unit tests pass (storage_backend_test: 73/73)
• Integration tests pass (not applicable)
• 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 BucketStorageBackend::BatchOffload method to record the admission time of newly added buckets when the LRU eviction policy is active, preventing them from being immediately evicted. A unit test is added to verify this behavior. The reviewer suggested explicitly casting the steady clock duration to nanoseconds to ensure portability and correctness, as raw clock ticks are implementation-defined.

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]

Using std::chrono::steady_clock::now().time_since_epoch().count() directly returns clock ticks, which are implementation-defined and not guaranteed to be in nanoseconds (even though the variable is named last_access_ns_). To ensure portability and correctness across different platforms and compilers, it is highly recommended to explicitly cast the duration to std::chrono::nanoseconds before calling .count().

                admission_time = std::chrono::duration_cast<std::chrono::nanoseconds>(
                                     std::chrono::steady_clock::now().time_since_epoch())
                                     .count();

[feichai0017]

Handled in 592cd02: all LRU timestamps now use an explicit nanosecond cast.

[codecov-commenter]

⚠️ 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!

[he-yufeng]

The core fix is right: seeding a fresh bucket's lru_index_ key with 0 made it look infinitely stale, so a just-written bucket was the first eviction candidate; stamping it with the admission time fixes that. The regression test captures it well (without the change lru_old survives and lru_new is wrongly evicted).

One consistency gap worth a look, though. This change makes the BatchOffload path insert {admission_time, bucket_id} into lru_index_, but the rest of the code still assumes the key is 0 for that bucket:

• the delete path does lru_index_.erase({0LL, bucket_id}) (and so do the metadata-load cleanup paths), and
• the metadata-recovery path still emplace(0LL, bucket_id).

Because lru_index_ is a std::set<{ts, bucket_id}>, erase({0LL, bucket_id}) no longer matches a bucket that was admitted through BatchOffload (its real key is {admission_time, bucket_id}), so deleting such a bucket leaves a dangling {admission_time, deleted_id} entry behind. The eviction loop's self-healing (buckets_.find(id) == end() -> erase(top_it)) does eventually reap those orphans, so this isn't a hard leak — but until an eviction pass runs (e.g. on a deployment that stays under the watermark), the stale entries accumulate in the index.

So the fix is sound for the eviction ordering it targets; the loose end is that erase({0LL, ...}) and the recovery-path emplace(0LL, ...) now silently disagree with the admission key. Two ways to close it: have the delete/cleanup paths erase with the bucket's real last_access_ns_ (the actual key), or lean explicitly on the eviction loop's repair and drop the direct erase({0LL, ...}) calls. Either is fine — mostly want the "key is always 0" assumption to stop being half-true after this change. Did you consider stamping the recovery-path buckets the same way, for symmetry?

[feichai0017]

@he-yufeng Thanks for catching this. 6df3835 removes the zero-timestamp assumption: recovery registers validated buckets with one recovery timestamp, and delete/rollback paths erase LRU entries by bucket ID.