correct handling for expired items in eviction

- we first check if an item is expired under mmContainer
  lock and if so mark it for eviction so it is recycled
  back up to allocateInternalTier.

Author: byrnedj

cachelib/allocator/CacheAllocator-inl.h

@@ -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();
@@ -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
@@ -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
@@ -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);
@@ -1805,13 +1808,9 @@ typename CacheAllocator<CacheTrait>::WriteHandle
 CacheAllocator<CacheTrait>::tryEvictToNextMemoryTier(
     TierId tid, PoolId pid, Item& item, bool fromBgThread) {
   XDCHECK(item.isMoving());
+  XDCHECK(!item.isExpired());
   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