Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: introduce new in-memory cache abstraction (intrusive version) #270

Merged
merged 43 commits into from
Mar 7, 2024

Conversation

MrCroxx
Copy link
Owner

@MrCroxx MrCroxx commented Feb 28, 2024

What's changed and what's your intention?

Please explain IN DETAIL what the changes are in this PR and why they are needed. :D

A complete version of #262 .

Summary (from the crate documentation):

//! This crate provides a concurrent in-memory cache component that supports replaceable eviction algorithm.
//!
//! # Motivation
//!
//! There are a few goals to achieve with the crate:
//!
//! 1. Pluggable eviction algorithm with the same abstraction.
//! 2. Tracking the real memory usage by the cache. Including both holding by the cache and by the external users.
//! 3. Reduce the concurrent read-through requests into one.
//!
//! To achieve them, the crate needs to combine the advantages of the implementations of RocksDB and CacheLib.
//!
//! # Design
//!
//! The cache is mainly compused of the following components:
//! 1. handle             : Carries the cached entry, reference count, pointer links in the eviction container, etc.
//! 2. indexer            : Indexes cached keys to the handles.
//! 3. eviction container : Defines the order of eviction. Usually implemented with intrusive data structures.
//!
//! Because a handle needs to be referenced and mutated by both the indexer and the eviction container in the same
//! thread, it is hard to implement in 100% safe Rust without overhead. So, the APIs of the indexer and the eviciton
//! container are defined with `NonNull` pointers of the handles.
//!
//! When some entry is inserted into the cache, the associated handle should be transmuted into pointer without
//! dropping. When some entry is removed from the cache, the pointer of the associated handle should be transmuted into
//! an owned data structure.
//!
//! # Handle Lifetime
//!
//! The handle is created during a new entry is being inserted, and then inserted into both the indexer and the eviction
//! container.
//!
//! The handle is return if the entry is retrieved from the cache. The handle will track the count of the external
//! owners to decide the time to reclaim.
//!
//! When a key is removed or updated, the original handle will be removed from the indexer and the eviction container,
//! and waits to be released by all the external owners before reclamation.
//!
//! When the cache is full and being inserted, a handle will be evicted from the eviction container based on the
//! eviction algorithm. The evicted handle will NOT be removed from the indexer immediately because it still occupies
//! memory and can be used by queries followed up.
//!
//! After the handle is released by all the external owners, the eviction container will update its order or evict it
//! based on the eviction algorithm. If it doesn't appear in the eviction container, it may be reinserted if it it still
//! in the indexer and there is enough space. Otherwise, it will be removed from both the indexer and the eviction
//! container.
//!
//! The handle that does not appear in either the indexer or the eviction container, and has no external owner, will be
//! destroyed.

Checklist

  • I have written necessary rustdoc comments
  • I have added necessary unit tests and integration tests
  • I have passed make check and make test or make all in my local envirorment.

Related issues or PRs (optional)

Related PR in RisingWave: risingwavelabs/risingwave#15382

Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Signed-off-by: MrCroxx <mrcroxx@outlook.com>
TODO: add ut for Cache

Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Signed-off-by: MrCroxx <mrcroxx@outlook.com>
@MrCroxx MrCroxx added the feature New feature or request label Feb 28, 2024
@MrCroxx MrCroxx self-assigned this Feb 28, 2024
Copy link

codecov bot commented Feb 28, 2024

Codecov Report

Attention: Patch coverage is 87.35060% with 127 lines in your changes are missing coverage. Please review.

Project coverage is 75.55%. Comparing base (198b8ae) to head (eaa69fc).

Files Patch % Lines
foyer-memory/src/cache.rs 82.25% 85 Missing ⚠️
foyer-intrusive/src/collections/dlist.rs 14.28% 12 Missing ⚠️
foyer-memory/src/eviction/lru.rs 95.61% 11 Missing ⚠️
foyer-memory/src/handle.rs 89.42% 11 Missing ⚠️
foyer-memory/src/eviction/fifo.rs 93.63% 7 Missing ⚠️
foyer-memory/src/indexer.rs 97.82% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #270      +/-   ##
==========================================
+ Coverage   74.22%   75.55%   +1.33%     
==========================================
  Files          52       56       +4     
  Lines        5997     6861     +864     
