test/crimson/seastore: clean up transaction_manager based tests

Permits using either vanilla TransactionManager and
InterruptedTransactionManager, updates users to use submit_transaction
helpers.

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

diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h
index 38796b720cbd22031b81ec646c3baad1aff0e2c5..b1675ecac79c729d41a76c094566fc2dd61d5ba9 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.h
+++ b/src/crimson/os/seastore/transaction_manager.h
@@ -573,13 +573,13 @@ using TransactionManagerRef = std::unique_ptr<TransactionManager>;
 #define FORWARD(METHOD)                                        \
   template <typename... Args>                          \
   auto METHOD(Args&&... args) const {                  \
-    return tm.METHOD(std::forward<Args>(args)...);     \
+    return tm->METHOD(std::forward<Args>(args)...);    \
   }
 
 #define PARAM_FORWARD(METHOD)                                  \
   template <typename T, typename... Args>                      \
   auto METHOD(Args&&... args) const {                          \
-    return tm.METHOD<T>(std::forward<Args>(args)...);  \
+    return tm->METHOD<T>(std::forward<Args>(args)...); \
   }
 
 #define INT_FORWARD(METHOD)                                            \
@@ -588,7 +588,7 @@ using TransactionManagerRef = std::unique_ptr<TransactionManager>;
     return with_trans_intr(                                            \
       t,                                                               \
       [this](auto&&... args) {                                         \
-       return tm.METHOD(std::forward<decltype(args)>(args)...);        \
+       return tm->METHOD(std::forward<decltype(args)>(args)...);       \
       },                                                               \
       std::forward<Args>(args)...);                                    \
   }
