Loading cache improve eviction use policy #9708
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Use std::pmr::polymorphic_allocator instead of std::allocator - the former allows not to define the allocated object during the template specification. As a result we won't have to have lru_entry defined before loading_cache, which in line would allow us to rearrange classes making all classes internal to loading_cache and hence simplifying the interface. Signed-off-by: Vlad Zolotarov <vladz@scylladb.com>
Hide internal classes inside the loading_cache class: * Simpler calls - no need for a tricky back-referencing to access loading_cache fields. * Cleaner interface. Signed-off-by: Vlad Zolotarov <vladz@scylladb.com>
* Store a reference to a parent (loading_cache) object instead of holding
references to separate fields.
* Access loading_cache fields via accessors.
* Move the LRU "touch" logic to the loading_cache.
* Keep only a plain "list entry" logic in the lru_entry class.
Signed-off-by: Vlad Zolotarov <vladz@scylladb.com>
…cache entry when accessed Always "touch" a prepared_statements_cache entry when it's accessed via authorized_prepared_statements_cache. If we don't do this it may turn out that the most recently used prepared statement doesn't have the newest last_read timestamp and can get evicted before the not-so-recently-read statement if we need to create space in the prepared statements cache for a new entry. And this is going to trigger an eviction of the corresponding entry from the authorized_prepared_cache breaking the LRU paradigm of these caches. Fixes scylladb#9590 Signed-off-by: Vlad Zolotarov <vladz@scylladb.com>
|
@nyh FYI. |
|
Hold on - there is some issue with a unit test. |
…(LFRU) eviction policy
This patch implements a simple variation of LFRU eviction policy:
* We define 2 dynamic cache sections which total size should not exceed the maximum cache size.
* New cache entry is always added to the "unprivileged" section.
* After a cache entry is read more than SectionHitThreshold times it moves to the second cache section.
* Both sections' entries obey expiration and reload rules in the same way as before this patch.
* When cache entries need to be evicted due to a size restriction "unprivileged" section's
least recently used entries are evicted first.
Note:
With a 2 sections cache it's not enough for a new entry to have the latest timestamp
in order not be evicted right after insertion: e.g. if all all other entries
are from the privileged section.
And obviously we want to allow new cache entries to be added to a cache.
Therefore we can no longer first add a new entry and then shrink the cache.
Switching the order of these two operations resolves the culprit.
Fixes scylladb#8674
Signed-off-by: Vlad Zolotarov <vladz@scylladb.com>
Provide a template parameter to provide a static callbacks object to increment a counter of evictions from the unprivileged section. If entries are evicted from the cache while still in the unprivileged section indicates a not efficient usage of the cache and should be investigated. This patch instruments authorized_prepared_statements_cache and a prepared_statements_cache objects to provide non-empty callbacks. Signed-off-by: Vlad Zolotarov <vladz@scylladb.com>
vladzcloudius
force-pushed
the
loading_cache_improve_eviction_use_policy-v3
branch
from
e58fcab
to
4cb245f
Compare
Force-pushed the fix. |
nyh
approved these changes
Dec 1, 2021
| @@ -86,6 +96,7 @@ struct simple_entry_size { | |||
| /// | |||
| /// \tparam Key type of the cache key | |||
| /// \tparam Tp type of the cached value | |||
| /// \tparam SectionHitThreshold number of hits after which a cache item is going to be moved to the privileged cache section. | |||
There was a problem hiding this comment.
I've said this too many times already but the word "after" is ambiguous. It should have been phrased differently.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This series introduces a new version of a loading_cache class.
The old implementation was susceptible to a "pollution" phenomena when frequently used entry can get evicted by an intensive burst of "used once" entries pushed into the cache.
The new version is going to have a privileged and unprivileged cache sections and there's a new loading_cache template parameter - SectionHitThreshold. The new cache algorithm goes as follows:
least recently used entries are evicted first.
More details may be found in #8674.
In addition, during a testing another issue was found in the authorized_prepared_statements_cache: #9590.
There is a patch that fixes it as well.