Have FlashCacheComponent own the navy::Device it uses

Summary: Make FlashCacheComponent own the device it's using for cache.  Otherwise, tests/cachebench/users will all create the same wrapper class that holds on to a device & cache instance to ensure the device is alive while the cache is around.

Reviewed By: pbhandar2

Differential Revision: D92296967

fbshipit-source-id: b7ab64d697750ed5cc8a9603b288c536375d8235

Author: Rob Lyerly

cachelib/interface/components/FlashCacheComponent.cpp

@@ -127,9 +127,12 @@ class FlashCacheItem : public CacheItem {
 // ============================================================================
 
 /* static */ Result<FlashCacheComponent> FlashCacheComponent::create(
-    std::string name, navy::BlockCache::Config&& config) noexcept {
+    std::string name,
+    navy::BlockCache::Config&& config,
+    std::unique_ptr<navy::Device> device) noexcept {
   try {
-    return FlashCacheComponent(std::move(name), std::move(config));
+    return FlashCacheComponent(std::move(name), std::move(config),
+                               std::move(device));
   } catch (const std::invalid_argument& ia) {
     return makeError(Error::Code::INVALID_CONFIG, ia.what());
   }
@@ -349,9 +352,12 @@ folly::coro::Task<UnitResult> FlashCacheComponent::remove(ReadHandle&& handle) {
 }
 
 FlashCacheComponent::FlashCacheComponent(std::string&& name,
-                                         navy::BlockCache::Config&& config)
-    : name_(std::move(name)),
-      cache_(std::make_unique<navy::BlockCache>(std::move(config))) {}
+                                         navy::BlockCache::Config&& config,
+                                         std::unique_ptr<Device> device)
+    : name_(std::move(name)), device_(std::move(device)) {
+  config.device = device_.get();
+  cache_ = std::make_unique<navy::BlockCache>(std::move(config));
+}
 
 bool FlashCacheComponent::writeBackImpl(CacheItem& item, bool allowReplace) {
   auto& fccItem = static_cast<FlashCacheItem&>(item);
@@ -425,11 +431,13 @@ class ConsistentFlashCacheItem : public FlashCacheItem {
 /* static */ Result<ConsistentFlashCacheComponent>
 ConsistentFlashCacheComponent::create(std::string name,
                                       navy::BlockCache::Config&& config,
+                                      std::unique_ptr<Device> device,
                                       std::unique_ptr<Hash> hasher,
                                       uint8_t shardsPower) noexcept {
   try {
     return ConsistentFlashCacheComponent(std::move(name), std::move(config),
-                                         std::move(hasher), shardsPower);
+                                         std::move(device), std::move(hasher),
+                                         shardsPower);
   } catch (const std::invalid_argument& ia) {
     return makeError(Error::Code::INVALID_CONFIG, ia.what());
   }
@@ -496,9 +504,11 @@ folly::coro::Task<UnitResult> ConsistentFlashCacheComponent::remove(
 ConsistentFlashCacheComponent::ConsistentFlashCacheComponent(
     std::string&& name,
     navy::BlockCache::Config&& config,
+    std::unique_ptr<Device> device,
     std::unique_ptr<Hash> hasher,
     uint8_t shardsPower)
-    : FlashCacheComponent(std::move(name), std::move(config)),
+    : FlashCacheComponent(
+          std::move(name), std::move(config), std::move(device)),
       serializer_(std::move(hasher), shardsPower) {}
 
 } // namespace facebook::cachelib::interface

cachelib/interface/components/FlashCacheComponent.h

@@ -43,11 +43,14 @@ class FlashCacheComponent : public CacheComponent {
    *
    * @param name name of the flash cache
    * @param config the BlockCache::Config to use for the cache
+   * @param device the device to use for the cache
    * @return FlashCacheComponent if the flash cache could be initialized, an
    * error otherwise.
    */
   static Result<FlashCacheComponent> create(
-      std::string name, navy::BlockCache::Config&& config) noexcept;
+      std::string name,
+      navy::BlockCache::Config&& config,
+      std::unique_ptr<navy::Device> device) noexcept;
 
   // ------------------------------ Interface ------------------------------ //
 
@@ -64,7 +67,9 @@ class FlashCacheComponent : public CacheComponent {
   folly::coro::Task<UnitResult> remove(ReadHandle&& handle) override;
 
  protected:
-  FlashCacheComponent(std::string&& name, navy::BlockCache::Config&& config);
+  FlashCacheComponent(std::string&& name,
+                      navy::BlockCache::Config&& config,
+                      std::unique_ptr<navy::Device> device);
 
   // Helpers in which the returned cache item is templatized so we can share the
   // implementation with child classes
@@ -77,6 +82,8 @@ class FlashCacheComponent : public CacheComponent {
 
  private:
   std::string name_;
+  // Note: device_ must be declared before cache_ so that it outlives it
+  std::unique_ptr<navy::Device> device_;
   std::unique_ptr<navy::BlockCache> cache_;
 
   // Runs func() on a RegionManager worker fiber. Should not be called from an
@@ -142,6 +149,7 @@ class ConsistentFlashCacheComponent : public FlashCacheComponent {
    *
    * @param name name of the flash cache
    * @param config the BlockCache::Config to use for the cache
+   * @param device the device to use for the cache
    * @param hasher the hasher to use for sharding
    * @param shardsPower the number of shards to use (2^shardsPower) when
    * serializing operations, max is ShardedSerializer::kMaxShardsPower
@@ -151,6 +159,7 @@ class ConsistentFlashCacheComponent : public FlashCacheComponent {
   static Result<ConsistentFlashCacheComponent> create(
       std::string name,
       navy::BlockCache::Config&& config,
+      std::unique_ptr<navy::Device> device,
       std::unique_ptr<Hash> hasher,
       uint8_t shardsPower) noexcept;
 
@@ -195,6 +204,7 @@ class ConsistentFlashCacheComponent : public FlashCacheComponent {
 
   ConsistentFlashCacheComponent(std::string&& name,
                                 navy::BlockCache::Config&& config,
+                                std::unique_ptr<navy::Device> device,
                                 std::unique_ptr<Hash> hasher,
                                 uint8_t shardsPower);
 

cachelib/interface/components/tests/CacheComponentTest.cpp

@@ -84,25 +84,21 @@ class FlashCacheFactory : public CacheFactory {
       : hits_(/* count */ kDeviceSize / kRegionSize, /* value */ 0) {}
 
   std::unique_ptr<CacheComponent> create() override {
-    device_ = makeDevice();
-    auto flashCache = ASSERT_OK(
-        FlashCacheComponent::create("CacheComponentTest", makeConfig()));
+    auto flashCache = ASSERT_OK(FlashCacheComponent::create(
+        "CacheComponentTest", makeConfig(), makeDevice()));
     return std::make_unique<FlashCacheComponent>(std::move(flashCache));
   }
 
  protected:
   using BlockCache = navy::BlockCache;
 
-  std::unique_ptr<navy::Device> device_;
-
   std::unique_ptr<navy::Device> makeDevice() {
     return navy::createMemoryDevice(kDeviceSize, /* encryptor */ nullptr);
   }
   BlockCache::Config makeConfig() {
     BlockCache::Config config;
     config.regionSize = kRegionSize;
     config.cacheSize = kDeviceSize;
-    config.device = device_.get();
     config.evictionPolicy =
         std::make_unique<NiceMock<navy::MockPolicy>>(&hits_);
     return config;
@@ -118,10 +114,9 @@ class FlashCacheFactory : public CacheFactory {
 class ConsistentFlashCacheFactory : public FlashCacheFactory {
  public:
   std::unique_ptr<CacheComponent> create() override {
-    device_ = makeDevice();
     auto consistentFlashCache = ASSERT_OK(ConsistentFlashCacheComponent::create(
-        "CacheComponentTest", makeConfig(), std::make_unique<MurmurHash2>(),
-        kShardsPower));
+        "CacheComponentTest", makeConfig(), makeDevice(),
+        std::make_unique<MurmurHash2>(), kShardsPower));
     return std::make_unique<ConsistentFlashCacheComponent>(
         std::move(consistentFlashCache));
   }