Chained item movement between tiers - sync on the parent item (#84)

* Chained item movement between tiers - currently sync on the parent
item for moving.
 - updated tests accordingly, note that we can no longer swap
   parent item if chained item is being moved for slab release.

* added some debug checks around chained item check
* fix slab release behavior if no movecb

Author: byrnedj

cachelib/allocator/CacheAllocator-inl.h

@@ -1565,6 +1565,26 @@ class CacheAllocator : public CacheBase {
   //            if the item is invalid
   WriteHandle allocateChainedItemInternal(const Item& parent, uint32_t size);
 
+  // Allocate a chained item to a specific tier
+  //
+  // The resulting chained item does not have a parent item yet
+  // and if we fail to link to the chain for any reasoin
+  // the chained item will be freed once the handle is dropped.
+  //
+  // The parent item parameter here is mainly used to find the
+  // correct pool to allocate memory for this chained item
+  //
+  // @param parent    parent item
+  // @param size      the size for the chained allocation
+  // @param tid       the tier to allocate on
+  //
+  // @return    handle to the chained allocation
+  // @throw     std::invalid_argument if the size requested is invalid or
+  //            if the item is invalid
+  WriteHandle allocateChainedItemInternalTier(const Item& parent,
+                                              uint32_t size,
+                                              TierId tid);
+
   // Given an existing item, allocate a new one for the
   // existing one to later be moved into.
   //
@@ -1669,9 +1689,8 @@ class CacheAllocator : public CacheBase {
   // will be unmarked as having chained allocations. Parent will not be null
   // after calling this API.
   //
-  // Parent and NewParent must be valid handles to items with same key and
-  // parent must have chained items and parent handle must be the only
-  // outstanding handle for parent. New parent must be without any chained item
+  // NewParent must be valid handles to item with same key as Parent and
+  // Parent must have chained items. New parent must be without any chained item
   // handles.
   //
   // Chained item lock for the parent's key needs to be held in exclusive mode.

cachelib/allocator/CacheAllocator.h

@@ -4917,7 +4917,7 @@ class BaseAllocatorTest : public AllocatorTest<AllocatorT> {
 
       std::memcpy(newItem.getMemory(), oldItem.getMemory(), oldItem.getSize());
       ++numMoves;
-    });
+    }, {}, 1000000 /* lots of moving tries */);
 
     AllocatorT alloc(config);
     const size_t numBytes = alloc.getCacheMemoryStats().ramCacheSize;
@@ -4958,15 +4958,15 @@ class BaseAllocatorTest : public AllocatorTest<AllocatorT> {
         }
 
         /* sleep override */
