Skip to content

Commit d817018

Browse files
author
Sergei Vinogradov
committed
Fix eviction flow and removeCb calls
Without this fix removeCb called even in case when Item is moved between tiers.
1 parent 7b36c51 commit d817018

1 file changed

Lines changed: 14 additions & 9 deletions

File tree

cachelib/allocator/CacheAllocator-inl.h

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1463,10 +1463,17 @@ CacheAllocator<CacheTrait>::findEviction(TierId tid, PoolId pid, ClassId cid) {
14631463
// for chained items, the ownership of the parent can change. We try to
14641464
// evict what we think as parent and see if the eviction of parent
14651465
// recycles the child we intend to.
1466-
auto toReleaseHandle =
1467-
itr->isChainedItem()
1468-
? advanceIteratorAndTryEvictChainedItem(tid, pid, itr)
1469-
: advanceIteratorAndTryEvictRegularItem(tid, pid, mmContainer, itr);
1466+
1467+
ItemHandle toReleaseHandle = tryEvictToNextMemoryTier(tid, pid, itr);
1468+
bool movedToNextTier = false;
1469+
if(toReleaseHandle) {
1470+
movedToNextTier = true;
1471+
} else {
1472+
toReleaseHandle =
1473+
itr->isChainedItem()
1474+
? advanceIteratorAndTryEvictChainedItem(tid, pid, itr)
1475+
: advanceIteratorAndTryEvictRegularItem(tid, pid, mmContainer, itr);
1476+
}
14701477

14711478
if (toReleaseHandle) {
14721479
if (toReleaseHandle->hasChainedItem()) {
@@ -1497,7 +1504,7 @@ CacheAllocator<CacheTrait>::findEviction(TierId tid, PoolId pid, ClassId cid) {
14971504
// recycle the candidate.
14981505
if (ReleaseRes::kRecycled ==
14991506
releaseBackToAllocator(itemToRelease, RemoveContext::kEviction,
1500-
/* isNascent */ false, candidate)) {
1507+
/* isNascent */ movedToNextTier, candidate)) {
15011508
return candidate;
15021509
}
15031510
}
@@ -1564,6 +1571,7 @@ template <typename ItemPtr>
15641571
typename CacheAllocator<CacheTrait>::ItemHandle
15651572
CacheAllocator<CacheTrait>::tryEvictToNextMemoryTier(
15661573
TierId tid, PoolId pid, ItemPtr& item) {
1574+
if(item->isChainedItem()) return {}; // TODO: We do not support ChainedItem yet
15671575
if(item->isExpired()) return acquire(item);
15681576

15691577
TierId nextTier = tid; // TODO - calculate this based on some admission policy
@@ -1597,9 +1605,6 @@ template <typename CacheTrait>
15971605
typename CacheAllocator<CacheTrait>::ItemHandle
15981606
CacheAllocator<CacheTrait>::advanceIteratorAndTryEvictRegularItem(
15991607
TierId tid, PoolId pid, MMContainer& mmContainer, EvictionIterator& itr) {
1600-
auto evictHandle = tryEvictToNextMemoryTier(tid, pid, itr);
1601-
if(evictHandle) return evictHandle;
1602-
16031608
Item& item = *itr;
16041609

16051610
const bool evictToNvmCache = shouldWriteToNvmCache(item);
@@ -1618,7 +1623,7 @@ CacheAllocator<CacheTrait>::advanceIteratorAndTryEvictRegularItem(
16181623
// if we remove the item from both access containers and mm containers
16191624
// below, we will need a handle to ensure proper cleanup in case we end up
16201625
// not evicting this item
1621-
evictHandle = accessContainer_->removeIf(item, &itemEvictionPredicate);
1626+
auto evictHandle = accessContainer_->removeIf(item, &itemEvictionPredicate);
16221627

16231628
if (!evictHandle) {
16241629
++itr;

0 commit comments

Comments
 (0)