From 64a8bfbd48da5b8778741ddea51a0cbbf5dfa978 Mon Sep 17 00:00:00 2001 From: Yingxin Cheng Date: Sun, 7 Aug 2022 21:08:49 +0800 Subject: [PATCH] crimson/os/seastore: be explicit about detecting out-dated delta using segment info Signed-off-by: Yingxin Cheng --- src/crimson/os/seastore/cache.cc | 35 +++++++++++++++++-- src/crimson/os/seastore/cache.h | 16 +++++++++ .../os/seastore/journal/segmented_journal.cc | 30 +--------------- .../os/seastore/transaction_manager.cc | 4 +++ 4 files changed, 53 insertions(+), 32 deletions(-) diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index e1678dc7f0986..107574cb7d1c8 100644 --- a/src/crimson/os/seastore/cache.cc +++ b/src/crimson/os/seastore/cache.cc @@ -1102,13 +1102,16 @@ record_t Cache::prepare_record( } else { auto sseq = NULL_SEG_SEQ; auto stype = segment_type_t::NULL_SEG; - if (cleaner != nullptr && i->get_paddr().get_addr_type() == - paddr_types_t::SEGMENT) { + + // FIXME: This is specific to the segmented implementation + if (segment_provider != nullptr && + i->get_paddr().get_addr_type() == paddr_types_t::SEGMENT) { auto sid = i->get_paddr().as_seg_paddr().get_segment_id(); - auto &sinfo = cleaner->get_seg_info(sid); + auto &sinfo = segment_provider->get_seg_info(sid); sseq = sinfo.seq; stype = sinfo.type; } + record.push_back( delta_info_t{ i->get_type(), @@ -1636,6 +1639,32 @@ Cache::replay_delta( assert(alloc_tail != JOURNAL_SEQ_NULL); ceph_assert(modify_time != NULL_TIME); + // FIXME: This is specific to the segmented implementation + /* The journal may validly contain deltas for extents in + * since released segments. We can detect those cases by + * checking whether the segment in question currently has a + * sequence number > the current journal segment seq. We can + * safetly skip these deltas because the extent must already + * have been rewritten. + */ + if (segment_provider != nullptr && + delta.paddr != P_ADDR_NULL && + delta.paddr.get_addr_type() == paddr_types_t::SEGMENT) { + auto& seg_addr = delta.paddr.as_seg_paddr(); + auto& seg_info = segment_provider->get_seg_info(seg_addr.get_segment_id()); + auto delta_paddr_segment_seq = seg_info.seq; + auto delta_paddr_segment_type = seg_info.type; + if (delta_paddr_segment_seq != delta.ext_seq || + delta_paddr_segment_type != delta.seg_type) { + DEBUG("delta is obsolete, delta_paddr_segment_seq={}," + " delta_paddr_segment_type={} -- {}", + segment_seq_printer_t{delta_paddr_segment_seq}, + delta_paddr_segment_type, + delta); + return replay_delta_ertr::make_ready_future(false); + } + } + if (delta.type == extent_types_t::JOURNAL_TAIL) { // this delta should have been dealt with during segment cleaner mounting return replay_delta_ertr::make_ready_future(false); diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index 4a4a280bcde72..c57a0b8e96c34 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -27,6 +27,7 @@ namespace crimson::os::seastore { class BackrefManager; class AsyncCleaner; +class SegmentProvider; struct backref_entry_t { backref_entry_t( @@ -679,6 +680,18 @@ public: CachedExtentRef i ///< [in] ref to existing extent ); + /** + * set_segment_provider + * + * Set to provide segment information to help identify out-dated delta. + * + * FIXME: This is specific to the segmented implementation + */ + void set_segment_provider(SegmentProvider &sp) { + assert(segment_provider == nullptr); + segment_provider = &sp; + } + /** * prepare_record * @@ -968,6 +981,9 @@ private: journal_seq_t last_commit = JOURNAL_SEQ_MIN; + // FIXME: This is specific to the segmented implementation + SegmentProvider *segment_provider = nullptr; + /** * dirty * diff --git a/src/crimson/os/seastore/journal/segmented_journal.cc b/src/crimson/os/seastore/journal/segmented_journal.cc index 82eadf80fbae0..19008138df4ba 100644 --- a/src/crimson/os/seastore/journal/segmented_journal.cc +++ b/src/crimson/os/seastore/journal/segmented_journal.cc @@ -234,7 +234,7 @@ SegmentedJournal::replay_segment( return seastar::do_with( scan_valid_records_cursor(seq), SegmentManagerGroup::found_record_handler_t( - [s_type=header.type, &handler, this, &stats]( + [&handler, this, &stats]( record_locator_t locator, const record_group_header_t& header, const bufferlist& mdbuf) @@ -254,7 +254,6 @@ SegmentedJournal::replay_segment( return seastar::do_with( std::move(*maybe_record_deltas_list), [write_result=locator.write_result, - s_type, this, FNAME, &handler, @@ -263,7 +262,6 @@ SegmentedJournal::replay_segment( return crimson::do_for_each( record_deltas_list, [write_result, - s_type, this, FNAME, &handler, @@ -280,38 +278,12 @@ SegmentedJournal::replay_segment( return crimson::do_for_each( record_deltas.deltas, [locator, - s_type, this, - FNAME, &handler, &stats](auto &p) { auto& modify_time = p.first; auto& delta = p.second; - /* The journal may validly contain deltas for extents in - * since released segments. We can detect those cases by - * checking whether the segment in question currently has a - * sequence number > the current journal segment seq. We can - * safetly skip these deltas because the extent must already - * have been rewritten. - */ - if (delta.paddr != P_ADDR_NULL) { - auto& seg_addr = delta.paddr.as_seg_paddr(); - auto& seg_info = segment_provider.get_seg_info(seg_addr.get_segment_id()); - auto delta_paddr_segment_seq = seg_info.seq; - auto delta_paddr_segment_type = seg_info.type; - if (s_type == segment_type_t::NULL_SEG || - (delta_paddr_segment_seq != delta.ext_seq || - delta_paddr_segment_type != delta.seg_type)) { - SUBDEBUG(seastore_cache, - "delta is obsolete, delta_paddr_segment_seq={}," - " delta_paddr_segment_type={} -- {}", - segment_seq_printer_t{delta_paddr_segment_seq}, - delta_paddr_segment_type, - delta); - return replay_ertr::now(); - } - } return handler( locator, delta, diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc index 806e646daff8a..d97c30a453951 100644 --- a/src/crimson/os/seastore/transaction_manager.cc +++ b/src/crimson/os/seastore/transaction_manager.cc @@ -654,6 +654,10 @@ TransactionManagerRef make_transaction_manager( *backref_manager, cleaner_is_detailed); + if (primary_device->get_device_type() == device_type_t::SEGMENTED) { + cache->set_segment_provider(*async_cleaner); + } + auto p_device_type = primary_device->get_device_type(); JournalRef journal; if (p_device_type == device_type_t::SEGMENTED) { -- 2.39.5