-        std::this_thread::sleep_for(std::chrono::milliseconds(100));
+        std::this_thread::sleep_for(std::chrono::milliseconds(1000));
       }
     };
 
     // Release 5 slabs
     auto releaseFn = [&] {
       for (unsigned int i = 0; i < 5;) {
         /* sleep override */
-        std::this_thread::sleep_for(std::chrono::milliseconds(100));
+        std::this_thread::sleep_for(std::chrono::milliseconds(1000));
 
         ClassId cid = static_cast<ClassId>(i);
         alloc.releaseSlab(pid, cid, SlabReleaseMode::kRebalance);
@@ -5126,7 +5126,7 @@ class BaseAllocatorTest : public AllocatorTest<AllocatorT> {
     auto releaseFn = [&] {
       for (unsigned int i = 0; i < 5;) {
         /* sleep override */
-        std::this_thread::sleep_for(std::chrono::milliseconds(100));
+        std::this_thread::sleep_for(std::chrono::milliseconds(1000));
 
         ClassId cid = static_cast<ClassId>(i);
         alloc.releaseSlab(pid, cid, SlabReleaseMode::kRebalance);
@@ -5969,7 +5969,6 @@ class BaseAllocatorTest : public AllocatorTest<AllocatorT> {
     EXPECT_EQ(nullptr,
               util::allocateAccessible(alloc, poolId, "large", largeSize));
 
-    std::this_thread::sleep_for(std::chrono::seconds{1});
     // trigger the slab rebalance
     EXPECT_EQ(nullptr,
               util::allocateAccessible(alloc, poolId, "large", largeSize));

cachelib/allocator/tests/BaseAllocatorTest.h

@@ -214,6 +214,9 @@ class RebalanceStrategyTest : public testing::Test {
     config.poolRebalancerFreeAllocThreshold = 20;
 
     initAllocatorConfigForStrategy(config, LruTailAge);
+    //TODO: why does this fail with orig. value of 8?
+    //on upstream this fails too, it always reports 4 instead
+    //of the original test value, which is 8 expected slabs
     doWork(config, true, 8);
   }
 

cachelib/allocator/tests/RebalanceStrategyTest.cpp

@@ -209,16 +209,6 @@ void RefCountTest::testMarkForEvictionAndMoving() {
     ASSERT_EQ(ret, 0);
   }
 
-  {
-    // cannot mark moving when ref count > 0
-    RefcountWithFlags ref;
-    ref.markInMMContainer();
-
-    ref.incRef();
-
-    ASSERT_FALSE(ref.markMoving());
-  }
-
   {
     // cannot mark for eviction when ref count > 0
     RefcountWithFlags ref;

cachelib/allocator/tests/RefCountTest.cpp

@@ -104,7 +104,7 @@ class SimpleRebalanceTest : public testing::Test {
 
     // Sleep for 2 seconds to let the rebalancing work
     /* sleep override */
-    std::this_thread::sleep_for(std::chrono::seconds(3));
+    std::this_thread::sleep_for(std::chrono::seconds(10));
 
     // Evicted keys shouldn't be in the allocator anymore
     ASSERT_FALSE(evictedKeys.empty());

cachelib/allocator/tests/SimpleRebalancingTest.h

@@ -77,7 +77,7 @@ class CacheStressor : public Stressor {
         std::unique_lock<folly::SharedMutex> lock;
 
         CacheStressSyncObj(CacheStressor& s, std::string itemKey)
-            : lock{s.chainedItemAcquireUniqueLock(itemKey)} {}
+            : lock{s.chainedItemTryAcquireUniqueLock(itemKey)} {}
       };
       movingSync = [this](typename CacheT::Item::Key key) {
         return std::make_unique<CacheStressSyncObj>(*this, key.str());
@@ -247,6 +247,10 @@ class CacheStressor : public Stressor {
     using Lock = std::unique_lock<folly::SharedMutex>;
     return lockEnabled_ ? Lock{getLock(key)} : Lock{};
   }
+  auto chainedItemTryAcquireUniqueLock(Key key) {
+    using Lock = std::unique_lock<folly::SharedMutex>;
+    return lockEnabled_ ? Lock{getLock(key), std::try_to_lock} : Lock{};
+  }
 
   // populate the input item handle according to the stress setup.
   void populateItem(WriteHandle& handle, const std::string& itemValue = "") {

cachelib/cachebench/runner/CacheStressor.h

@@ -0,0 +1,35 @@
+// @nolint like default.json, but moves items during slab release instead of evicting them.
+{
+    "cache_config" : {
+      "cacheSizeMB" : 2248,
+      "cacheDir": "/tmp/mem-tier5",
+      "memoryTiers" : [
+        {
+          "ratio": 1,
+          "memBindNodes": 0
+        }, {
+          "ratio": 1,
+          "memBindNodes": 0
+        }
+      ],
+      "poolRebalanceIntervalSec" : 1,
+      "moveOnSlabRelease" : true,
+      "rebalanceMinSlabs" : 2,
+      "evictorThreads": 2,
+      "promoterThreads": 2
+    },
+    "test_config" :
+      {
+        "preallocateCache" : true,
+        "numOps" : 20000000,
+        "numThreads" : 32,
+        "numKeys" : 250000,
+        "generator": "online",
+        "keySizeRange" : [1, 8, 32, 64, 128, 256, 512],
+        "keySizeRangeProbability" : [0.1, 0.1, 0.2, 0.2, 0.3, 0.1],
+        "valSizeRange" : [1, 128, 512, 1024, 4096, 10240, 20480, 40960, 60000],
+        "valSizeRangeProbability" : [0.1, 0.1, 0.1, 0.2, 0.2, 0.1, 0.1, 0.1],
+        "getRatio" : 0.70,
+        "setRatio" : 0.30
+      }
+  }

cachelib/cachebench/test_configs/small_moving_bg.json

@@ -13,3 +13,4 @@ fi
 
 ../bin/cachebench --json_test_config ../test_configs/consistency/navy.json
 ../bin/cachebench --json_test_config ../test_configs/consistency/navy-multi-tier.json
+../bin/cachebench --json_test_config ../test_configs/small_moving_bg.json