crimson/os/seastore/transaction_manager: pass t by ref to submit_transaction

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

diff --git a/src/crimson/os/seastore/seastore.cc b/src/crimson/os/seastore/seastore.cc
index cf0b68516a6abfc845ddea40819f3040089e9e35..20f36667dde908c5a3631371e9c255da04822566 100644 (file)
--- a/src/crimson/os/seastore/seastore.cc
+++ b/src/crimson/os/seastore/seastore.cc
@@ -98,7 +98,7 @@ seastar::future<> SeaStore::mkfs(uuid_d new_osd_fsid)
            *t,
            coll_root);
          return transaction_manager->submit_transaction(
-           std::move(t));
+           *t);
        });
       });
   }).safe_then([this] {
@@ -637,7 +637,7 @@ seastar::future<> SeaStore::do_transaction(
         // destruction in debug mode, which need to be done before calling
         // submit_transaction().
         ctx.onodes.clear();
-       return transaction_manager->submit_transaction(std::move(ctx.transaction));
+       return transaction_manager->submit_transaction(*ctx.transaction);
       }).safe_then([&ctx]() {
        for (auto i : {
            ctx.ext_transaction.get_on_applied(),
@@ -1067,7 +1067,7 @@ seastar::future<> SeaStore::write_meta(const std::string& key,
         return transaction_manager->update_root_meta(
          *t, key, value
        ).safe_then([this, &t] {
-         return transaction_manager->submit_transaction(std::move(t));
+         return transaction_manager->submit_transaction(*t);
        });
       });
     }).handle_error(

diff --git a/src/crimson/os/seastore/segment_cleaner.cc b/src/crimson/os/seastore/segment_cleaner.cc
index 59d4610c30580311c2254ac2991f6df2b1208357..b0815d029c60fb6f5d8f3aea50152d259ddc878d 100644 (file)
--- a/src/crimson/os/seastore/segment_cleaner.cc
+++ b/src/crimson/os/seastore/segment_cleaner.cc
@@ -288,9 +288,9 @@ SegmentCleaner::gc_trim_journal_ret SegmentCleaner::gc_trim_journal()
        [this](auto &tref) {
          return with_trans_intr(*tref, [this, &tref](auto &t) {
            return rewrite_dirty(t, get_dirty_tail()
-           ).si_then([this, &tref] {
+           ).si_then([this, &t] {
              return ecb->submit_transaction_direct(
-               std::move(tref));
+               t);
            });
          });
        });
@@ -363,11 +363,11 @@ SegmentCleaner::gc_reclaim_space_ret SegmentCleaner::gc_reclaim_space()
                      }
                    });
                  }
-               ).si_then([this, &tref] {
+               ).si_then([this, &t] {
                  if (scan_cursor->is_complete()) {
-                   tref->mark_segment_to_release(scan_cursor->get_offset().segment);
+                   t.mark_segment_to_release(scan_cursor->get_offset().segment);
                  }
-                 return ecb->submit_transaction_direct(std::move(tref));
+                 return ecb->submit_transaction_direct(t);
                });
              });
            });

diff --git a/src/crimson/os/seastore/segment_cleaner.h b/src/crimson/os/seastore/segment_cleaner.h
index 914c6597f1f362d7116a033fa57513e4c472eeab..256225d7f5400cb3762ec8ebf03d8dd860df1cd6 100644 (file)
--- a/src/crimson/os/seastore/segment_cleaner.h
+++ b/src/crimson/os/seastore/segment_cleaner.h
@@ -337,7 +337,7 @@ public:
     using submit_transaction_direct_ret =
       submit_transaction_direct_iertr::future<>;
     virtual submit_transaction_direct_ret submit_transaction_direct(
-      TransactionRef t) = 0;
+      Transaction &t) = 0;
   };
 
 private:

diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc
index 1c540e87611767dfa7e9bcc9b36bf30fe9e70e1c..0b0216546a069a8152303c785028236755154b8c 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.cc
+++ b/src/crimson/os/seastore/transaction_manager.cc
@@ -50,7 +50,7 @@ TransactionManager::mkfs_ertr::future<> TransactionManager::mkfs()
          return with_trans_intr(
            *transaction,
            [this, FNAME, &transaction](auto&) {
-             return submit_transaction_direct(std::move(transaction));
+             return submit_transaction_direct(*transaction);
            }
          ).handle_error(
            crimson::ct_error::eagain::handle([] {
@@ -219,27 +219,25 @@ TransactionManager::refs_ret TransactionManager::dec_ref(
 
 TransactionManager::submit_transaction_iertr::future<>
 TransactionManager::submit_transaction(
-  TransactionRef t)
+  Transaction &t)
 {
   LOG_PREFIX(TransactionManager::submit_transaction);
-  DEBUGT("about to await throttle", *t);
-  auto &tref = *t;
+  DEBUGT("about to await throttle", t);
   return trans_intr::make_interruptible(
-    tref.handle.enter(write_pipeline.wait_throttle)
+    t.handle.enter(write_pipeline.wait_throttle)
   ).then_interruptible([this] {
     return trans_intr::make_interruptible(segment_cleaner->await_hard_limits());
-  }).then_interruptible([this, t=std::move(t)]() mutable {
-    return submit_transaction_direct(std::move(t));
+  }).then_interruptible([this, &t]() {
+    return submit_transaction_direct(t);
   });
 }
 
 TransactionManager::submit_transaction_direct_ret
 TransactionManager::submit_transaction_direct(
-  TransactionRef t)
+  Transaction &tref)
 {
   LOG_PREFIX(TransactionManager::submit_transaction_direct);
-  DEBUGT("about to prepare", *t);
-  auto &tref = *t;
+  DEBUGT("about to prepare", tref);
   return trans_intr::make_interruptible(
     tref.handle.enter(write_pipeline.prepare)
   ).then_interruptible([this, FNAME, &tref]() mutable
@@ -274,8 +272,8 @@ TransactionManager::submit_transaction_direct(
       crimson::ct_error::all_same_way([](auto e) {
        ceph_assert(0 == "Hit error submitting to journal");
       }));
-    }).finally([t=std::move(t)]() mutable {
-      t->handle.exit();
+    }).finally([&tref]() {
+      tref.handle.exit();
     });
 }
 

diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h
index 8c3ac173fce071c51dce464c02ae7dab918323ff..f0e588a3078e3eb1d720ddcbcea1862c6447ad6c 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.h
+++ b/src/crimson/os/seastore/transaction_manager.h
@@ -378,12 +378,12 @@ public:
    * Atomically submits transaction to persistence
    */
   using submit_transaction_iertr = base_iertr;
-  submit_transaction_iertr::future<> submit_transaction(TransactionRef);
+  submit_transaction_iertr::future<> submit_transaction(Transaction &);
 
   /// SegmentCleaner::ExtentCallbackInterface
   using SegmentCleaner::ExtentCallbackInterface::submit_transaction_direct_ret;
   submit_transaction_direct_ret submit_transaction_direct(
-    TransactionRef t) final;
+    Transaction &t) final;
 
   using SegmentCleaner::ExtentCallbackInterface::get_next_dirty_extents_ret;
   get_next_dirty_extents_ret get_next_dirty_extents(
@@ -623,15 +623,7 @@ public:
   INT_FORWARD(reserve_region)
   INT_FORWARD(find_hole)
   PARAM_INT_FORWARD(alloc_extents)
-
-
-  auto submit_transaction(TransactionRef t) const {
-    return with_trans_intr(
-      *t,
-      [this, t=std::move(t)](auto &) mutable {
-       return tm.submit_transaction(std::move(t));
-      });
-  }
+  INT_FORWARD(submit_transaction)
 
   INT_FORWARD(read_root_meta)
   INT_FORWARD(update_root_meta)

diff --git a/src/crimson/tools/store_nbd/tm_driver.cc b/src/crimson/tools/store_nbd/tm_driver.cc
index 8cabeecb045dc69b5d559b059f0eb36b8aa77dd9..64271bc0ee204d15e4d9dca868737ccee2cea4a9 100644 (file)
--- a/src/crimson/tools/store_nbd/tm_driver.cc
+++ b/src/crimson/tools/store_nbd/tm_driver.cc
@@ -43,7 +43,7 @@ seastar::future<> TMDriver::write(
          assert(ext->get_bptr().length() == ptr.length());
          ext->get_bptr().swap(ptr);
          logger().debug("submitting transaction");
-         return tm->submit_transaction(std::move(t));
+         return tm->submit_transaction(*t);
        });
       });
   }).handle_error(

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 9a314dcb5f94f42dab5a3105faed2e2631a82f42..362edac7e17d959af1cc5168e3e44835438eb397 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
@@ -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(std::move(t));
+             return tm->submit_transaction(*t);
            });
          });
       }).safe_then([this] {
@@ -104,7 +104,7 @@ struct fltree_onode_manager_test_t
   void with_transaction(F&& f) {
     auto t = tm->create_transaction();
     std::invoke(f, *t);
-    tm->submit_transaction(std::move(t)).unsafe_get0();
+    submit_transaction(std::move(t));
     segment_cleaner->run_until_halt().get0();
   }
 

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 010c02301630439e70e884af75dd760b2bfff9e6..0792e9575c64050ac88bfdae538bfdf03f141cdd 100644 (file)
--- a/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc
+++ b/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc
@@ -1536,7 +1536,7 @@ TEST_F(d_seastore_tm_test_t, 6_random_tree_insert_erase)
     {
       auto t = tm->create_transaction();
       tree->bootstrap(*t).unsafe_get();
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
       segment_cleaner->run_until_halt().get0();
     }
 
@@ -1544,7 +1544,7 @@ TEST_F(d_seastore_tm_test_t, 6_random_tree_insert_erase)
     {
       auto t = tm->create_transaction();
       tree->insert(*t).unsafe_get();
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
       segment_cleaner->run_until_halt().get0();
     }
     {
@@ -1567,7 +1567,7 @@ TEST_F(d_seastore_tm_test_t, 6_random_tree_insert_erase)
     {
       auto t = tm->create_transaction();
       tree->erase(*t, kvs.size() / 4 * 3).unsafe_get();
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
       segment_cleaner->run_until_halt().get0();
     }
     {
@@ -1589,7 +1589,7 @@ TEST_F(d_seastore_tm_test_t, 6_random_tree_insert_erase)
     {
       auto t = tm->create_transaction();
       tree->erase(*t, kvs.size()).unsafe_get();
-      tm->submit_transaction(std::move(t)).unsafe_get();
+      submit_transaction(std::move(t));
       segment_cleaner->run_until_halt().get0();
     }
     {
@@ -1638,11 +1638,14 @@ TEST_F(d_seastore_tm_test_t, 7_tree_insert_erase_eagain)
     ++num_ops;
     repeat_eagain([this, &tree, &num_ops_eagain] {
       ++num_ops_eagain;
-      auto t = tm->create_transaction();
-      return tree->bootstrap(*t
-      ).safe_then([this, t = std::move(t)] () mutable {
-        return tm->submit_transaction(std::move(t));
-      });
+      return seastar::do_with(
+       tm->create_transaction(),
+       [this, &tree](auto &t) {
+         return tree->bootstrap(*t
+         ).safe_then([this, &t] {
+           return tm->submit_transaction(*t);
+         });
+       });
     }).unsafe_get0();
     segment_cleaner->run_until_halt().get0();
 
@@ -1654,12 +1657,15 @@ TEST_F(d_seastore_tm_test_t, 7_tree_insert_erase_eagain)
         ++num_ops;
         repeat_eagain([this, &tree, &num_ops_eagain, &iter] {
           ++num_ops_eagain;
-          auto t = tm->create_transaction();
-          return tree->insert_one(*t, iter
-          ).safe_then([this, t = std::move(t)] (auto cursor) mutable {
-            cursor.invalidate();
-            return tm->submit_transaction(std::move(t));
-          });
+         return seastar::do_with(
+           tm->create_transaction(),
+           [this, &tree, &iter](auto &t) {
+             return tree->insert_one(*t, iter
+             ).safe_then([this, &t](auto cursor) {
+               cursor.invalidate();
+               return tm->submit_transaction(*t);
+             });
+           });
         }).unsafe_get0();
         segment_cleaner->run_until_halt().get0();
         ++iter;
@@ -1698,11 +1704,14 @@ TEST_F(d_seastore_tm_test_t, 7_tree_insert_erase_eagain)
         ++num_ops;
         repeat_eagain([this, &tree, &num_ops_eagain, &iter] {
           ++num_ops_eagain;
-          auto t = tm->create_transaction();
-          return tree->erase_one(*t, iter
-          ).safe_then([this, t = std::move(t)] () mutable {
-            return tm->submit_transaction(std::move(t));
-          });
+         return seastar::do_with(
+           tm->create_transaction(),
+           [this, &tree, &iter](auto &t) {
+             return tree->erase_one(*t, iter
+             ).safe_then([this, &t] () mutable {
+               return tm->submit_transaction(*t);
+             });
+           });
         }).unsafe_get0();
         segment_cleaner->run_until_halt().get0();
         ++iter;

diff --git a/src/test/crimson/seastore/test_collection_manager.cc b/src/test/crimson/seastore/test_collection_manager.cc
index 511868444e6c610c8720bc109ad48caddf625964..8fd31880b37242b481120243973096f288231455 100644 (file)
--- a/src/test/crimson/seastore/test_collection_manager.cc
+++ b/src/test/crimson/seastore/test_collection_manager.cc
@@ -69,11 +69,6 @@ struct collection_manager_test_t :
     auto t = tm->create_transaction();
     checking_mappings(coll_root, *t);
   }
-
-  void submit_transaction(TransactionRef &&t) {
-    tm->submit_transaction(std::move(t)).unsafe_get0();
-    segment_cleaner->run_until_halt().get0();
-  }
 };
 
 TEST_F(collection_manager_test_t, basic)

diff --git a/src/test/crimson/seastore/test_object_data_handler.cc b/src/test/crimson/seastore/test_object_data_handler.cc
index a5777bd4f1c7902e2d26f814918ab7968b6bdf59..c38ed6ccf3c4f0fe70c43a0f1a687992568baf9e 100644 (file)
--- a/src/test/crimson/seastore/test_object_data_handler.cc
+++ b/src/test/crimson/seastore/test_object_data_handler.cc
@@ -44,7 +44,7 @@ struct object_data_handler_test_t:
   object_data_handler_test_t() {}
 
   auto submit_transaction(TransactionRef &&t) {
-    return tm->submit_transaction(std::move(t)
+    return tm->submit_transaction(*t
     ).safe_then([this] {
       return segment_cleaner->run_until_halt();
     });

diff --git a/src/test/crimson/seastore/test_omap_manager.cc b/src/test/crimson/seastore/test_omap_manager.cc
index 8b4889c847067205966cb87f17af259e116a9064..570b8b1b816a360be2d30370bd844d8dcf41ac40 100644 (file)
--- a/src/test/crimson/seastore/test_omap_manager.cc
+++ b/src/test/crimson/seastore/test_omap_manager.cc
@@ -193,11 +193,6 @@ struct omap_manager_test_t :
     omap_manager = omap_manager::create_omap_manager(*tm);
     logger().debug("{}: end", __func__);
   }
-
-  void submit_transaction(TransactionRef &&t) {
-    tm->submit_transaction(std::move(t)).unsafe_get0();
-    segment_cleaner->run_until_halt().get0();
-  }
 };
 
 TEST_F(omap_manager_test_t, basic)

diff --git a/src/test/crimson/seastore/test_transaction_manager.cc b/src/test/crimson/seastore/test_transaction_manager.cc
index 8e95c0e0684917ccd22df3213ec81e603df6ec65..5ee55acd15c8d3a7c8a0f7697d55c8255af2af1b 100644 (file)
--- a/src/test/crimson/seastore/test_transaction_manager.cc
+++ b/src/test/crimson/seastore/test_transaction_manager.cc
@@ -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(std::move(t.t)
+    bool success = tm->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 4323124f650b61eb1025df91b72dc3d91d5fadd5..f839939ed9c5f58c64c3f6bce86017716256c505 100644 (file)
--- a/src/test/crimson/seastore/transaction_manager_test_state.h
+++ b/src/test/crimson/seastore/transaction_manager_test_state.h
@@ -144,6 +144,11 @@ protected:
       crimson::ct_error::assert_all{"Error in teardown"}
     );
   }
+
+  void submit_transaction(TransactionRef t) {
+    tm->submit_transaction(*t).unsafe_get0();
+    segment_cleaner->run_until_halt().get0();
+  }
 };
 
 class TestSegmentManagerWrapper final : public SegmentManager {

diff --git a/src/tools/crimson/perf_staged_fltree.cc b/src/tools/crimson/perf_staged_fltree.cc
index 88a98ef9565666d85f8184084548a7a23175ad66..3f5f98fc28aae84c0c29179b229d84c47a6725e4 100644 (file)
--- a/src/tools/crimson/perf_staged_fltree.cc
+++ b/src/tools/crimson/perf_staged_fltree.cc
@@ -36,15 +36,13 @@ class PerfTree : public TMTestState {
         {
           auto t = tm->create_transaction();
           tree->bootstrap(*t).unsafe_get();
-          tm->submit_transaction(std::move(t)).unsafe_get();
-          segment_cleaner->run_until_halt().get0();
+          submit_transaction(std::move(t));
         }
         {
           auto t = tm->create_transaction();
           tree->insert(*t).unsafe_get();
           auto start_time = mono_clock::now();
-          tm->submit_transaction(std::move(t)).unsafe_get();
-          segment_cleaner->run_until_halt().get0();
+          submit_transaction(std::move(t));
           std::chrono::duration<double> duration = mono_clock::now() - start_time;
           logger().warn("submit_transaction() done! {}s", duration.count());
         }
@@ -57,8 +55,7 @@ class PerfTree : public TMTestState {
         {
           auto t = tm->create_transaction();
           tree->erase(*t, kvs.size() * erase_ratio).unsafe_get();
-          tm->submit_transaction(std::move(t)).unsafe_get();
-          segment_cleaner->run_until_halt().get0();
+          submit_transaction(std::move(t));
         }
         {
           auto t = tm->create_transaction();