From 9f03c737a348ca98779dc6abd0d37729667f50ac Mon Sep 17 00:00:00 2001 From: Samuel Just Date: Tue, 30 Jun 2020 12:25:00 -0700 Subject: [PATCH] crimson/os/seastore: move transaction out of cache Transaction is going to gain some knowledge of logical addresses. Signed-off-by: Samuel Just --- src/crimson/os/seastore/cache.h | 66 +-------------- src/crimson/os/seastore/lba_manager.h | 8 +- .../lba_manager/btree/btree_lba_manager.cc | 6 +- .../lba_manager/btree/btree_lba_manager.h | 7 +- src/crimson/os/seastore/seastore.h | 3 +- src/crimson/os/seastore/transaction.h | 80 +++++++++++++++++++ .../os/seastore/transaction_manager.cc | 4 +- src/crimson/os/seastore/transaction_manager.h | 2 +- .../seastore/test_btree_lba_manager.cc | 6 +- .../crimson/seastore/test_seastore_cache.cc | 6 +- 10 files changed, 98 insertions(+), 90 deletions(-) create mode 100644 src/crimson/os/seastore/transaction.h diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index 622478368ad..cc530512eab 100644 --- 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 fresh_block_list; ///< list of fresh blocks - std::list 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; - /** * Cache * @@ -145,10 +85,6 @@ public: Cache(SegmentManager &segment_manager); ~Cache(); - TransactionRef get_transaction() { - return std::make_unique(); - } - /// 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 c29652c78a8..4a6f2e95476 100644 --- 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; 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 3449b40b719..f4635960165 100644 --- 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 cdd3af7c401..f064d26df5d 100644 --- 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 03cce9dcb7c..38afac2e647 100644 --- 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; class Journal; class LBAManager; class TransactionManager; -class Transaction; -using TransactionRef = std::unique_ptr; 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 index 00000000000..61752b5dafe --- /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 + +#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 fresh_block_list; ///< list of fresh blocks + std::list 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; + +inline TransactionRef make_transaction() { + return std::make_unique(); +} + +} diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc index 785eaf1c0fa..8b959186966 100644 --- 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 814cbcf3591..b93019701bd 100644 --- 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 be5408ebacd..05d76638db6 100644 --- 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(*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 96fd87aa46b..0e4b29986dd 100644 --- 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>(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( -- 2.39.5