From 9a840daa4a790e525392ef4ab98ceb0ce8543186 Mon Sep 17 00:00:00 2001 From: Samuel Just Date: Mon, 21 Jun 2021 17:10:29 -0700 Subject: [PATCH] crimson/os/seastore/cache: convert to use interruptible future Introduces InterruptedCache wrapper for now for components not yet converted. Signed-off-by: Samuel Just --- src/crimson/os/seastore/cache.cc | 38 +++-- src/crimson/os/seastore/cache.h | 159 ++++++++++++++---- .../lba_manager/btree/btree_lba_manager.h | 2 +- .../lba_manager/btree/lba_btree_node.h | 2 +- .../os/seastore/transaction_manager.cc | 2 +- src/crimson/os/seastore/transaction_manager.h | 2 +- .../crimson/seastore/test_seastore_cache.cc | 6 +- 7 files changed, 154 insertions(+), 57 deletions(-) diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index 19a874017c2..908600341a0 100644 --- a/src/crimson/os/seastore/cache.cc +++ b/src/crimson/os/seastore/cache.cc @@ -34,17 +34,19 @@ Cache::retire_extent_ret Cache::retire_extent_addr( if (auto ext = t.write_set.find_offset(addr); ext != t.write_set.end()) { DEBUGT("found {} in t.write_set", t, addr); t.add_to_retired_set(CachedExtentRef(&*ext)); - return retire_extent_ertr::now(); + return retire_extent_iertr::now(); } else if (auto iter = extents.find_offset(addr); iter != extents.end()) { auto ret = CachedExtentRef(&*iter); - return ret->wait_io().then([&t, ret=std::move(ret)]() mutable { + return trans_intr::make_interruptible( + ret->wait_io() + ).then_interruptible([&t, ret=std::move(ret)]() mutable { t.add_to_retired_set(ret); - return retire_extent_ertr::now(); + return retire_extent_iertr::now(); }); } else { t.add_to_retired_uncached(addr, length); - return retire_extent_ertr::now(); + return retire_extent_iertr::now(); } } @@ -415,15 +417,19 @@ void Cache::init() { Cache::mkfs_ertr::future<> Cache::mkfs(Transaction &t) { - return get_root(t).safe_then([this, &t](auto croot) { - duplicate_for_write(t, croot); - return mkfs_ertr::now(); - }).handle_error( - mkfs_ertr::pass_further{}, - crimson::ct_error::assert_all{ - "Invalid error in Cache::mkfs" - } - ); + return with_trans_intr( + t, + [this](auto &t) { + return get_root(t).si_then([this, &t](auto croot) { + duplicate_for_write(t, croot); + return base_ertr::now(); + }); + }).handle_error( + mkfs_ertr::pass_further{}, + crimson::ct_error::assert_all{ + "Invalid error in Cache::mkfs" + } + ); } Cache::close_ertr::future<> Cache::close() @@ -545,8 +551,7 @@ Cache::get_root_ret Cache::get_root(Transaction &t) LOG_PREFIX(Cache::get_root); if (t.root) { DEBUGT("root already on transaction {}", t, *t.root); - return get_root_ret( - get_root_ertr::ready_future_marker{}, + return get_root_iertr::make_ready_future( t.root); } else { auto ret = root; @@ -555,8 +560,7 @@ Cache::get_root_ret Cache::get_root(Transaction &t) DEBUGT("got root read {}", t, *ret); t.root = ret; t.add_to_read_set(ret); - return get_root_ret( - get_root_ertr::ready_future_marker{}, + return get_root_iertr::make_ready_future( ret); }); } diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index 7087013f9d0..fe0f1a13ca9 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -86,8 +86,8 @@ namespace crimson::os::seastore { class Cache { public: using base_ertr = crimson::errorator< - crimson::ct_error::input_output_error, - crimson::ct_error::eagain>; + crimson::ct_error::input_output_error>; + using base_iertr = trans_iertr; Cache(SegmentManager &segment_manager); ~Cache(); @@ -137,8 +137,8 @@ public: } /// Declare paddr retired in t - using retire_extent_ertr = base_ertr; - using retire_extent_ret = retire_extent_ertr::future<>; + using retire_extent_iertr = base_iertr; + using retire_extent_ret = base_iertr::future<>; retire_extent_ret retire_extent_addr( Transaction &t, paddr_t addr, extent_len_t length); @@ -147,8 +147,8 @@ public: * * returns ref to current root or t.root if modified in t */ - using get_root_ertr = base_ertr; - using get_root_ret = get_root_ertr::future; + using get_root_iertr = base_iertr; + using get_root_ret = get_root_iertr::future; get_root_ret get_root(Transaction &t); /** @@ -181,13 +181,10 @@ public: auto ret = TCachedExtentRef(static_cast(&*iter)); return ret->wait_io( ).then([ret=std::move(ret)]() mutable -> get_extent_ret { - if (ret->is_valid()) { - return get_extent_ret( - get_extent_ertr::ready_future_marker{}, - std::move(ret)); - } else { - return crimson::ct_error::eagain::make(); - } + assert(ret->is_valid()); + return get_extent_ret( + get_extent_ertr::ready_future_marker{}, + std::move(ret)); }); } else { auto ref = CachedExtent::make_cached_extent_ref( @@ -223,7 +220,10 @@ public: * * Returns extent at offset if in cache */ - seastar::future get_extent_if_cached( + using get_extent_if_cached_iertr = base_iertr; + using get_extent_if_cached_ret = + get_extent_if_cached_iertr::future; + get_extent_if_cached_ret get_extent_if_cached( Transaction &t, paddr_t offset) { return seastar::do_with( @@ -234,7 +234,9 @@ public: if (status == Transaction::get_extent_ret::PRESENT) { wait = ret->wait_io(); } - return wait.then([ret] { return std::move(ret); }); + return trans_intr::make_interruptible( + wait.then([ret] { return std::move(ret); }) + ); }); } @@ -248,28 +250,31 @@ public: * * t *must not* have retired offset */ + using get_extent_iertr = base_iertr; template - get_extent_ertr::future> get_extent( - Transaction &t, ///< [in,out] current transaction - paddr_t offset, ///< [in] starting addr - segment_off_t length ///< [in] length - ) { + get_extent_iertr::future> get_extent( + Transaction &t, + paddr_t offset, + segment_off_t length) { CachedExtentRef ret; auto result = t.get_extent(offset, &ret); if (result != Transaction::get_extent_ret::ABSENT) { assert(result != Transaction::get_extent_ret::RETIRED); - return get_extent_ertr::make_ready_future>( + return seastar::make_ready_future>( ret->cast()); } else { - return get_extent(offset, length).safe_then( + return trans_intr::make_interruptible( + get_extent(offset, length) + ).si_then( [&t](auto ref) mutable { t.add_to_read_set(ref); - return get_extent_ertr::make_ready_future>( + return get_extent_iertr::make_ready_future>( std::move(ref)); }); } } + /** * get_extent_by_type * @@ -283,7 +288,10 @@ public: segment_off_t length ///< [in] length ); - get_extent_ertr::future get_extent_by_type( + using get_extent_by_type_iertr = get_extent_iertr; + using get_extent_by_type_ret = get_extent_by_type_iertr::future< + CachedExtentRef>; + get_extent_by_type_ret get_extent_by_type( Transaction &t, extent_types_t type, paddr_t offset, @@ -292,12 +300,13 @@ public: CachedExtentRef ret; auto status = query_cache_for_extent(t, offset, &ret); if (status == Transaction::get_extent_ret::RETIRED) { - return get_extent_ertr::make_ready_future(); + return seastar::make_ready_future(); } else if (status == Transaction::get_extent_ret::PRESENT) { - return get_extent_ertr::make_ready_future(ret); + return seastar::make_ready_future(ret); } else { - return get_extent_by_type(type, offset, laddr, length - ).safe_then([=, &t](CachedExtentRef ret) { + return trans_intr::make_interruptible( + get_extent_by_type(type, offset, laddr, length) + ).si_then([=, &t](CachedExtentRef ret) { t.add_to_read_set(ret); return get_extent_ertr::make_ready_future( std::move(ret)); @@ -422,9 +431,8 @@ public: * after replay to allow lba_manager (or w/e) to read in any ancestor * blocks. */ - using init_cached_extents_ertr = crimson::errorator< - crimson::ct_error::input_output_error>; - using init_cached_extents_ret = replay_delta_ertr::future<>; + using init_cached_extents_iertr = base_iertr; + using init_cached_extents_ret = init_cached_extents_iertr::future<>; template init_cached_extents_ret init_cached_extents( Transaction &t, @@ -438,11 +446,11 @@ public: std::forward(f), std::move(dirty), [&t](auto &f, auto &refs) mutable { - return crimson::do_for_each( + return trans_intr::do_for_each( refs, [&t, &f](auto &e) { return f(t, e); }); - }).handle_error( - init_cached_extents_ertr::pass_further{}, + }).handle_error_interruptible( + init_cached_extents_iertr::pass_further{}, crimson::ct_error::assert_all{ "Invalid error in Cache::init_cached_extents" } @@ -588,4 +596,87 @@ private: }; using CacheRef = std::unique_ptr; +#define FORWARD(METHOD) \ + template \ + auto METHOD(Args&&... args) const { \ + return cache.METHOD(std::forward(args)...); \ + } + +#define PARAM_FORWARD(METHOD) \ + template \ + auto METHOD(Args&&... args) const { \ + return cache.METHOD(std::forward(args)...); \ + } + +#define INT_FORWARD(METHOD) \ + template \ + auto METHOD(Transaction &t, Args&&... args) const { \ + return with_trans_intr( \ + t, \ + [this](auto&&... args) { \ + return cache.METHOD(args...); \ + }, \ + std::forward(args)...); \ + } + +#define PARAM_INT_FORWARD(METHOD) \ + template \ + auto METHOD(Transaction &t, Args&&... args) const { \ + return with_trans_intr( \ + t, \ + [this](auto&&... args) { \ + return cache.METHOD(args...); \ + }, \ + std::forward(args)...); \ + } + +/// Temporary translator to non-interruptible futures +class InterruptedCache { + Cache &cache; +public: + InterruptedCache(Cache &cache) : cache(cache) {} + + FORWARD(init) + FORWARD(mkfs) + FORWARD(replay_delta) + FORWARD(init_cached_extents) + FORWARD(drop_from_cache) + FORWARD(create_transaction) + FORWARD(create_weak_transaction) + FORWARD(try_construct_record) + FORWARD(complete_commit) + FORWARD(close) + FORWARD(dump_contents) + FORWARD(get_next_dirty_extents) + FORWARD(update_extent_from_transaction) + INT_FORWARD(get_extent_if_cached) + FORWARD(get_oldest_dirty_from) + PARAM_FORWARD(alloc_new_extent) + FORWARD(alloc_new_extent_by_type) + INT_FORWARD(get_extent_by_type) + INT_FORWARD(retire_extent_addr) + FORWARD(retire_extent) + INT_FORWARD(get_root) + FORWARD(get_root_fast) + FORWARD(duplicate_for_write) + PARAM_INT_FORWARD(get_extent) +}; + +class InterruptedCacheRef { + std::unique_ptr ref; + InterruptedCache icache; +public: + template + InterruptedCacheRef(std::unique_ptr cache) + : ref(std::move(cache)), icache(*ref) {} + + auto &operator*() { + return icache; + } + + auto operator->() { + return &icache; + } +}; + } 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 b7864ed4920..91a1f99c022 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 @@ -126,7 +126,7 @@ public: ~BtreeLBAManager(); private: SegmentManager &segment_manager; - Cache &cache; + InterruptedCache cache; btree_pin_set_t pin_set; diff --git a/src/crimson/os/seastore/lba_manager/btree/lba_btree_node.h b/src/crimson/os/seastore/lba_manager/btree/lba_btree_node.h index 6ded255f1ba..9c2b498217b 100644 --- a/src/crimson/os/seastore/lba_manager/btree/lba_btree_node.h +++ b/src/crimson/os/seastore/lba_manager/btree/lba_btree_node.h @@ -16,7 +16,7 @@ namespace crimson::os::seastore::lba_manager::btree { using base_ertr = LBAManager::base_ertr; struct op_context_t { - Cache &cache; + InterruptedCache cache; btree_pin_set_t &pins; Transaction &trans; }; diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc index af5a8804c76..b063a58e86a 100644 --- a/src/crimson/os/seastore/transaction_manager.cc +++ b/src/crimson/os/seastore/transaction_manager.cc @@ -312,7 +312,7 @@ TransactionManager::get_extent_if_live_ret TransactionManager::get_extent_if_liv DEBUGT("type {}, addr {}, laddr {}, len {}", t, type, addr, laddr, len); return cache->get_extent_if_cached(t, addr - ).then([this, FNAME, &t, type, addr, laddr, len](auto extent) + ).safe_then([this, FNAME, &t, type, addr, laddr, len](auto extent) -> get_extent_if_live_ret { if (extent) { return get_extent_if_live_ret( diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index 2fb7df7922b..8f8ba5d9bdf 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -544,7 +544,7 @@ private: SegmentManager &segment_manager; SegmentCleanerRef segment_cleaner; - CacheRef cache; + InterruptedCacheRef cache; LBAManagerRef lba_manager; JournalRef journal; diff --git a/src/test/crimson/seastore/test_seastore_cache.cc b/src/test/crimson/seastore/test_seastore_cache.cc index bf58c8c454d..43b9ffa353e 100644 --- a/src/test/crimson/seastore/test_seastore_cache.cc +++ b/src/test/crimson/seastore/test_seastore_cache.cc @@ -21,13 +21,15 @@ namespace { struct cache_test_t : public seastar_test_suite_t { segment_manager::EphemeralSegmentManagerRef segment_manager; - Cache cache; + InterruptedCacheRef ref; + InterruptedCache &cache; paddr_t current{0, 0}; journal_seq_t seq; cache_test_t() : segment_manager(segment_manager::create_test_ephemeral()), - cache(*segment_manager) {} + ref(std::make_unique(*segment_manager)), + cache(*ref) {} seastar::future> submit_transaction( TransactionRef t) { -- 2.47.3