@@ -599,18 +599,24 @@ using TransactionManagerRef = std::unique_ptr<TransactionManager>;
     return with_trans_intr(                                            \
       t,                                                               \
       [this](auto&&... args) {                                         \
-       return tm.METHOD<T>(std::forward<decltype(args)>(args)...);     \
+       return tm->METHOD<T>(std::forward<decltype(args)>(args)...);    \
       },                                                               \
       std::forward<Args>(args)...);                                    \
   }
 
 /// Temporary translator to non-interruptible futures
 class InterruptedTransactionManager {
-  TransactionManager &tm;
+  TransactionManager *tm = nullptr;
 public:
+  InterruptedTransactionManager() = default;
   InterruptedTransactionManager(const InterruptedTransactionManager &) = default;
   InterruptedTransactionManager(InterruptedTransactionManager &&) = default;
-  InterruptedTransactionManager(TransactionManager &tm) : tm(tm) {}
+  InterruptedTransactionManager(TransactionManager &tm) : tm(&tm) {}
+
+  InterruptedTransactionManager &operator=(
+    const InterruptedTransactionManager &) = default;
+  InterruptedTransactionManager &operator=(
+    InterruptedTransactionManager &&) = default;
 
   FORWARD(mkfs)
   FORWARD(mount)
@@ -642,6 +648,8 @@ public:
 
   FORWARD(get_segment_cleaner)
   FORWARD(get_lba_manager)
+
+  void reset() { tm = nullptr; }
 };
 
 class InterruptedTMRef {

diff --git a/src/test/crimson/seastore/onode_tree/test_fltree_onode_manager.cc b/src/test/crimson/seastore/onode_tree/test_fltree_onode_manager.cc
index 362edac7e17d959af1cc5168e3e44835438eb397..bdbdd3d1845009c34d648dab87ca96b0376e36db 100644 (file)
--- a/src/test/crimson/seastore/onode_tree/test_fltree_onode_manager.cc
+++ b/src/test/crimson/seastore/onode_tree/test_fltree_onode_manager.cc
@@ -71,7 +71,7 @@ struct fltree_onode_manager_test_t
 
   virtual void _init() final {
     TMTestState::_init();
-    manager.reset(new FLTreeOnodeManager(*tm));
+    manager.reset(new FLTreeOnodeManager(itm));
   }
 
   virtual void _destroy() final {
@@ -89,7 +89,7 @@ struct fltree_onode_manager_test_t
          [this](auto &t) {
            return manager->mkfs(*t
            ).safe_then([this, &t] {
-             return tm->submit_transaction(*t);
+             return submit_transaction_fut(*t);
            });
          });
       }).safe_then([this] {

diff --git a/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc b/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc
index 0792e9575c64050ac88bfdae538bfdf03f141cdd..79ce42aed768714cba57cefccf533a59ac38a8af 100644 (file)
--- a/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc
+++ b/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc
@@ -1528,7 +1528,7 @@ TEST_F(d_seastore_tm_test_t, 6_random_tree_insert_erase)
         {8, 11,  64, 256, 301, 320},
         {8, 16, 128, 512, 576, 640},
         {0, 16}, {0, 10}, {0, 4});
-    auto moved_nm = (TEST_SEASTORE ? NodeExtentManager::create_seastore(*tm)
+    auto moved_nm = (TEST_SEASTORE ? NodeExtentManager::create_seastore(itm)
                                    : NodeExtentManager::create_dummy(IS_DUMMY_SYNC));
     auto p_nm = moved_nm.get();
     auto tree = std::make_unique<TreeBuilder<TRACK_CURSORS, BoundedValue>>(
@@ -1554,7 +1554,7 @@ TEST_F(d_seastore_tm_test_t, 6_random_tree_insert_erase)
     if constexpr (TEST_SEASTORE) {
       logger().info("seastore replay insert begin");
       restart();
-      tree->reload(NodeExtentManager::create_seastore(*tm));
+      tree->reload(NodeExtentManager::create_seastore(itm));
       logger().info("seastore replay insert end");
     }
     {
@@ -1577,7 +1577,7 @@ TEST_F(d_seastore_tm_test_t, 6_random_tree_insert_erase)
     if constexpr (TEST_SEASTORE) {
       logger().info("seastore replay erase-1 begin");
       restart();
-      tree->reload(NodeExtentManager::create_seastore(*tm));
+      tree->reload(NodeExtentManager::create_seastore(itm));
       logger().info("seastore replay erase-1 end");
     }
     {
@@ -1599,7 +1599,7 @@ TEST_F(d_seastore_tm_test_t, 6_random_tree_insert_erase)
     if constexpr (TEST_SEASTORE) {
       logger().info("seastore replay erase-2 begin");
       restart();
-      tree->reload(NodeExtentManager::create_seastore(*tm));
+      tree->reload(NodeExtentManager::create_seastore(itm));
       logger().info("seastore replay erase-2 end");
     }
     {
@@ -1627,7 +1627,7 @@ TEST_F(d_seastore_tm_test_t, 7_tree_insert_erase_eagain)
         {8, 16, 128, 576,  992, 1200},
         {0, 8}, {0, 10}, {0, 4});
     auto moved_nm = NodeExtentManager::create_seastore(
-        *tm, L_ADDR_MIN, EAGAIN_PROBABILITY);
+        itm, L_ADDR_MIN, EAGAIN_PROBABILITY);
     auto p_nm = static_cast<SeastoreNodeExtentManager<true>*>(moved_nm.get());
     auto tree = std::make_unique<TreeBuilder<TRACK_CURSORS, ExtendedValue>>(
         kvs, std::move(moved_nm));
@@ -1643,7 +1643,7 @@ TEST_F(d_seastore_tm_test_t, 7_tree_insert_erase_eagain)
        [this, &tree](auto &t) {
          return tree->bootstrap(*t
          ).safe_then([this, &t] {
-           return tm->submit_transaction(*t);
+           return submit_transaction_fut(*t);
          });
        });
     }).unsafe_get0();
@@ -1663,7 +1663,7 @@ TEST_F(d_seastore_tm_test_t, 7_tree_insert_erase_eagain)
              return tree->insert_one(*t, iter
              ).safe_then([this, &t](auto cursor) {
                cursor.invalidate();
-               return tm->submit_transaction(*t);
+               return submit_transaction_fut(*t);
              });
            });
         }).unsafe_get0();
@@ -1709,7 +1709,7 @@ TEST_F(d_seastore_tm_test_t, 7_tree_insert_erase_eagain)
            [this, &tree, &iter](auto &t) {
              return tree->erase_one(*t, iter
              ).safe_then([this, &t] () mutable {
-               return tm->submit_transaction(*t);
+               return submit_transaction_fut(*t);
              });
            });
         }).unsafe_get0();

diff --git a/src/test/crimson/seastore/test_object_data_handler.cc b/src/test/crimson/seastore/test_object_data_handler.cc
index c38ed6ccf3c4f0fe70c43a0f1a687992568baf9e..95770a3c5c2cff9b876daca7dd7f01e11189bc10 100644 (file)
--- a/src/test/crimson/seastore/test_object_data_handler.cc
+++ b/src/test/crimson/seastore/test_object_data_handler.cc
@@ -43,13 +43,6 @@ struct object_data_handler_test_t:
 
   object_data_handler_test_t() {}
 