==========================================
+ Hits         4451     5184     +733     
- Misses       1546     1677     +131     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

if let Some(old) = self.indexer.insert(ptr) {
debug_assert!(old.as_ref().base().is_in_eviction());
self.eviction.remove(old);
debug_assert!(!old.as_ref().base().is_in_eviction());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add one more debug_assert?

debug_assert!(!old.as_ref().base().is_in_indexer());

///
/// Return `Some(..)` if the handle is released, or `None` if the handle is still in use.
unsafe fn remove(&mut self, hash: u64, key: &K) -> Option<(K, V)> {
let ptr = self.indexer.remove(hash, key)?;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

debug_assert!(!ptr.as_ref().base().is_in_indexer());

// The handles in the indexer covers the handles in the eviction container.
// So only the handles drained from the indexer need to be released.
for ptr in ptrs {
if let Some(entry) = self.try_release_handle(ptr) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

debug_assert!(!ptr.as_ref().base().is_in_indexer());

let base = evicted.as_ref().base();
debug_assert!(base.is_in_indexer());
debug_assert!(!base.is_in_eviction());
if let Some(entry) = self.try_release_handle(evicted) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed offline: reinsert in try_release_handle may be triggered, which will cause surprise, because self.usage.load(Ordering::Relaxed) <= self.capacity may not be held here.

Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Copy link
Collaborator

@Little-Wallace Little-Wallace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Signed-off-by: MrCroxx <mrcroxx@outlook.com>
@MrCroxx MrCroxx mentioned this pull request Mar 6, 2024
Copy link
Collaborator

@hzxa21 hzxa21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for the PR!

Signed-off-by: MrCroxx <mrcroxx@outlook.com>
//! memory and can be used by queries followed up.
//!
//! After the handle is released by all the external owners, the eviction container will update its order or evict it
//! based on the eviction algorithm. If it doesn't appear in the eviction container, it may be reinserted if it it still
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo ? if it it .....

Signed-off-by: MrCroxx <mrcroxx@outlook.com>
@MrCroxx MrCroxx merged commit 65c1646 into main Mar 7, 2024
12 checks passed
@MrCroxx MrCroxx deleted the xx/foyer-memory-intrusive branch March 7, 2024 03:41
MrCroxx added a commit that referenced this pull request Apr 17, 2024
* feat: introduce foyer-memory Cache abstraction

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* chore: remove old foyer-memory code

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* feat: introduce base handle

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* feat: make cache abstraction use handle trait with base handle

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* feat: impl fifo eviction

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* chore: add basic tests

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* refactor: refactor directory hierarchy

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* fix: call access when get

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* chore: make hakari happy

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* fix: fix dangling ptr after remove

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* feat: introduce RemovableQueue to make FIFO impl easilier.

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* refactor: use removable queue to simplify fifo impl

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* fix: fix removable queue grow token updates

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* chore: tiny refactors

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* refactor: use full type name for trait associated type

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* feat: introduce associate type Context for handle

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* feat: introduce lru eviction policy for foyer-memory

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* fix: add a simple ut and fix bugs

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* chore: remove unused generic type

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* refactor: use rocksdb lru impl instead of cachelib lru

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* feat: add waiters and entry API for request dedup

TODO: add ut for Cache

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* feat: introduce new in-memory cache abstraction into foyer-memory

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* refactor: use intrusive dlist to impl fifo

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* refactor: use intrusive dlist to impl LRU with high-pri pool

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* fix: fix reinsert bugs

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* test: add ut and fix some bugs

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* chore: make clippy happy

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* feat: expose foyer-memory via foyer crate

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* feat: allow entry future returns context besides

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* chore: make lru context clone and copy

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* chore: make lru context derive Eq and PartialEq

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* feat: impl Future for Entry

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* chore: remove unused comments

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* doc: add crate document

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* test: add more uts for cache

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* test: add ut to cover not reinsert for full

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

* chore: fix typo

Signed-off-by: MrCroxx <mrcroxx@outlook.com>

---------

Signed-off-by: MrCroxx <mrcroxx@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants