crimson/os/seastore: move transaction out of cache

Transaction is going to gain some knowledge of logical
addresses.

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 622478368adfe2ecde4dd75725e5e22ebc1dae47..cc530512eab077c5540b57e583b1d4675f3dfaab 100644 (file)
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -9,6 +9,7 @@
 
 #include "include/buffer.h"
 #include "crimson/os/seastore/seastore_types.h"
+#include "crimson/os/seastore/transaction.h"
 #include "crimson/os/seastore/segment_manager.h"
 #include "crimson/common/errorator.h"
 #include "crimson/os/seastore/cached_extent.h"
@@ -16,67 +17,6 @@
 
 namespace crimson::os::seastore {
 
-/**
- * Transaction
- *
- * Representation of in-progress mutation. Used exclusively through Cache methods.
- */
-class Transaction {
-  friend class Cache;
-
-  RootBlockRef root;        ///< ref to root if mutated by transaction
-
-  segment_off_t offset = 0; ///< relative offset of next block
-
-  pextent_set_t read_set;   ///< set of extents read by paddr
-  ExtentIndex write_set;    ///< set of extents written by paddr
-
-  std::list<CachedExtentRef> fresh_block_list;   ///< list of fresh blocks
-  std::list<CachedExtentRef> mutated_block_list; ///< list of mutated blocks
-
-  pextent_set_t retired_set; ///< list of extents mutated by this transaction
-
-  CachedExtentRef get_extent(paddr_t addr) {
-    if (auto iter = write_set.find_offset(addr);
-       iter != write_set.end()) {
-      return CachedExtentRef(&*iter);
-    } else if (
-      auto iter = read_set.find(addr);
-      iter != read_set.end()) {
-      return *iter;
-    } else {
-      return CachedExtentRef();
-    }
-  }
-
-  void add_to_retired_set(CachedExtentRef ref) {
-    if (!ref->is_initial_pending()) {
-      // && retired_set.count(ref->get_paddr()) == 0
-      // If it's already in the set, insert here will be a noop,
-      // which is what we want.
-      retired_set.insert(ref);
-    }
-  }
-
-  void add_to_read_set(CachedExtentRef ref) {
-    ceph_assert(read_set.count(ref) == 0);
-    read_set.insert(ref);
-  }
-
-  void add_fresh_extent(CachedExtentRef ref) {
-    fresh_block_list.push_back(ref);
-    ref->set_paddr(make_record_relative_paddr(offset));
-    offset += ref->get_length();
-    write_set.insert(*ref);
-  }
-
-  void add_mutated_extent(CachedExtentRef ref) {
-    mutated_block_list.push_back(ref);
-    write_set.insert(*ref);
-  }
-};
-using TransactionRef = std::unique_ptr<Transaction>;
-
 /**
  * Cache
  *
@@ -145,10 +85,6 @@ public:
   Cache(SegmentManager &segment_manager);
   ~Cache();
 
-  TransactionRef get_transaction() {
-    return std::make_unique<Transaction>();
-  }
-
   /// Declare ref retired in t
   void retire_extent(Transaction &t, CachedExtentRef ref) {
     t.add_to_retired_set(ref);

diff --git a/src/crimson/os/seastore/lba_manager.h b/src/crimson/os/seastore/lba_manager.h
index c29652c78a8699b59ef81d68b0940724fe1ddf5e..4a6f2e954762f755dc92f40184e0f7a7f35da8f5 100644 (file)
--- a/src/crimson/os/seastore/lba_manager.h
+++ b/src/crimson/os/seastore/lba_manager.h
@@ -129,14 +129,12 @@ public:
     laddr_t addr) = 0;
 
   // TODO: probably unused, removed
-  using submit_lba_transaction_ertr = crimson::errorator<
+  using complete_transaction_ertr = crimson::errorator<
     crimson::ct_error::input_output_error>;
-  using submit_lba_transaction_ret = submit_lba_transaction_ertr::future<>;
-  virtual submit_lba_transaction_ret submit_lba_transaction(
+  using complete_transaction_ret = complete_transaction_ertr::future<>;
+  virtual complete_transaction_ret complete_transaction(
     Transaction &t) = 0;
 
-  virtual TransactionRef create_transaction() = 0;
-
   virtual ~LBAManager() {}
 };
 using LBAManagerRef = std::unique_ptr<LBAManager>;

diff --git a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc
index 3449b40b71990b6a2a1b5c4512d7d804a9102b91..f46359601654ec9182d851c7984a358351aec614 100644 (file)
--- a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc
+++ b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc
@@ -156,12 +156,12 @@ BtreeLBAManager::set_extent(
     });
 }
 
-BtreeLBAManager::submit_lba_transaction_ret
-BtreeLBAManager::submit_lba_transaction(
+BtreeLBAManager::complete_transaction_ret
+BtreeLBAManager::complete_transaction(
   Transaction &t)
 {
   // This is a noop for now and may end up not being necessary
-  return submit_lba_transaction_ertr::now();
+  return complete_transaction_ertr::now();
 }
 
 BtreeLBAManager::BtreeLBAManager(

diff --git a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h
index cdd3af7c40195734951b2b66b4df8536754be2e7..f064d26df5df203c1f8491c7ba507fa4278d5851 100644 (file)
--- a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h
+++ b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h
@@ -80,14 +80,9 @@ public:
     return update_refcount(t, addr, 1);
   }
 
-  submit_lba_transaction_ret submit_lba_transaction(
+  complete_transaction_ret complete_transaction(
     Transaction &t) final;
 
-  TransactionRef create_transaction() final {
-    auto t = new Transaction;
-    return TransactionRef(t);
-  }
-
 private:
   SegmentManager &segment_manager;
   Cache &cache;

diff --git a/src/crimson/os/seastore/seastore.h b/src/crimson/os/seastore/seastore.h
index 03cce9dcb7c96ec0f606ff981bf6c6f1e08d2fc4..38afac2e64761646ed212401fa734b4044c3ef12 100644 (file)
--- a/src/crimson/os/seastore/seastore.h
+++ b/src/crimson/os/seastore/seastore.h
@@ -17,6 +17,7 @@
 
 #include "os/Transaction.h"
 #include "crimson/os/futurized_store.h"
+#include "transaction.h"
 
 namespace crimson::os::seastore {
 
@@ -28,8 +29,6 @@ using OnodeRef = boost::intrusive_ptr<Onode>;
 class Journal;
 class LBAManager;
 class TransactionManager;
-class Transaction;
-using TransactionRef = std::unique_ptr<Transaction>;
 class Cache;
 
 class SeaStore final : public FuturizedStore {

diff --git a/src/crimson/os/seastore/transaction.h b/src/crimson/os/seastore/transaction.h
new file mode 100644 (file)
index 0000000..61752b5
--- /dev/null
+++ b/src/crimson/os/seastore/transaction.h
@@ -0,0 +1,80 @@
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab
+
+#pragma once
+
+#include <iostream>
+
+#include "crimson/os/seastore/seastore_types.h"
+#include "crimson/os/seastore/cached_extent.h"
+#include "crimson/os/seastore/root_block.h"
+
+namespace crimson::os::seastore {
+
+/**
+ * Transaction
+ *
+ * Representation of in-progress mutation. Used exclusively through Cache methods.
+ */
+class Transaction {
+  friend class Cache;
+
+  RootBlockRef root;        ///< ref to root if mutated by transaction
+
+  segment_off_t offset = 0; ///< relative offset of next block
+
+  pextent_set_t read_set;   ///< set of extents read by paddr
+  ExtentIndex write_set;    ///< set of extents written by paddr
+
+  std::list<CachedExtentRef> fresh_block_list;   ///< list of fresh blocks
+  std::list<CachedExtentRef> mutated_block_list; ///< list of mutated blocks
+
+  pextent_set_t retired_set; ///< list of extents mutated by this transaction
+
+public:
+  CachedExtentRef get_extent(paddr_t addr) {
+    if (auto iter = write_set.find_offset(addr);
+       iter != write_set.end()) {
+      return CachedExtentRef(&*iter);
+    } else if (
+      auto iter = read_set.find(addr);
+      iter != read_set.end()) {
+      return *iter;
+    } else {
+      return CachedExtentRef();
+    }
+  }
+
+  void add_to_retired_set(CachedExtentRef ref) {
+    if (!ref->is_initial_pending()) {
+      // && retired_set.count(ref->get_paddr()) == 0
+      // If it's already in the set, insert here will be a noop,
+      // which is what we want.
+      retired_set.insert(ref);
+    }
+  }
+
+  void add_to_read_set(CachedExtentRef ref) {
+    ceph_assert(read_set.count(ref) == 0);
+    read_set.insert(ref);
+  }
+
+  void add_fresh_extent(CachedExtentRef ref) {
+    fresh_block_list.push_back(ref);
+    ref->set_paddr(make_record_relative_paddr(offset));
+    offset += ref->get_length();
+    write_set.insert(*ref);
+  }
+
+  void add_mutated_extent(CachedExtentRef ref) {
+    mutated_block_list.push_back(ref);
+    write_set.insert(*ref);
+  }
+};
+using TransactionRef = std::unique_ptr<Transaction>;
+
+inline TransactionRef make_transaction() {
+  return std::make_unique<Transaction>();
+}
+
+}

diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc
index 785eaf1c0fadfeec45a1f557a0def09c5e713474..8b959186966707d3d155287312003adf869af709 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.cc
+++ b/src/crimson/os/seastore/transaction_manager.cc
@@ -36,7 +36,7 @@ TransactionManager::mkfs_ertr::future<> TransactionManager::mkfs()
   return journal.open_for_write().safe_then([this] {
     logger().debug("TransactionManager::mkfs: about to do_with");
     return seastar::do_with(
-      lba_manager.create_transaction(),
+      create_transaction(),
       [this](auto &transaction) {
        logger().debug("TransactionManager::mkfs: about to cache.mkfs");
        cache.init();
@@ -132,7 +132,7 @@ TransactionManager::submit_transaction(
   logger().debug("TransactionManager::submit_transaction");
 
   return journal.submit_record(std::move(*record)).safe_then(
-    [this, t=std::move(t)](paddr_t addr) {
+    [this, t=std::move(t)](paddr_t addr) mutable {
       cache.complete_commit(*t, addr);
     },
     submit_transaction_ertr::pass_further{},

diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h
index 814cbcf359130f6d8ec5b0f97ab6c9095254e39e..b93019701bda520ad112cb29c827588f92df1d82 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.h
+++ b/src/crimson/os/seastore/transaction_manager.h
@@ -139,7 +139,7 @@ public:
 
   /// Creates empty transaction
   TransactionRef create_transaction() {
-    return lba_manager.create_transaction();
+    return make_transaction();
   }
 
   /**

diff --git a/src/test/crimson/seastore/test_btree_lba_manager.cc b/src/test/crimson/seastore/test_btree_lba_manager.cc
index be5408ebacd304a92762ba89f25a2fe9f5148e37..05d76638db6cf220b167f4f7e904700e4b0363d8 100644 (file)
--- a/src/test/crimson/seastore/test_btree_lba_manager.cc
+++ b/src/test/crimson/seastore/test_btree_lba_manager.cc
@@ -62,7 +62,7 @@ struct btree_lba_manager_test :
     }
 
     return journal.submit_record(std::move(*record)).safe_then(
-      [this, t=std::move(t)](paddr_t addr) {
+      [this, t=std::move(t)](paddr_t addr) mutable {
        cache.complete_commit(*t, addr);
       },
       crimson::ct_error::all_same_way([](auto e) {
@@ -76,7 +76,7 @@ struct btree_lba_manager_test :
       return journal.open_for_write();
     }).safe_then([this] {
       return seastar::do_with(
-       lba_manager->create_transaction(),
+       make_transaction(),
        [this](auto &transaction) {
          cache.init();
          return cache.mkfs(*transaction
@@ -119,7 +119,7 @@ struct btree_lba_manager_test :
 
   auto create_transaction() {
     auto t = test_transaction_t{
-      lba_manager->create_transaction(),
+      make_transaction(),
       test_lba_mappings
     };
     cache.alloc_new_extent<TestBlockPhysical>(*t.t, TestBlockPhysical::SIZE);

diff --git a/src/test/crimson/seastore/test_seastore_cache.cc b/src/test/crimson/seastore/test_seastore_cache.cc
index 96fd87aa46b16972319422eba32bfbc78d7e16a0..0e4b29986dd3839224e3b04931ed8e3e2a24cde8 100644 (file)
--- a/src/test/crimson/seastore/test_seastore_cache.cc
+++ b/src/test/crimson/seastore/test_seastore_cache.cc
@@ -54,7 +54,7 @@ struct cache_test_t : public seastar_test_suite_t {
       std::move(bl),
       true
     ).safe_then(
-      [this, prev, t=std::move(t)] {
+      [this, prev, t=std::move(t)]() mutable {
        cache.complete_commit(*t, prev);
        return seastar::make_ready_future<std::optional<paddr_t>>(prev);
       },
@@ -65,14 +65,14 @@ struct cache_test_t : public seastar_test_suite_t {
   }
 
   auto get_transaction() {
-    return TransactionRef(new Transaction);
+    return make_transaction();
   }
 
   seastar::future<> set_up_fut() final {
     return segment_manager.init().safe_then(
       [this] {
        return seastar::do_with(
-         TransactionRef(new Transaction()),
+         make_transaction(),
          [this](auto &transaction) {
            cache.init();
            return cache.mkfs(*transaction).safe_then(