crimson/os/seastore/collection_manager: convert to use interruptible_future

Signed-off-by: Samuel Just <sjust@redhat.com>

src/crimson/os/seastore/collection_manager.cc

@@ -7,7 +7,7 @@
 
 namespace crimson::os::seastore::collection_manager {
 
-CollectionManagerRef create_coll_manager(InterruptedTransactionManager trans_manager) {
+CollectionManagerRef create_coll_manager(TransactionManager &trans_manager) {
   return CollectionManagerRef(new FlatCollectionManager(trans_manager));
 }
 

src/crimson/os/seastore/collection_manager.h

@@ -26,19 +26,17 @@ struct coll_info_t {
 /// Interface for maintaining set of collections
 class CollectionManager {
 public:
-  using base_ertr = with_trans_ertr<
+  using base_iertr = TransactionManager::read_extent_iertr;
-    TransactionManager::read_extent_iertr>;
 
     /// Initialize collection manager instance for an empty store
-  using mkfs_ertr = with_trans_ertr<
+  using mkfs_iertr = TransactionManager::alloc_extent_iertr;
-    TransactionManager::alloc_extent_iertr>;
+  using mkfs_ret = mkfs_iertr::future<coll_root_t>;
-  using mkfs_ret = mkfs_ertr::future<coll_root_t>;
   virtual mkfs_ret mkfs(
     Transaction &t) = 0;
 
   /// Create collection
-  using create_ertr = base_ertr;
+  using create_iertr = base_iertr;
-  using create_ret = create_ertr::future<>;
+  using create_ret = create_iertr::future<>;
   virtual create_ret create(
     coll_root_t &root,
     Transaction &t,
@@ -47,24 +45,24 @@ public:
   ) = 0;
 
   /// List collections with info
-  using list_ertr = base_ertr;
+  using list_iertr = base_iertr;
   using list_ret_bare = std::vector<std::pair<coll_t, coll_info_t>>;
-  using list_ret = list_ertr::future<list_ret_bare>;
+  using list_ret = list_iertr::future<list_ret_bare>;
   virtual list_ret list(
     const coll_root_t &root,
     Transaction &t) = 0;
 
   /// Remove cid
-  using remove_ertr = base_ertr;
+  using remove_iertr = base_iertr;
-  using remove_ret = remove_ertr::future<>;
+  using remove_ret = remove_iertr::future<>;
   virtual remove_ret remove(
     const coll_root_t &coll_root,
     Transaction &t,
     coll_t cid) = 0;
 
   /// Update info for cid
-  using update_ertr = base_ertr;
+  using update_iertr = base_iertr;
-  using update_ret = base_ertr::future<>;
+  using update_ret = base_iertr::future<>;
   virtual update_ret update(
     const coll_root_t &coll_root,
     Transaction &t,
@@ -79,7 +77,7 @@ using CollectionManagerRef = std::unique_ptr<CollectionManager>;
 namespace collection_manager {
 /* creat CollectionMapManager for Collection  */
 CollectionManagerRef create_coll_manager(
-  InterruptedTransactionManager trans_manager);
+  TransactionManager &trans_manager);
 
 }
 

src/crimson/os/seastore/collection_manager/collection_flat_node.cc

@@ -53,7 +53,7 @@ CollectionNode::list()
     list_result.emplace_back(coll, bits);
   }
   return list_ret(
-    list_ertr::ready_future_marker{},
+    interruptible::ready_future_marker{},
     std::move(list_result));
 }
 
@@ -72,7 +72,7 @@ CollectionNode::create(coll_context_t cc, coll_t coll, unsigned bits)
   if (encoded_sizeof((base_coll_map_t&)decoded) > get_bptr().length()) {
     decoded.erase(iter);
     return create_ret(
-      create_ertr::ready_future_marker{},
+      interruptible::ready_future_marker{},
       create_result_t::OVERFLOW);
   } else {
     if (auto buffer = maybe_get_delta_buffer(); buffer) {
@@ -80,7 +80,7 @@ CollectionNode::create(coll_context_t cc, coll_t coll, unsigned bits)
     }
     copy_to_node();
     return create_ret(
-      create_ertr::ready_future_marker{},
+      interruptible::ready_future_marker{},
       create_result_t::SUCCESS);
   }
 }

src/crimson/os/seastore/collection_manager/collection_flat_node.h

@@ -9,7 +9,7 @@
 
 namespace crimson::os::seastore::collection_manager {
 struct coll_context_t {
-  InterruptedTransactionManager tm;
+  TransactionManager &tm;
   Transaction &t;
 };
 
@@ -115,7 +115,7 @@ struct CollectionNode
     return is_mutation_pending() ? &delta_buffer : nullptr;
   }
 
-  using list_ertr = CollectionManager::list_ertr;
+  using list_iertr = CollectionManager::list_iertr;
   using list_ret = CollectionManager::list_ret;
   list_ret list();
 
@@ -124,15 +124,15 @@ struct CollectionNode
     SUCCESS,
     OVERFLOW
   };
