From 3b54d41c7887c163cc3c141f111341679210cfa3 Mon Sep 17 00:00:00 2001 From: Yingxin Cheng Date: Fri, 28 Jan 2022 20:12:10 +0800 Subject: [PATCH] crimson/os/seastore: initialize segment_info_t correctly when open/empty Previously, segment_info_t was not initialized for out-of-line segments when open, including journal_segment_seq and the legacy out_of_line. And the journal_segment_seq was not reset when empty. Signed-off-by: Yingxin Cheng --- .../os/seastore/extent_placement_manager.cc | 2 +- src/crimson/os/seastore/journal.cc | 6 +-- src/crimson/os/seastore/segment_cleaner.cc | 16 +++++-- src/crimson/os/seastore/segment_cleaner.h | 42 +++++++++---------- .../seastore/test_btree_lba_manager.cc | 2 +- .../crimson/seastore/test_seastore_journal.cc | 2 +- 6 files changed, 36 insertions(+), 34 deletions(-) diff --git a/src/crimson/os/seastore/extent_placement_manager.cc b/src/crimson/os/seastore/extent_placement_manager.cc index be35bb09b428f..8d4f632ab6d30 100644 --- a/src/crimson/os/seastore/extent_placement_manager.cc +++ b/src/crimson/os/seastore/extent_placement_manager.cc @@ -252,7 +252,7 @@ SegmentedAllocator::Writer::roll_segment(bool set_rolling) { } return segment_provider.get_segment( - segment_manager.get_device_id() + segment_manager.get_device_id(), OOL_SEG_SEQ ).safe_then([this](auto segment) { return segment_manager.open(segment); }).safe_then([this](auto segref) { diff --git a/src/crimson/os/seastore/journal.cc b/src/crimson/os/seastore/journal.cc index b742bec0a026e..dd2bd28ca5628 100644 --- a/src/crimson/os/seastore/journal.cc +++ b/src/crimson/os/seastore/journal.cc @@ -385,7 +385,8 @@ Journal::JournalSegmentManager::roll() current_journal_segment->close() : Segment::close_ertr::now() ).safe_then([this] { - return segment_provider->get_segment(get_device_id()); + return segment_provider->get_segment( + get_device_id(), next_journal_segment_seq); }).safe_then([this](auto segment) { return segment_manager.open(segment); }).safe_then([this](auto sref) { @@ -395,9 +396,6 @@ Journal::JournalSegmentManager::roll() if (old_segment_id != NULL_SEG_ID) { segment_provider->close_segment(old_segment_id); } - segment_provider->set_journal_segment( - current_journal_segment->get_segment_id(), - get_segment_seq()); }).handle_error( roll_ertr::pass_further{}, crimson::ct_error::assert_all{ diff --git a/src/crimson/os/seastore/segment_cleaner.cc b/src/crimson/os/seastore/segment_cleaner.cc index e0ccd19026ad5..846bb72b24c4c 100644 --- a/src/crimson/os/seastore/segment_cleaner.cc +++ b/src/crimson/os/seastore/segment_cleaner.cc @@ -17,14 +17,17 @@ SET_SUBSYS(seastore_cleaner); namespace crimson::os::seastore { -void segment_info_set_t::segment_info_t::set_open() { +void segment_info_set_t::segment_info_t::set_open(segment_seq_t seq) { assert(state == Segment::segment_state_t::EMPTY); + assert(segment_seq_to_type(seq) != segment_type_t::NULL_SEG); state = Segment::segment_state_t::OPEN; + journal_segment_seq = seq; } void segment_info_set_t::segment_info_t::set_empty() { assert(state == Segment::segment_state_t::CLOSED); state = Segment::segment_state_t::EMPTY; + journal_segment_seq = NULL_SEG_SEQ; } void segment_info_set_t::segment_info_t::set_closed() { @@ -204,16 +207,19 @@ void SegmentCleaner::register_metrics() }); } -SegmentCleaner::get_segment_ret SegmentCleaner::get_segment(device_id_t id) +SegmentCleaner::get_segment_ret SegmentCleaner::get_segment( + device_id_t id, segment_seq_t seq) { + assert(segment_seq_to_type(seq) != segment_type_t::NULL_SEG); for (auto it = segments.device_begin(id); it != segments.device_end(id); ++it) { auto id = it->first; auto& segment_info = it->second; if (segment_info.is_empty()) { - mark_open(id); - logger().debug("{}: returning segment {}", __func__, id); + logger().debug("{}: returning segment {} {}", + __func__, id, segment_seq_printer_t{seq}); + mark_open(id, seq); return get_segment_ret( get_segment_ertr::ready_future_marker{}, id); @@ -262,6 +268,8 @@ void SegmentCleaner::update_journal_tail_committed(journal_seq_t committed) void SegmentCleaner::close_segment(segment_id_t segment) { + ceph_assert(segment_seq_to_type(segments[segment].journal_segment_seq) != + segment_type_t::NULL_SEG); mark_closed(segment); } diff --git a/src/crimson/os/seastore/segment_cleaner.h b/src/crimson/os/seastore/segment_cleaner.h index 3d0168dd38096..e8b917d7533b5 100644 --- a/src/crimson/os/seastore/segment_cleaner.h +++ b/src/crimson/os/seastore/segment_cleaner.h @@ -61,7 +61,7 @@ class segment_info_set_t { return segment_seq_to_type(journal_segment_seq); } - void set_open(); + void set_open(segment_seq_t); void set_empty(); void set_closed(); @@ -275,15 +275,13 @@ public: using get_segment_ertr = crimson::errorator< crimson::ct_error::input_output_error>; using get_segment_ret = get_segment_ertr::future; - virtual get_segment_ret get_segment(device_id_t id) = 0; + virtual get_segment_ret get_segment( + device_id_t id, segment_seq_t seq) = 0; virtual void close_segment(segment_id_t) {} - virtual void set_journal_segment( - segment_id_t segment, - segment_seq_t seq) {} - virtual journal_seq_t get_journal_tail_target() const = 0; + virtual void update_journal_tail_committed(journal_seq_t tail_committed) = 0; virtual segment_seq_t get_seq(segment_id_t id) { return 0; } @@ -717,22 +715,11 @@ public: using init_segments_ret = init_segments_ertr::future; init_segments_ret init_segments(); - get_segment_ret get_segment(device_id_t id) final; + get_segment_ret get_segment( + device_id_t id, segment_seq_t seq) final; void close_segment(segment_id_t segment) final; - void set_journal_segment( - segment_id_t segment, segment_seq_t seq) final { - assert(segment.device_id() == - segments[segment.device_id()]->device_id); - assert(segment.device_segment_id() < - segments[segment.device_id()]->num_segments); - segments[segment].journal_segment_seq = seq; - segments[segment].out_of_line = false; - segments.new_journal_segment(); - assert(segments[segment].is_open()); - } - journal_seq_t get_journal_tail_target() const final { return journal_tail_target; } @@ -1263,6 +1250,7 @@ private: "SegmentCleaner::init_mark_segment_closed: segment {}, seq {}", segment, segment_seq_printer_t{seq}); + ceph_assert(segment_seq_to_type(seq) != segment_type_t::NULL_SEG); mark_closed(segment); segments[segment].journal_segment_seq = seq; auto s_type = segments[segment].get_type(); @@ -1311,6 +1299,7 @@ private: space_tracker->dump_usage(segment); assert(space_tracker->get_usage(segment) == 0); } + auto s_type = segment_info.get_type(); segment_info.set_empty(); stats.empty_segments++; crimson::get_logger(ceph_subsys_seastore_cleaner @@ -1323,7 +1312,6 @@ private: should_block_on_gc(), get_projected_available_ratio(), get_projected_reclaim_ratio()); - auto s_type = segment_info.get_type(); ceph_assert(s_type != segment_type_t::NULL_SEG); if (s_type == segment_type_t::JOURNAL) { segments.journal_segment_emptied(); @@ -1331,7 +1319,7 @@ private: maybe_wake_gc_blocked_io(); } - void mark_open(segment_id_t segment) { + void mark_open(segment_id_t segment, segment_seq_t seq) { assert(segment.device_id() == segments[segment.device_id()]->device_id); assert(segment.device_segment_id() < @@ -1339,14 +1327,22 @@ private: assert(segments[segment].is_empty()); assert(segments.get_empty_segments(segment.device_id()) > 0); segments.segment_opened(segment); - segments[segment].set_open(); + auto& segment_info = segments[segment]; + segment_info.set_open(seq); + + auto s_type = segment_info.get_type(); + ceph_assert(s_type != segment_type_t::NULL_SEG); + if (s_type == segment_type_t::JOURNAL) { + segments.new_journal_segment(); + } assert(stats.empty_segments > 0); stats.empty_segments--; crimson::get_logger(ceph_subsys_seastore_cleaner - ).info("mark open: {}, empty_segments {}" + ).info("mark open: {} {}, empty_segments {}" ", opened_segments {}, should_block_on_gc {}" ", projected_avail_ratio {}, projected_reclaim_ratio {}", segment, + segment_seq_printer_t{seq}, stats.empty_segments, segments.get_opened_segments(), should_block_on_gc(), diff --git a/src/test/crimson/seastore/test_btree_lba_manager.cc b/src/test/crimson/seastore/test_btree_lba_manager.cc index 7bf2e01a4ad38..9e2fe06692cbf 100644 --- a/src/test/crimson/seastore/test_btree_lba_manager.cc +++ b/src/test/crimson/seastore/test_btree_lba_manager.cc @@ -43,7 +43,7 @@ struct btree_test_base : void update_segment_avail_bytes(paddr_t offset) final {} - get_segment_ret get_segment(device_id_t id) final { + get_segment_ret get_segment(device_id_t id, segment_seq_t seq) final { auto ret = next; next = segment_id_t{ next.device_id(), diff --git a/src/test/crimson/seastore/test_seastore_journal.cc b/src/test/crimson/seastore/test_seastore_journal.cc index 9fc4421c4065a..f1271e2c8229f 100644 --- a/src/test/crimson/seastore/test_seastore_journal.cc +++ b/src/test/crimson/seastore/test_seastore_journal.cc @@ -84,7 +84,7 @@ struct journal_test_t : seastar_test_suite_t, SegmentProvider { void update_segment_avail_bytes(paddr_t offset) final {} - get_segment_ret get_segment(device_id_t id) final { + get_segment_ret get_segment(device_id_t id, segment_seq_t seq) final { auto ret = next; next = segment_id_t{ next.device_id(), -- 2.39.5