From: Samuel Just Date: Fri, 20 Aug 2021 08:03:22 +0000 (-0700) Subject: crimson/os/seastore: get_next_dirty_extents: record in transaction read set X-Git-Tag: v17.1.0~1003^2~8 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=cd044ad1cc992d6c27796250e518d47a645f84b1;p=ceph.git crimson/os/seastore: get_next_dirty_extents: record in transaction read set Record the extents in the read set after wait_io() as in get_extent. This should ensure that the interruptible_future machinery will handle the event that one of them gets invalidated prior to beging rewritten. Signed-off-by: Samuel Just --- diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index d30a73a1503..98e76346d21 100644 --- a/src/crimson/os/seastore/cache.cc +++ b/src/crimson/os/seastore/cache.cc @@ -1053,40 +1053,72 @@ Cache::replay_delta( } Cache::get_next_dirty_extents_ret Cache::get_next_dirty_extents( + Transaction &t, journal_seq_t seq, size_t max_bytes) { LOG_PREFIX(Cache::get_next_dirty_extents); - std::vector ret; + std::vector cand; size_t bytes_so_far = 0; for (auto i = dirty.begin(); i != dirty.end() && bytes_so_far < max_bytes; ++i) { - CachedExtentRef cand; if (i->get_dirty_from() != journal_seq_t() && i->get_dirty_from() < seq) { - DEBUG("next {}", *i); - if (!(ret.empty() || - ret.back()->get_dirty_from() <= i->get_dirty_from())) { - DEBUG("last {}, next {}", *ret.back(), *i); + DEBUGT("next {}", t, *i); + if (!(cand.empty() || + cand.back()->get_dirty_from() <= i->get_dirty_from())) { + ERRORT("last {}, next {}", t, *cand.back(), *i); } - assert(ret.empty() || ret.back()->get_dirty_from() <= i->get_dirty_from()); + assert(cand.empty() || cand.back()->get_dirty_from() <= i->get_dirty_from()); bytes_so_far += i->get_length(); - ret.push_back(&*i); + cand.push_back(&*i); + } else { break; } } return seastar::do_with( - std::move(ret), - [FNAME](auto &ret) { - return seastar::do_for_each( - ret, - [FNAME](auto &ext) { + std::move(cand), + decltype(cand)(), + [FNAME, this, &t](auto &cand, auto &ret) { + return trans_intr::do_for_each( + cand, + [FNAME, this, &t, &ret](auto &ext) { DEBUG("waiting on {}", *ext); - return ext->wait_io(); - }).then([&ret]() mutable { - return seastar::make_ready_future>( - std::move(ret)); + + return trans_intr::make_interruptible( + ext->wait_io() + ).then_interruptible([FNAME, this, ext, &t, &ret] { + if (!ext->is_valid()) { + invalidate(t, *ext); + return; + } + + CachedExtentRef on_transaction; + auto result = t.get_extent(ext->get_paddr(), &on_transaction); + if (result == Transaction::get_extent_ret::ABSENT) { + DEBUGT("{} absent on t", t, *ext); + t.add_to_read_set(ext); + if (ext->get_type() == extent_types_t::ROOT) { + if (t.root) { + assert(&*t.root == &*ext); + assert(0 == "t.root would have to already be in the read set"); + } else { + assert(&*ext == &*root); + t.root = root; + } + } + ret.push_back(ext); + } else if (result == Transaction::get_extent_ret::PRESENT) { + DEBUGT("{} present on t as {}", t, *ext, *on_transaction); + ret.push_back(on_transaction); + } else { + assert(result == Transaction::get_extent_ret::RETIRED); + DEBUGT("{} retired on t", t, *ext); + } + }); + }).then_interruptible([&ret] { + return std::move(ret); }); }); } diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index 0417dbff9f4..d610e0f52d8 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -520,6 +520,7 @@ public: return t.root; } else { t.add_to_read_set(extent); + t.root = extent->cast(); return extent; } } else { @@ -545,11 +546,17 @@ public: return out; } - /// returns extents with get_dirty_from() < seq - using get_next_dirty_extents_ertr = crimson::errorator<>; - using get_next_dirty_extents_ret = get_next_dirty_extents_ertr::future< + /** + * get_next_dirty_extents + * + * Returns extents with get_dirty_from() < seq and adds to read set of + * t. + */ + using get_next_dirty_extents_iertr = base_iertr; + using get_next_dirty_extents_ret = get_next_dirty_extents_iertr::future< std::vector>; get_next_dirty_extents_ret get_next_dirty_extents( + Transaction &t, journal_seq_t seq, size_t max_bytes); diff --git a/src/crimson/os/seastore/segment_cleaner.cc b/src/crimson/os/seastore/segment_cleaner.cc index f2e43273152..283b7eece89 100644 --- a/src/crimson/os/seastore/segment_cleaner.cc +++ b/src/crimson/os/seastore/segment_cleaner.cc @@ -2,6 +2,7 @@ // vim: ts=8 sw=2 smarttab #include "crimson/common/log.h" +#include "crimson/os/seastore/logging.h" #include "crimson/os/seastore/segment_cleaner.h" #include "crimson/os/seastore/transaction_manager.h" @@ -220,20 +221,19 @@ SegmentCleaner::rewrite_dirty_ret SegmentCleaner::rewrite_dirty( Transaction &t, journal_seq_t limit) { - return trans_intr::make_interruptible( - ecb->get_next_dirty_extents( - limit, - config.journal_rewrite_per_cycle) - ).then_interruptible([=, &t](auto dirty_list) { + LOG_PREFIX(SegmentCleaner::rewrite_dirty); + return ecb->get_next_dirty_extents( + t, + limit, + config.journal_rewrite_per_cycle + ).si_then([=, &t](auto dirty_list) { return seastar::do_with( std::move(dirty_list), - [this, &t](auto &dirty_list) { + [FNAME, this, &t](auto &dirty_list) { return trans_intr::do_for_each( dirty_list, - [this, &t](auto &e) { - logger().debug( - "SegmentCleaner::rewrite_dirty cleaning {}", - *e); + [FNAME, this, &t](auto &e) { + DEBUGT("cleaning {}", t, *e); return ecb->rewrite_extent(t, e); }); }); diff --git a/src/crimson/os/seastore/segment_cleaner.h b/src/crimson/os/seastore/segment_cleaner.h index 2ff0faccf21..474275d399f 100644 --- a/src/crimson/os/seastore/segment_cleaner.h +++ b/src/crimson/os/seastore/segment_cleaner.h @@ -264,20 +264,19 @@ public: ); } - /** - * get_next_dirty_extent - * - * returns all extents with dirty_from < bound - */ - using get_next_dirty_extents_iertr = crimson::errorator<>; + /// See Cache::get_next_dirty_extents + using get_next_dirty_extents_iertr = trans_iertr< + crimson::errorator< + crimson::ct_error::input_output_error> + >; using get_next_dirty_extents_ret = get_next_dirty_extents_iertr::future< std::vector>; virtual get_next_dirty_extents_ret get_next_dirty_extents( + Transaction &t, ///< [in] current transaction journal_seq_t bound,///< [in] return extents with dirty_from < bound size_t max_bytes ///< [in] return up to max_bytes of extents ) = 0; - using extent_mapping_ertr = crimson::errorator< crimson::ct_error::input_output_error, crimson::ct_error::eagain>; diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc index de87f053ba4..d6fc4a11bd3 100644 --- a/src/crimson/os/seastore/transaction_manager.cc +++ b/src/crimson/os/seastore/transaction_manager.cc @@ -269,10 +269,11 @@ TransactionManager::submit_transaction_direct( TransactionManager::get_next_dirty_extents_ret TransactionManager::get_next_dirty_extents( + Transaction &t, journal_seq_t seq, size_t max_bytes) { - return cache->get_next_dirty_extents(seq, max_bytes); + return cache->get_next_dirty_extents(t, seq, max_bytes); } TransactionManager::rewrite_extent_ret TransactionManager::rewrite_extent( diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index 0a8036967dd..db229b12e23 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -370,6 +370,7 @@ public: using SegmentCleaner::ExtentCallbackInterface::get_next_dirty_extents_ret; get_next_dirty_extents_ret get_next_dirty_extents( + Transaction &t, journal_seq_t seq, size_t max_bytes) final;