-  using create_ertr = CollectionManager::create_ertr;
+  using create_iertr = CollectionManager::create_iertr;
-  using create_ret = create_ertr::future<create_result_t>;
+  using create_ret = create_iertr::future<create_result_t>;
   create_ret create(coll_context_t cc, coll_t coll, unsigned bits);
 
-  using remove_ertr = CollectionManager::remove_ertr;
+  using remove_iertr = CollectionManager::remove_iertr;
   using remove_ret = CollectionManager::remove_ret;
   remove_ret remove(coll_context_t cc, coll_t coll);
 
-  using update_ertr = CollectionManager::update_ertr;
+  using update_iertr = CollectionManager::update_iertr;
   using update_ret = CollectionManager::update_ret;
   update_ret update(coll_context_t cc, coll_t coll, unsigned bits);
 

src/crimson/os/seastore/collection_manager/flat_collection_manager.cc

@@ -22,7 +22,7 @@ constexpr static segment_off_t MIN_FLAT_BLOCK_SIZE = 4<<10;
 [[maybe_unused]] constexpr static segment_off_t MAX_FLAT_BLOCK_SIZE = 4<<20;
 
 FlatCollectionManager::FlatCollectionManager(
-  InterruptedTransactionManager tm)
+  TransactionManager &tm)
   : tm(tm) {}
 
 FlatCollectionManager::mkfs_ret
@@ -32,12 +32,12 @@ FlatCollectionManager::mkfs(Transaction &t)
   logger().debug("FlatCollectionManager: {}", __func__);
   return tm.alloc_extent<CollectionNode>(
     t, L_ADDR_MIN, MIN_FLAT_BLOCK_SIZE
-  ).safe_then([](auto&& root_extent) {
+  ).si_then([](auto&& root_extent) {
     coll_root_t coll_root = coll_root_t(
       root_extent->get_laddr(),
       MIN_FLAT_BLOCK_SIZE
     );
-    return mkfs_ertr::make_ready_future<coll_root_t>(coll_root);
+    return mkfs_iertr::make_ready_future<coll_root_t>(coll_root);
   });
 }
 
@@ -51,8 +51,8 @@ FlatCollectionManager::get_coll_root(const coll_root_t &coll_root, Transaction &
     cc.t,
     coll_root.get_location(),
     coll_root.get_size()
-  ).safe_then([](auto&& e) {
+  ).si_then([](auto&& e) {
-    return get_root_ertr::make_ready_future<CollectionNodeRef>(std::move(e));
+    return get_root_iertr::make_ready_future<CollectionNodeRef>(std::move(e));
   });
 }
 
