Skip to content

correct handling for expired items in eviction #86

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 6, 2023
Merged

correct handling for expired items in eviction #86

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions cachelib/allocator/CacheAllocator-inl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1600,11 +1600,12 @@ CacheAllocator<CacheTrait>::findEviction(TierId tid, PoolId pid, ClassId cid) {

Item* toRecycle = nullptr;
Item* candidate = nullptr;
bool isExpired = false;
typename NvmCacheT::PutToken token;

mmContainer.withEvictionIterator([this, tid, pid, cid, &candidate, &toRecycle,
&searchTries, &mmContainer, &lastTier,
&token](auto&& itr) {
&isExpired, &token](auto&& itr) {
if (!itr) {
++searchTries;
(*stats_.evictionAttempts)[tid][pid][cid].inc();
Expand Down Expand Up @@ -1633,14 +1634,16 @@ CacheAllocator<CacheTrait>::findEviction(TierId tid, PoolId pid, ClassId cid) {

if (evictToNvmCache && !token_.isValid()) {
stats_.evictFailConcurrentFill.inc();
} else if ( (lastTier && candidate_->markForEviction()) ||
} else if ( ((lastTier || candidate_->isExpired()) &&
candidate_->markForEviction()) ||
(!lastTier && candidate_->markMoving(true)) ) {
XDCHECK(candidate_->isMoving() || candidate_->isMarkedForEviction());
// markForEviction to make sure no other thead is evicting the item
// nor holding a handle to that item if this is last tier
// since we won't be moving the item to the next tier
toRecycle = toRecycle_;
candidate = candidate_;
isExpired = candidate_->isExpired();
token = std::move(token_);

// Check if parent changed for chained items - if yes, we cannot
Expand Down Expand Up @@ -1674,7 +1677,7 @@ CacheAllocator<CacheTrait>::findEviction(TierId tid, PoolId pid, ClassId cid) {
XDCHECK(toRecycle);
XDCHECK(candidate);

auto evictedToNext = lastTier ? nullptr
auto evictedToNext = (lastTier || isExpired) ? nullptr
: tryEvictToNextMemoryTier(*candidate, false);
if (!evictedToNext) {
//if insertOrReplace was called during move
Expand All @@ -1700,7 +1703,7 @@ CacheAllocator<CacheTrait>::findEviction(TierId tid, PoolId pid, ClassId cid) {
// as exclusive since we will not be moving the item to the next tier
// but rather just evicting all together, no need to
// markForEvictionWhenMoving
auto ret = lastTier ? true : candidate->markForEvictionWhenMoving();
auto ret = (lastTier || isExpired) ? true : candidate->markForEvictionWhenMoving();
XDCHECK(ret);

unlinkItemForEviction(*candidate);
Expand Down Expand Up @@ -1807,11 +1810,6 @@ CacheAllocator<CacheTrait>::tryEvictToNextMemoryTier(
XDCHECK(item.isMoving());
XDCHECK(item.getRefCount() == 0);
if(item.hasChainedItem()) return WriteHandle{}; // TODO: We do not support ChainedItem yet
if(item.isExpired()) {
accessContainer_->remove(item);
item.unmarkMoving();
return acquire(&item);
}

TierId nextTier = tid; // TODO - calculate this based on some admission policy
while (++nextTier < getNumTiers()) { // try to evict down to the next memory tiers
Expand Down