crimson/os/seastore: route all transaction creations through cache

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

diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index 1b0c00f5113bf0131fffa027c521f5c3d0dde199..c48f27eaa511aa785643d5f45d5246ff4aa2481c 100644 (file)
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -90,6 +90,22 @@ public:
   Cache(SegmentManager &segment_manager);
   ~Cache();
 
+  /// Creates empty transaction
+  TransactionRef create_transaction() {
+    return std::make_unique<Transaction>(
+      get_dummy_ordering_handle(),
+      false
+    );
+  }
+
+  /// Creates empty weak transaction
+  TransactionRef create_weak_transaction() {
+    return std::make_unique<Transaction>(
+      get_dummy_ordering_handle(),
+      true
+    );
+  }
+
   /**
    * drop_from_cache
    *

diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/fwd.h b/src/crimson/os/seastore/onode_manager/staged-fltree/fwd.h
index 57f7d63aed8cc7f5485bf797103b20706b8da1ce..890e1fcd08c0721c876d2288de59ee7d8bd29420 100644 (file)
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/fwd.h
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/fwd.h
@@ -19,7 +19,6 @@ namespace crimson::os::seastore::onode {
 
 using crimson::os::seastore::Transaction;
 using crimson::os::seastore::TransactionRef;
-using crimson::os::seastore::make_transaction;
 using crimson::os::seastore::laddr_t;
 using crimson::os::seastore::L_ADDR_MIN;
 using crimson::os::seastore::L_ADDR_NULL;

diff --git a/src/crimson/os/seastore/seastore.cc b/src/crimson/os/seastore/seastore.cc
index 211eb5a1f8608a94dd48bda49d34130323232a99..899019e0bd3cafdb1739353a8589b6715f8eeaa3 100644 (file)
--- a/src/crimson/os/seastore/seastore.cc
+++ b/src/crimson/os/seastore/seastore.cc
@@ -70,7 +70,7 @@ seastar::future<> SeaStore::mkfs(uuid_d new_osd_fsid)
   return transaction_manager->mkfs(
   ).safe_then([this] {
     return seastar::do_with(
-      make_transaction(),
+      transaction_manager->create_transaction(),
       [this](auto &t) {
        return onode_manager->mkfs(*t
        ).safe_then([this, &t] {
@@ -139,7 +139,7 @@ seastar::future<std::vector<coll_t>> SeaStore::list_collections()
       return repeat_eagain([this, &ret] {
 
        return seastar::do_with(
-         make_transaction(),
+         transaction_manager->create_transaction(),
          [this, &ret](auto &t) {
            return transaction_manager->read_collection_root(*t
            ).safe_then([this, &ret, &t](auto coll_root) {

diff --git a/src/crimson/os/seastore/seastore.h b/src/crimson/os/seastore/seastore.h
index 8c71b96b82f08ad40664d1ad752c00a7e710dfcd..7857ac0f67303e575b925da0cd86d3385f79d5be 100644 (file)
--- a/src/crimson/os/seastore/seastore.h
+++ b/src/crimson/os/seastore/seastore.h
@@ -158,9 +158,9 @@ private:
     return seastar::do_with(
       internal_context_t{ ch, std::move(t) },
       std::forward<F>(f),
-      [](auto &ctx, auto &f) {
+      [this](auto &ctx, auto &f) {
        return repeat_eagain([&]() {
-         ctx.reset(make_transaction());
+         ctx.reset(transaction_manager->create_transaction());
          return std::invoke(f, ctx);
        }).handle_error(
          crimson::ct_error::eagain::pass_further{},
@@ -184,7 +184,7 @@ private:
       std::forward<F>(f),
       [=](auto &oid, auto &ret, auto &t, auto &onode, auto &f) {
        return repeat_eagain([&, this] {
-         t = make_transaction();
+         t = transaction_manager->create_transaction();
          return onode_manager->get_onode(
            *t, oid
          ).safe_then([&, this](auto onode_ret) {

diff --git a/src/crimson/os/seastore/seastore_types.h b/src/crimson/os/seastore/seastore_types.h
index de6485ba4b01ac92e0cf5f0e83c8e63da91f599a..1855049fd75605fc0b8f080874f4ea405c5e417c 100644 (file)
--- a/src/crimson/os/seastore/seastore_types.h
+++ b/src/crimson/os/seastore/seastore_types.h
@@ -243,6 +243,10 @@ struct journal_seq_t {
 };
 WRITE_CMP_OPERATORS_2(journal_seq_t, segment_seq, offset)
 WRITE_EQ_OPERATORS_2(journal_seq_t, segment_seq, offset)
+constexpr journal_seq_t JOURNAL_SEQ_MIN{
+  0,
+  paddr_t{0, 0}
+};
 
 std::ostream &operator<<(std::ostream &out, const journal_seq_t &seq);
 

diff --git a/src/crimson/os/seastore/segment_cleaner.cc b/src/crimson/os/seastore/segment_cleaner.cc
index 6dc2a78472d7e6f6573840042ca1b02bd563c376..3a0af1388a4aa3465f53a2a68e214d9c8252c80f 100644 (file)
--- a/src/crimson/os/seastore/segment_cleaner.cc
+++ b/src/crimson/os/seastore/segment_cleaner.cc
@@ -266,7 +266,7 @@ SegmentCleaner::gc_trim_journal_ret SegmentCleaner::gc_trim_journal()
   return repeat_eagain(
     [this] {
       return seastar::do_with(
-       make_transaction(),
+       ecb->create_transaction(),
        [this](auto &t) {
          return rewrite_dirty(*t, get_dirty_tail()
          ).safe_then([this, &t] {
@@ -310,7 +310,7 @@ SegmentCleaner::gc_reclaim_space_ret SegmentCleaner::gc_reclaim_space()
            "SegmentCleaner::gc_reclaim_space: processing {} extents",
            extents.size());
          return seastar::do_with(
-           make_transaction(),
+           ecb->create_transaction(),
            [this, &extents](auto &t) mutable {
              return crimson::do_for_each(
                extents,

diff --git a/src/crimson/os/seastore/segment_cleaner.h b/src/crimson/os/seastore/segment_cleaner.h
index 26dab0dd8dfd9713a3f977243aaa1dae2abc2174..84ed7a766fd105ea69766c69f3e3d2caaf26d40b 100644 (file)
--- a/src/crimson/os/seastore/segment_cleaner.h
+++ b/src/crimson/os/seastore/segment_cleaner.h
@@ -251,6 +251,9 @@ public:
   class ExtentCallbackInterface {
   public:
     virtual ~ExtentCallbackInterface() = default;
+
+    virtual TransactionRef create_transaction() = 0;
+
     /**
      * get_next_dirty_extent
      *

diff --git a/src/crimson/os/seastore/transaction.h b/src/crimson/os/seastore/transaction.h
index e487fa6d42ad5eea06d1b826914ef12a29d2b7bc..4510c525746eb60d264207433a9ff84592948456 100644 (file)
--- a/src/crimson/os/seastore/transaction.h
+++ b/src/crimson/os/seastore/transaction.h
@@ -113,8 +113,7 @@ public:
 
 private:
   friend class Cache;
-  friend Ref make_transaction();
-  friend Ref make_weak_transaction();
+  friend Ref make_test_transaction();
 
   /**
    * If set, *this may not be used to perform writes and will not provide
@@ -155,17 +154,12 @@ public:
 };
 using TransactionRef = Transaction::Ref;
 
-inline TransactionRef make_transaction() {
+/// Should only be used with dummy staged-fltree node extent manager
+inline TransactionRef make_test_transaction() {
   return std::make_unique<Transaction>(
     get_dummy_ordering_handle(),
     false
   );
 }
 
-inline TransactionRef make_weak_transaction() {
-  return std::make_unique<Transaction>(
-    get_dummy_ordering_handle(),
-    true);
-}
-
 }

diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc
index 3b7d74733a799f3b31b65da33dad55ae7674f66b..47dd9da5fb23dee91117fa0e5504e9ae79ec446b 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.cc
+++ b/src/crimson/os/seastore/transaction_manager.cc
@@ -73,7 +73,7 @@ TransactionManager::mount_ertr::future<> TransactionManager::mount()
   }).safe_then([this](auto addr) {
     segment_cleaner->set_journal_head(addr);
     return seastar::do_with(
-      make_weak_transaction(),
+      create_weak_transaction(),
       [this](auto &t) {
        return cache->init_cached_extents(*t, [this](auto &t, auto &e) {
          return lba_manager->init_cached_extent(t, e);

diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h
index 2fa84cb7462b7ce9e48a7981424b1511f9093e7d..a49af8c9e5ca1cece71fe2f2198f185a31cdf606 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.h
+++ b/src/crimson/os/seastore/transaction_manager.h
@@ -86,13 +86,13 @@ public:
   close_ertr::future<> close();
 
   /// Creates empty transaction
-  TransactionRef create_transaction() {
-    return make_transaction();
+  TransactionRef create_transaction() final {
+    return cache->create_transaction();
   }
 
-  /// Creates weak transaction
+  /// Creates empty weak transaction
   TransactionRef create_weak_transaction() {
-    return make_weak_transaction();
+    return cache->create_weak_transaction();
   }
 
   /**

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 03dbac51d81c45b49c6ffae889eed0da229a15ab..3a5bde87496487b6994ba201b5a0d17e21dc007b 100644 (file)
--- a/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc
+++ b/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc
@@ -137,7 +137,7 @@ TEST_F(a_basic_test_t, 2_node_sizes)
 {
   run_async([this] {
     auto nm = NodeExtentManager::create_dummy(IS_DUMMY_SYNC);
-    auto t = make_transaction();
+    auto t = make_test_transaction();
     ValueBuilderImpl<TestValue> vb;
     context_t c{*nm, vb, *t};
     std::array<std::pair<NodeImplURef, NodeExtentMutable>, 16> nodes = {
@@ -181,7 +181,7 @@ struct b_dummy_tree_test_t : public seastar_test_suite_t {
 
   b_dummy_tree_test_t()
     : moved_nm{NodeExtentManager::create_dummy(IS_DUMMY_SYNC)},
-      ref_t{make_transaction()},
+      ref_t{make_test_transaction()},
       t{*ref_t},
       c{*moved_nm, vb, t},
       tree{std::move(moved_nm)} {}
@@ -403,7 +403,7 @@ class TestTree {
  public:
   TestTree()
     : moved_nm{NodeExtentManager::create_dummy(IS_DUMMY_SYNC)},
-      ref_t{make_transaction()},
+      ref_t{make_test_transaction()},
       t{*ref_t},
       c{*moved_nm, vb, t},
       tree{std::move(moved_nm)},
@@ -457,7 +457,7 @@ class TestTree {
                           const split_expectation_t& expected) {
     return seastar::async([this, key, value, expected] {
       TestBtree tree_clone(NodeExtentManager::create_dummy(IS_DUMMY_SYNC));
-      auto ref_t_clone = make_transaction();
+      auto ref_t_clone = make_test_transaction();
       Transaction& t_clone = *ref_t_clone;
       tree_clone.test_clone_from(t_clone, t, tree).unsafe_get0();
 
@@ -966,7 +966,7 @@ class DummyChildPool {
   std::optional<TestBtree> p_btree;
   NodeExtentManager* p_nm = nullptr;
   ValueBuilderImpl<TestValue> vb;
-  TransactionRef ref_t = make_transaction();
+  TransactionRef ref_t = make_test_transaction();
 
   std::random_device rd;
   std::set<Ref<DummyChild>> splitable_nodes;

diff --git a/src/test/crimson/seastore/test_btree_lba_manager.cc b/src/test/crimson/seastore/test_btree_lba_manager.cc
index 544d074e81644f28bfcc43477820e63c0b363abc..a7e2b96689a0eb3c84f4e2e880b5cbc87e6c2e49 100644 (file)
--- a/src/test/crimson/seastore/test_btree_lba_manager.cc
+++ b/src/test/crimson/seastore/test_btree_lba_manager.cc
@@ -78,7 +78,7 @@ struct btree_lba_manager_test :
       return journal.open_for_write();
     }).safe_then([this](auto addr) {
       return seastar::do_with(
-       make_transaction(),
+       cache.create_transaction(),
        [this](auto &transaction) {
          cache.init();
          return cache.mkfs(*transaction
@@ -121,7 +121,7 @@ struct btree_lba_manager_test :
 
   auto create_transaction() {
     auto t = test_transaction_t{
-      make_transaction(),
+      cache.create_transaction(),
       test_lba_mappings
     };
     cache.alloc_new_extent<TestBlockPhysical>(*t.t, TestBlockPhysical::SIZE);
@@ -130,7 +130,7 @@ struct btree_lba_manager_test :
 
   auto create_weak_transaction() {
     auto t = test_transaction_t{
-      make_weak_transaction(),
+      cache.create_weak_transaction(),
       test_lba_mappings
     };
     return t;

diff --git a/src/test/crimson/seastore/test_seastore_cache.cc b/src/test/crimson/seastore/test_seastore_cache.cc
index 913668b08355a22b2c9f20bff8cb3496a4157e95..bf58c8c454da85253167bea3af3d1085f8eac5f4 100644 (file)
--- a/src/test/crimson/seastore/test_seastore_cache.cc
+++ b/src/test/crimson/seastore/test_seastore_cache.cc
@@ -66,7 +66,7 @@ struct cache_test_t : public seastar_test_suite_t {
   }
 
   auto get_transaction() {
-    return make_transaction();
+    return cache.create_transaction();
   }
 
   seastar::future<> set_up_fut() final {
@@ -74,7 +74,7 @@ struct cache_test_t : public seastar_test_suite_t {
     ).safe_then(
       [this] {
        return seastar::do_with(
-         make_transaction(),
+         cache.create_transaction(),
          [this](auto &transaction) {
            cache.init();
            return cache.mkfs(*transaction).safe_then(