@@ -62,10 +62,10 @@ FlatCollectionManager::create(coll_root_t &coll_root, Transaction &t,
 {
   logger().debug("FlatCollectionManager: {}", __func__);
   return get_coll_root(coll_root, t
-  ).safe_then([=, &coll_root, &t] (auto &&extent) {
+  ).si_then([=, &coll_root, &t] (auto &&extent) {
     return extent->create(
       get_coll_context(t), cid, info.split_bits
-    ).safe_then([=, &coll_root, &t] (auto ret) {
+    ).si_then([=, &coll_root, &t] (auto ret) {
       switch (ret) {
       case CollectionNode::create_result_t::OVERFLOW: {
         logger().debug("FlatCollectionManager: {} overflow!", __func__);
@@ -76,23 +76,23 @@ FlatCollectionManager::create(coll_root_t &coll_root, Transaction &t,
 	assert(new_size < MAX_FLAT_BLOCK_SIZE);
         return tm.alloc_extent<CollectionNode>(
 	  t, L_ADDR_MIN, new_size
-	).safe_then([=, &coll_root, &t] (auto &&root_extent) {
+	).si_then([=, &coll_root, &t] (auto &&root_extent) {
           coll_root.update(root_extent->get_laddr(), root_extent->get_length());
 
 	  root_extent->decoded = extent->decoded;
 	  root_extent->loaded = true;
 	  return root_extent->create(
 	    get_coll_context(t), cid, info.split_bits
-	  ).safe_then([=, &t](auto result) {
+	  ).si_then([=, &t](auto result) {
 	    assert(result == CollectionNode::create_result_t::SUCCESS);
 	    return tm.dec_ref(t, extent->get_laddr());
-	  }).safe_then([] (auto) {
+	  }).si_then([] (auto) {
-            return create_ertr::make_ready_future<>();
+            return create_iertr::make_ready_future<>();
           });
         });
       }
       case CollectionNode::create_result_t::SUCCESS: {
-        return create_ertr::make_ready_future<>();
+        return create_iertr::make_ready_future<>();
       }
       }
       __builtin_unreachable();
@@ -105,7 +105,7 @@ FlatCollectionManager::list(const coll_root_t &coll_root, Transaction &t)
 {
   logger().debug("FlatCollectionManager: {}", __func__);
   return get_coll_root(coll_root, t)
-    .safe_then([] (auto extent) {
+    .si_then([] (auto extent) {
     return extent->list();
   });
 }
@@ -116,7 +116,7 @@ FlatCollectionManager::update(const coll_root_t &coll_root, Transaction &t,
 {
   logger().debug("FlatCollectionManager: {}", __func__);
   return get_coll_root(coll_root, t)
-    .safe_then([this, &t, cid, info] (auto extent) {
+    .si_then([this, &t, cid, info] (auto extent) {
       return extent->update(get_coll_context(t), cid, info.split_bits);
   });
 }
@@ -126,7 +126,7 @@ FlatCollectionManager::remove(const coll_root_t &coll_root, Transaction &t,
                               coll_t cid )
 {
   logger().debug("FlatCollectionManager: {}", __func__);
-  return get_coll_root(coll_root, t).safe_then([this, &t, cid] (auto extent) {
+  return get_coll_root(coll_root, t).si_then([this, &t, cid] (auto extent) {
     return extent->remove(get_coll_context(t), cid);
   });
 }

src/crimson/os/seastore/collection_manager/flat_collection_manager.h

@@ -13,18 +13,18 @@
 namespace crimson::os::seastore::collection_manager {
 
 class FlatCollectionManager : public CollectionManager {
-  InterruptedTransactionManager tm;
+  TransactionManager &tm;
 
   coll_context_t get_coll_context(Transaction &t) {
     return coll_context_t{tm, t};
   }
 
-  using get_root_ertr = base_ertr;
+  using get_root_iertr = base_iertr;
-  using get_root_ret = get_root_ertr::future<CollectionNodeRef>;
+  using get_root_ret = get_root_iertr::future<CollectionNodeRef>;
   get_root_ret get_coll_root(const coll_root_t &coll_root, Transaction &t);
 
 public:
-  explicit FlatCollectionManager(InterruptedTransactionManager tm);
+  explicit FlatCollectionManager(TransactionManager &tm);
 
   mkfs_ret mkfs(Transaction &t) final;
 

src/crimson/os/seastore/seastore.cc

@@ -87,7 +87,11 @@ seastar::future<> SeaStore::mkfs(uuid_d new_osd_fsid)
       [this](auto &t) {
 	return onode_manager->mkfs(*t
 	).safe_then([this, &t] {
-	  return collection_manager->mkfs(*t);
+	  return with_trans_intr(
+	    *t,
+	    [this](auto &t) {
+	      return collection_manager->mkfs(t);
+	    });
 	}).safe_then([this, &t](auto coll_root) {
 	  transaction_manager->write_collection_root(
 	    *t,
@@ -172,9 +176,13 @@ seastar::future<std::vector<coll_t>> SeaStore::list_collections()
 	  [this, &ret](auto &t) {
 	    return transaction_manager->read_collection_root(*t
 	    ).safe_then([this, &t](auto coll_root) {
-	      return collection_manager->list(
+	      return with_trans_intr(
-		coll_root,
+		*t,
-		*t);
+		[this, &coll_root](auto &t) {
+		  return collection_manager->list(
+		    coll_root,
+		    t);
+		});
 	    }).safe_then([&ret](auto colls) {
 	      ret.resize(colls.size());
 	      std::transform(
@@ -981,18 +989,21 @@ SeaStore::tm_ret SeaStore::_create_collection(
     return seastar::do_with(
       _cmroot,
       [=, &ctx](auto &cmroot) {
-	return collection_manager->create(
+	return with_trans_intr(
-	  cmroot,
 	  *ctx.transaction,
-	  cid,
+	  [=, &cmroot](auto &t) {
-	  bits
+	    return collection_manager->create(
-	).safe_then([=, &ctx, &cmroot] {
+	      cmroot,
-	  if (cmroot.must_update()) {
+	      t,
-	    transaction_manager->write_collection_root(
+	      cid,
-	      *ctx.transaction,
+	      bits);
-	      cmroot);
+	  }).safe_then([=, &ctx, &cmroot] {
-	  }
+	    if (cmroot.must_update()) {
-	});
+	      transaction_manager->write_collection_root(
+		*ctx.transaction,
+		cmroot);
+	    }
+	  });
       });
   }).handle_error(
     tm_ertr::pass_further{},
@@ -1012,18 +1023,21 @@ SeaStore::tm_ret SeaStore::_remove_collection(
     return seastar::do_with(
       _cmroot,
       [=, &ctx](auto &cmroot) {
-	return collection_manager->remove(
+	return with_trans_intr(
-	  cmroot,
 	  *ctx.transaction,
-	  cid
+	  [=, &cmroot](auto &t) {
-	).safe_then([=, &ctx, &cmroot] {
+	    return collection_manager->remove(
-	  // param here denotes whether it already existed, probably error
+	      cmroot,
-	  if (cmroot.must_update()) {
+	      t,
-	    transaction_manager->write_collection_root(
+	      cid);
-	      *ctx.transaction,
+	  }).safe_then([=, &ctx, &cmroot] {
-	      cmroot);
+	    // param here denotes whether it already existed, probably error
-	  }
+	    if (cmroot.must_update()) {
-	});
+	      transaction_manager->write_collection_root(
+		*ctx.transaction,
+		cmroot);
+	    }
+	  });
       });
   }).handle_error(
     tm_ertr::pass_further{},

src/test/crimson/seastore/test_collection_manager.cc

@@ -23,6 +23,21 @@ namespace {
 }
 
 
+#define TEST_COLL_FORWARD(METHOD)					\
+  template <typename... Args>				\
+  auto METHOD(coll_root_t &root, Transaction &t, Args&&... args) const { \
+    return with_trans_intr(						\
+    t,									\
+    [this](auto &t, auto &root, auto&&... args) {			\
+      return collection_manager->METHOD(				\
+        root,								\
+        t,								\
+        std::forward<decltype(args)>(args)...);				\
+      },								\
+      root,								\
+      std::forward<Args>(args)...).unsafe_get0();			\
+  }
+
 struct collection_manager_test_t :
   public seastar_test_suite_t,
   TMTestState {
@@ -55,8 +70,24 @@ struct collection_manager_test_t :
     logger().debug("{}: end", __func__);
   }
 
+  auto get_root() {
+    auto tref = tm->create_transaction();
+    auto coll_root = with_trans_intr(
+      *tref,
+      [this](auto &t) {
+	return collection_manager->mkfs(t);
+      }).unsafe_get0();
+    submit_transaction(std::move(tref));
+    return coll_root;
+  }
+
+  TEST_COLL_FORWARD(remove)
+  TEST_COLL_FORWARD(list)
+  TEST_COLL_FORWARD(create)
+  TEST_COLL_FORWARD(update)
+
   void checking_mappings(coll_root_t &coll_root, Transaction &t) {
-    auto coll_list = collection_manager->list(coll_root, t).unsafe_get0();
+    auto coll_list = list(coll_root, t);
     EXPECT_EQ(test_coll_mappings.size(), coll_list.size());
     for (std::pair<coll_t, coll_info_t> p : test_coll_mappings) {
       EXPECT_NE(
@@ -74,17 +105,12 @@ struct collection_manager_test_t :
 TEST_F(collection_manager_test_t, basic)
 {
   run_async([this] {
-    coll_root_t coll_root;
+    coll_root_t coll_root = get_root();
-    {
-      auto t = tm->create_transaction();
-      coll_root = collection_manager->mkfs(*t).unsafe_get0();
-      submit_transaction(std::move(t));
-    }
     {
       auto t = tm->create_transaction();
       for (int i = 0; i < 20; i++) {
         coll_t cid(spg_t(pg_t(i+1,i+2), shard_id_t::NO_SHARD));
-        collection_manager->create(coll_root, *t, cid, coll_info_t(i)).unsafe_get0();
+        create(coll_root, *t, cid, coll_info_t(i));
         test_coll_mappings.emplace(cid, coll_info_t(i));
       }
       checking_mappings(coll_root, *t);
@@ -97,7 +123,7 @@ TEST_F(collection_manager_test_t, basic)
     {
       auto t = tm->create_transaction();
       for (auto& ite : test_coll_mappings) {
-        collection_manager->remove(coll_root, *t, ite.first).unsafe_get0();
+        remove(coll_root, *t, ite.first);
         test_coll_mappings.erase(ite.first);
       }
       submit_transaction(std::move(t));
@@ -105,7 +131,7 @@ TEST_F(collection_manager_test_t, basic)
     replay();
     {
       auto t = tm->create_transaction();
-      auto list_ret = collection_manager->list(coll_root, *t).unsafe_get0();
+      auto list_ret = list(coll_root, *t);
       submit_transaction(std::move(t));
       EXPECT_EQ(list_ret.size(), test_coll_mappings.size());
     }
@@ -115,18 +141,13 @@ TEST_F(collection_manager_test_t, basic)
 TEST_F(collection_manager_test_t, overflow)
 {
   run_async([this] {
-    coll_root_t coll_root;
+    coll_root_t coll_root = get_root();
-    {
-      auto t = tm->create_transaction();
-      coll_root = collection_manager->mkfs(*t).unsafe_get0();
-      submit_transaction(std::move(t));
-    }
     auto old_location = coll_root.get_location();
 
     auto t = tm->create_transaction();
     for (int i = 0; i < 412; i++) {
       coll_t cid(spg_t(pg_t(i+1,i+2), shard_id_t::NO_SHARD));
-      collection_manager->create(coll_root, *t, cid, coll_info_t(i)).unsafe_get0();
+      create(coll_root, *t, cid, coll_info_t(i));
       test_coll_mappings.emplace(cid, coll_info_t(i));
     }
     submit_transaction(std::move(t));
@@ -141,17 +162,12 @@ TEST_F(collection_manager_test_t, overflow)
 TEST_F(collection_manager_test_t, update)
 {
   run_async([this] {
-    coll_root_t coll_root;
+    coll_root_t coll_root = get_root();
-    {
-      auto t = tm->create_transaction();
-      coll_root = collection_manager->mkfs(*t).unsafe_get0();
-      submit_transaction(std::move(t));
-    }
     {
       auto t = tm->create_transaction();
       for (int i = 0; i < 2; i++) {
         coll_t cid(spg_t(pg_t(1,i+1), shard_id_t::NO_SHARD));
-        collection_manager->create(coll_root, *t, cid, coll_info_t(i)).unsafe_get0();
+	create(coll_root, *t, cid, coll_info_t(i));
         test_coll_mappings.emplace(cid, coll_info_t(i));
       }
       submit_transaction(std::move(t));
@@ -161,7 +177,7 @@ TEST_F(collection_manager_test_t, update)
        auto iter2 = std::next(test_coll_mappings.begin(), 1);
        EXPECT_NE(iter1->second.split_bits, iter2->second.split_bits);
        auto t = tm->create_transaction();
-       collection_manager->update(coll_root, *t, iter1->first, iter2->second).unsafe_get0();
+       update(coll_root, *t, iter1->first, iter2->second);
        submit_transaction(std::move(t));
        iter1->second.split_bits = iter2->second.split_bits;
     }