-  auto submit_transaction(TransactionRef &&t) {
-    return tm->submit_transaction(*t
-    ).safe_then([this] {
-      return segment_cleaner->run_until_halt();
-    });
-  }
-
   void write(Transaction &t, objaddr_t offset, extent_len_t len, char fill) {
     ceph_assert(offset + len <= known_contents.length());
     size = std::max<extent_len_t>(size, offset + len);
@@ -65,7 +58,7 @@ struct object_data_handler_test_t:
        len));
     return ObjectDataHandler().write(
       ObjectDataHandler::context_t{
-       *tm,
+       itm,
        t,
        *onode,
       },
@@ -75,7 +68,7 @@ struct object_data_handler_test_t:
   void write(objaddr_t offset, extent_len_t len, char fill) {
     auto t = tm->create_transaction();
     write(*t, offset, len, fill);
-    return submit_transaction(std::move(t)).unsafe_get0();
+    return submit_transaction(std::move(t));
   }
 
   void truncate(Transaction &t, objaddr_t offset) {
@@ -86,7 +79,7 @@ struct object_data_handler_test_t:
        size - offset);
       ObjectDataHandler().truncate(
        ObjectDataHandler::context_t{
-         *tm,
+         itm,
          t,
          *onode
        },
@@ -97,13 +90,13 @@ struct object_data_handler_test_t:
   void truncate(objaddr_t offset) {
     auto t = tm->create_transaction();
     truncate(*t, offset);
-    return submit_transaction(std::move(t)).unsafe_get0();
+    return submit_transaction(std::move(t));
   }
 
   void read(Transaction &t, objaddr_t offset, extent_len_t len) {
     bufferlist bl = ObjectDataHandler().read(
       ObjectDataHandler::context_t{
-       *tm,
+       itm,
        t,
        *onode
       },

diff --git a/src/test/crimson/seastore/test_omap_manager.cc b/src/test/crimson/seastore/test_omap_manager.cc
index 570b8b1b816a360be2d30370bd844d8dcf41ac40..0363890b237dcd29bf666ac6b9afaf52187ab50c 100644 (file)
--- a/src/test/crimson/seastore/test_omap_manager.cc
+++ b/src/test/crimson/seastore/test_omap_manager.cc
@@ -55,7 +55,7 @@ struct omap_manager_test_t :
 
   seastar::future<> set_up_fut() final {
     return tm_setup().then([this] {
-      omap_manager = omap_manager::create_omap_manager(*tm);
+      omap_manager = omap_manager::create_omap_manager(itm);
       return seastar::now();
     });
   }
@@ -190,7 +190,7 @@ struct omap_manager_test_t :
   void replay() {
     logger().debug("{}: begin", __func__);
     restart();
-    omap_manager = omap_manager::create_omap_manager(*tm);
+    omap_manager = omap_manager::create_omap_manager(itm);
     logger().debug("{}: end", __func__);
   }
 };

diff --git a/src/test/crimson/seastore/test_transaction_manager.cc b/src/test/crimson/seastore/test_transaction_manager.cc
index 5ee55acd15c8d3a7c8a0f7697d55c8255af2af1b..0325dfc32d73715ba79199e6ffc9a090b4d14dae 100644 (file)
--- a/src/test/crimson/seastore/test_transaction_manager.cc
+++ b/src/test/crimson/seastore/test_transaction_manager.cc
@@ -351,11 +351,11 @@ struct transaction_manager_test_t :
   };
 
   test_transaction_t create_transaction() {
-    return { tm->create_transaction(), {} };
+    return { itm.create_transaction(), {} };
   }
 
   test_transaction_t create_weak_transaction() {
-    return { tm->create_weak_transaction(), {} };
+    return { itm.create_weak_transaction(), {} };
   }
 
   TestBlockRef alloc_extent(
@@ -363,7 +363,7 @@ struct transaction_manager_test_t :
     laddr_t hint,
     extent_len_t len,
     char contents) {
-    auto extent = tm->alloc_extent<TestBlock>(
+    auto extent = itm.alloc_extent<TestBlock>(
       *(t.t),
       hint,
       len).unsafe_get0();
@@ -427,7 +427,7 @@ struct transaction_manager_test_t :
     ceph_assert(test_mappings.contains(addr, t.mapping_delta));
     ceph_assert(test_mappings.get(addr, t.mapping_delta).desc.len == len);
 
-    auto ext = tm->read_extent<TestBlock>(
+    auto ext = itm.read_extent<TestBlock>(
       *t.t, addr, len
     ).unsafe_get0();
     EXPECT_EQ(addr, ext->get_laddr());
@@ -443,7 +443,7 @@ struct transaction_manager_test_t :
 
     using ertr = with_trans_ertr<TransactionManager::read_extent_iertr>;
     using ret = ertr::future<TestBlockRef>;
-    auto ext = tm->read_extent<TestBlock>(
+    auto ext = itm.read_extent<TestBlock>(
       *t.t, addr, len
     ).safe_then([](auto ext) -> ret {
       return ertr::make_ready_future<TestBlockRef>(ext);
@@ -470,7 +470,7 @@ struct transaction_manager_test_t :
       test_mappings.get(ref->get_laddr(), t.mapping_delta).desc.len ==
       ref->get_length());
 
-    auto ext = tm->get_mutable_extent(*t.t, ref)->cast<TestBlock>();
+    auto ext = itm.get_mutable_extent(*t.t, ref)->cast<TestBlock>();
     EXPECT_EQ(ext->get_laddr(), ref->get_laddr());
     EXPECT_EQ(ext->get_desc(), ref->get_desc());
     mutator.mutate(*ext, gen);
@@ -492,7 +492,7 @@ struct transaction_manager_test_t :
     ceph_assert(test_mappings.contains(offset, t.mapping_delta));
     ceph_assert(test_mappings.get(offset, t.mapping_delta).refcount > 0);
 
-    auto refcnt = tm->inc_ref(*t.t, offset).unsafe_get0();
+    auto refcnt = itm.inc_ref(*t.t, offset).unsafe_get0();
     auto check_refcnt = test_mappings.inc_ref(offset, t.mapping_delta);
     EXPECT_EQ(refcnt, check_refcnt);
   }
@@ -501,7 +501,7 @@ struct transaction_manager_test_t :
     ceph_assert(test_mappings.contains(offset, t.mapping_delta));
     ceph_assert(test_mappings.get(offset, t.mapping_delta).refcount > 0);
 
-    auto refcnt = tm->dec_ref(*t.t, offset).unsafe_get0();
+    auto refcnt = itm.dec_ref(*t.t, offset).unsafe_get0();
     auto check_refcnt = test_mappings.dec_ref(offset, t.mapping_delta);
     EXPECT_EQ(refcnt, check_refcnt);
     if (refcnt == 0)
@@ -536,7 +536,7 @@ struct transaction_manager_test_t :
   bool try_submit_transaction(test_transaction_t t) {
     using ertr = with_trans_ertr<TransactionManager::submit_transaction_iertr>;
     using ret = ertr::future<bool>;
-    bool success = tm->submit_transaction(*t.t
+    bool success = itm.submit_transaction(*t.t
     ).safe_then([]() -> ret {
       return ertr::make_ready_future<bool>(true);
     }).handle_error(

diff --git a/src/test/crimson/seastore/transaction_manager_test_state.h b/src/test/crimson/seastore/transaction_manager_test_state.h
index f839939ed9c5f58c64c3f6bce86017716256c505..22c9fa738e9a35e2149346062eb0ec1bc242a810 100644 (file)
--- a/src/test/crimson/seastore/transaction_manager_test_state.h
+++ b/src/test/crimson/seastore/transaction_manager_test_state.h
@@ -100,7 +100,8 @@ auto get_seastore(SegmentManagerRef sm) {
 
 class TMTestState : public EphemeralTestState {
 protected:
-  InterruptedTMRef tm;
+  TransactionManagerRef tm;
+  InterruptedTransactionManager itm;
   LBAManager *lba_manager;
   SegmentCleaner *segment_cleaner;
 
@@ -108,6 +109,7 @@ protected:
 
   virtual void _init() {
     tm = get_transaction_manager(*segment_manager);
+    itm = InterruptedTransactionManager(*tm);
     segment_cleaner = tm->get_segment_cleaner();
     lba_manager = tm->get_lba_manager();
   }
@@ -115,6 +117,7 @@ protected:
   virtual void _destroy() {
     segment_cleaner = nullptr;
     lba_manager = nullptr;
+    itm.reset();
     tm.reset();
   }
 
@@ -145,8 +148,16 @@ protected:
     );
   }
 
+  auto submit_transaction_fut(Transaction &t) {
+    return with_trans_intr(
+      t,
+      [this](auto &t) {
+       return tm->submit_transaction(t);
+      });
+  }
+
   void submit_transaction(TransactionRef t) {
-    tm->submit_transaction(*t).unsafe_get0();
+    submit_transaction_fut(*t).unsafe_get0();
     segment_cleaner->run_until_halt().get0();
   }
 };