Add LRU in MasterService, complexity O(1)

[zhaoyongke]

We found mooncake store will discard all the put() requests after its capacity reaches the max. This case is not applicable in our online service. We solved this via LRU( Least Recently Used) algorithm.

[xiaguan]

Thank you for your contribution. Here are a few points that might warrant consideration:

First, if eviction support were implemented, the garbage collection (GC) related functionalities could potentially be removed.

Second, as reflected in the codebase, our current implementation utilizes lock sharding, wherein each object is safeguarded by the lock corresponding to its shard. Implementing a Least Recently Used (LRU) eviction strategy, however, would likely necessitate a global lock to manage the LRU queue, potentially introducing performance bottlenecks due to contention.

My proposal involves repurposing the existing GC thread into a eviction thread. When eviction becomes necessary—perhaps triggered by monitoring specific watermarks or thresholds—this thread could select a target shard, acquire the lock exclusive to that shard, and subsequently perform eviction operations solely within its confines. This approach aims to circumvent the performance limitations associated with a global lock.

[xiaguan]

I've conceived a potentially simpler approach that we could discuss. If we modify the garbage collection (GC) producer's operation from 'get' to 'put_end', would this effectively function as a First-In, First-Out (FIFO) eviction mechanism?

[zhaoyongke]

Thank you for your contribution. Here are a few points that might warrant consideration:

First, if eviction support were implemented, the garbage collection (GC) related functionalities could potentially be removed.

Second, as reflected in the codebase, our current implementation utilizes lock sharding, wherein each object is safeguarded by the lock corresponding to its shard. Implementing a Least Recently Used (LRU) eviction strategy, however, would likely necessitate a global lock to manage the LRU queue, potentially introducing performance bottlenecks due to contention.

My proposal involves repurposing the existing GC thread into a eviction thread. When eviction becomes necessary—perhaps triggered by monitoring specific watermarks or thresholds—this thread could select a target shard, acquire the lock exclusive to that shard, and subsequently perform eviction operations solely within its confines. This approach aims to circumvent the performance limitations associated with a global lock.

Thanks for your constructive suggestions!
We noticed the GC option in mooncake master. It simply removes an item after get(), not applicable to single put() multiple get() situations(such as nPmD). we disabled this option by default.

[zhaoyongke]

I've conceived a potentially simpler approach that we could discuss. If we modify the garbage collection (GC) producer's operation from 'get' to 'put_end', would this effectively function as a First-In, First-Out (FIFO) eviction mechanism?

Exactly yes, we have tested this method, while in real cases, FIFO is not always the best eviction method, which client will frequently get a subset of a large prefilled datasets.

[xiaguan]

Can you ensure that the LRU implementation is thread-safe?

[zhaoyongke]

Can you ensure that the LRU implementation is thread-safe?

Work in process, later will fix

[Copilot]

Pull Request Overview

This PR introduces an LRU mechanism into the MasterService to prevent discarding put() requests when the store reaches maximum capacity. Key changes include:

• Initializing and clearing LRU data structures in the MasterService constructor and destructor.
• Updating the Get() and PutStart() methods to incorporate LRU update and eviction logic.
• Adding preprocessor definitions and corresponding LRU member variables to the header file.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
mooncake-store/src/master_service.cpp	Implements LRU update and eviction in various service methods.
mooncake-store/include/master_service.h	Adds preprocessor flags and LRU data structures.

Comments suppressed due to low confidence (2)

mooncake-store/include/master_service.h:18

• [nitpick] Consider defining USE_LRU_MASTER without a string value (e.g. using '#define USE_LRU_MASTER') to better align with standard preprocessor flag practices.

#define USE_LRU_MASTER "ON"

mooncake-store/src/master_service.cpp:143

• [nitpick] Consider refactoring the duplicate LRU update logic found in Get() and PutStart() into a helper function to improve code maintainability and reduce redundancy.

all_key_list_.push_front(key);

[Copilot]

Verify that the eviction condition correctly reflects the intended capacity constraint; if LRU_MAX_CAPACITY is the max allowed entries, eviction should occur only when adding a new key would exceed that capacity.

Suggested change

	if(all_key_list_.size() >= LRU_MAX_CAPACITY)
	if(all_key_list_.size() > LRU_MAX_CAPACITY)

[stmatengss]

Hi, @zhaoyongke , Thank you for your contributions, https://github.com/kvcache-ai/Mooncake/actions/runs/14609309219/job/40984273235?pr=287. master_service_test has failed. Please check it.

[stmatengss]

Define it in the CMake File?

[zhaoyongke]

Fine, I'll fix it

[stmatengss]

Use a class (e.g., class LRUList {....}) to wrap these two LRU lists.

[doujiang24]

yep, and we'd better add some basic tests for the new class.

[doujiang24]

we'd better move it to command arguments, so that we can change it on demand.