From ec90bcbfec5a26563ae2b9abc1d1d31d901c0a27 Mon Sep 17 00:00:00 2001 From: zhscn Date: Wed, 29 Jun 2022 20:26:54 +0800 Subject: [PATCH] crimson/os/seastore: update TransactionManager::get_extent_if_live TransactionManager::get_extents_if_live should return a list of extents that are located in range paddr~len. When SegmentCleaner invokes get_extents_if_live, the target extent may have been split into multiple pieces by other transaction, so only search the paddr as key will lose other pieces need to be rewritten. Signed-off-by: Zhang Song --- src/crimson/os/seastore/async_cleaner.cc | 18 ++- src/crimson/os/seastore/async_cleaner.h | 8 +- .../os/seastore/transaction_manager.cc | 109 ++++++++++-------- src/crimson/os/seastore/transaction_manager.h | 4 +- 4 files changed, 76 insertions(+), 63 deletions(-) diff --git a/src/crimson/os/seastore/async_cleaner.cc b/src/crimson/os/seastore/async_cleaner.cc index 0abdf926d65..1a5b3df1479 100644 --- a/src/crimson/os/seastore/async_cleaner.cc +++ b/src/crimson/os/seastore/async_cleaner.cc @@ -919,17 +919,19 @@ AsyncCleaner::_retrieve_live_extents( ent.type, ent.paddr, ent.len); - return ecb->get_extent_if_live( + return ecb->get_extents_if_live( t, ent.type, ent.paddr, ent.laddr, ent.len - ).si_then([this, FNAME, &extents, &ent, &seq, &t](auto ext) { - if (!ext) { + ).si_then([this, FNAME, &extents, &ent, &seq, &t](auto list) { + if (list.empty()) { DEBUGT("addr {} dead, skipping", t, ent.paddr); auto backref = backref_manager.get_cached_backref_removal(ent.paddr); if (seq == JOURNAL_SEQ_NULL || seq < backref.seq) { seq = backref.seq; } } else { - extents.emplace_back(std::move(ext)); + for (auto &e : list) { + extents.emplace_back(std::move(e)); + } } return ExtentCallbackInterface::rewrite_extent_iertr::now(); }); @@ -1043,7 +1045,8 @@ AsyncCleaner::gc_reclaim_space_ret AsyncCleaner::gc_reclaim_space() DEBUGT("del_backref {}~{} {} {}", t, del_backref.paddr, del_backref.len, del_backref.type, del_backref.seq); auto it = backrefs.find(del_backref.paddr); - if (it != backrefs.end()) + if (it != backrefs.end() && + it->len == del_backref.len) backrefs.erase(it); if (seq == JOURNAL_SEQ_NULL || (del_backref.seq != JOURNAL_SEQ_NULL && del_backref.seq > seq)) @@ -1308,7 +1311,10 @@ AsyncCleaner::maybe_release_segment(Transaction &t) return sm_group->release_segment(to_release ).safe_then([this, FNAME, &t, to_release] { auto old_usage = calc_utilization(to_release); - ceph_assert(old_usage == 0); + if(old_usage != 0) { + ERRORT("segment {} old_usage {} != 0", t, to_release, old_usage); + ceph_abort(); + } segments.mark_empty(to_release); auto new_usage = calc_utilization(to_release); adjust_segment_util(old_usage, new_usage); diff --git a/src/crimson/os/seastore/async_cleaner.h b/src/crimson/os/seastore/async_cleaner.h index 2d7a73890cb..8aef05a09bc 100644 --- a/src/crimson/os/seastore/async_cleaner.h +++ b/src/crimson/os/seastore/async_cleaner.h @@ -643,10 +643,10 @@ public: * See TransactionManager::get_extent_if_live and * LBAManager::get_physical_extent_if_live. */ - using get_extent_if_live_iertr = extent_mapping_iertr; - using get_extent_if_live_ret = get_extent_if_live_iertr::future< - CachedExtentRef>; - virtual get_extent_if_live_ret get_extent_if_live( + using get_extents_if_live_iertr = extent_mapping_iertr; + using get_extents_if_live_ret = get_extents_if_live_iertr::future< + std::list>; + virtual get_extents_if_live_ret get_extents_if_live( Transaction &t, extent_types_t type, paddr_t addr, diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc index 14813a66848..12ac86c7911 100644 --- a/src/crimson/os/seastore/transaction_manager.cc +++ b/src/crimson/os/seastore/transaction_manager.cc @@ -557,7 +557,7 @@ TransactionManager::rewrite_extent_ret TransactionManager::rewrite_extent( }); } -TransactionManager::get_extent_if_live_ret TransactionManager::get_extent_if_live( +TransactionManager::get_extents_if_live_ret TransactionManager::get_extents_if_live( Transaction &t, extent_types_t type, paddr_t addr, @@ -569,64 +569,67 @@ TransactionManager::get_extent_if_live_ret TransactionManager::get_extent_if_liv return cache->get_extent_if_cached(t, addr, type ).si_then([this, FNAME, &t, type, addr, laddr, len](auto extent) - -> get_extent_if_live_ret { - if (extent) { + -> get_extents_if_live_ret { + if (extent && extent->get_length() == (extent_len_t)len) { DEBUGT("{} {}~{} {} is live in cache -- {}", t, type, laddr, len, addr, *extent); - return get_extent_if_live_ret ( + std::list res; + res.emplace_back(std::move(extent)); + return get_extents_if_live_ret( interruptible::ready_future_marker{}, - extent); + res); } if (is_logical_type(type)) { using inner_ret = LBAManager::get_mapping_iertr::future; - return lba_manager->get_mapping( + return lba_manager->get_mappings( t, - laddr).si_then([=, &t] (LBAPinRef pin) -> inner_ret { - ceph_assert(pin->get_key() == laddr); - if (pin->get_val() == addr) { - if (pin->get_length() != (extent_len_t)len) { - ERRORT( - "Invalid pin {}~{} {} found for " - "extent {} {}~{} {}", - t, - pin->get_key(), - pin->get_length(), - pin->get_val(), - type, - laddr, - len, - addr); - ceph_abort(); - } - return cache->get_extent_by_type( - t, - type, - addr, - laddr, - len, - [this, pin=std::move(pin)](CachedExtent &extent) mutable { - auto lref = extent.cast(); - assert(!lref->has_pin()); - assert(!lref->has_been_invalidated()); - assert(!pin->has_been_invalidated()); - lref->set_pin(std::move(pin)); - lba_manager->add_pin(lref->get_pin()); - }).si_then([=, &t](auto ret) {; - DEBUGT("{} {}~{} {} is live as logical extent -- {}", - t, type, laddr, len, addr, extent); - return ret; + laddr, + len + ).si_then([=, &t](lba_pin_list_t pin_list) { + return seastar::do_with( + std::list(), + std::move(pin_list), + [=, &t](std::list &list, lba_pin_list_t &pin_list) { + auto &seg_addr = addr.as_seg_paddr(); + auto seg_addr_id = seg_addr.get_segment_id(); + return trans_intr::parallel_for_each(pin_list, [=, &seg_addr, &list, &t](LBAPinRef &pin) -> + Cache::get_extent_iertr::future<> { + auto pin_laddr = pin->get_key(); + auto pin_paddr = pin->get_val(); + auto pin_len = pin->get_length(); + + auto &pin_seg_addr = pin_paddr.as_seg_paddr(); + auto pin_seg_addr_id = pin_seg_addr.get_segment_id(); + + if (pin_seg_addr_id != seg_addr_id || + pin_paddr < seg_addr || + pin_paddr.add_offset(pin_len) > seg_addr.add_offset(len)) { + return seastar::now(); + } + return cache->get_extent_by_type( + t, type, pin_paddr, pin_laddr, pin_len, + [this, pin=std::move(pin)](CachedExtent &extent) mutable { + auto lref = extent.cast(); + assert(!lref->has_pin()); + assert(!lref->has_been_invalidated()); + assert(!pin->has_been_invalidated()); + lref->set_pin(std::move(pin)); + lba_manager->add_pin(lref->get_pin()); + } + ).si_then([=, &list](auto ret) { + list.emplace_back(std::move(ret)); + return seastar::now(); }); - } else { - DEBUGT("{} {}~{} {} is not live as logical extent", - t, type, laddr, len, addr); - return inner_ret( - interruptible::ready_future_marker{}, - CachedExtentRef()); - } - }).handle_error_interruptible(crimson::ct_error::enoent::handle([] { - return CachedExtentRef(); - }), crimson::ct_error::pass_further_all{}); + }).si_then([&list] { + return get_extents_if_live_ret( + interruptible::ready_future_marker{}, + std::move(list)); + }); + }); + }).handle_error_interruptible(crimson::ct_error::enoent::handle([] { + return std::list(); + }), crimson::ct_error::pass_further_all{}); } else { return lba_manager->get_physical_extent_if_live( t, @@ -642,7 +645,11 @@ TransactionManager::get_extent_if_live_ret TransactionManager::get_extent_if_liv DEBUGT("{} {}~{} {} is not live as physical extent", t, type, laddr, len, addr); } - return ret; + std::list res; + res.emplace_back(std::move(ret)); + return get_extents_if_live_ret( + interruptible::ready_future_marker{}, + std::move(res)); }); } }); diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index 823b1abcef9..2bfb7c69e9a 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -508,8 +508,8 @@ public: reclaim_gen_t target_generation, sea_time_point modify_time) final; - using AsyncCleaner::ExtentCallbackInterface::get_extent_if_live_ret; - get_extent_if_live_ret get_extent_if_live( + using AsyncCleaner::ExtentCallbackInterface::get_extents_if_live_ret; + get_extents_if_live_ret get_extents_if_live( Transaction &t, extent_types_t type, paddr_t addr, -- 2.39.5