From c62bd2c8a60b30927864c238b9620f3adf001c4f Mon Sep 17 00:00:00 2001 From: Yingxin Cheng Date: Mon, 27 May 2024 10:16:41 +0800 Subject: [PATCH] crimson/os/seastore/async_cleaner: fix incorrect get_num_rolls() The number of journal segments should not be based on the committed journal_head. Otherwise, if a new journal segment is just opened and the committed journal_head hasn't been updated, the result will be wrong. This causes ceph_assert(get_segments_reclaimable() == 0) in SegmentCleaner::get_next_reclaim_segment(). Signed-off-by: Yingxin Cheng (cherry picked from commit 5ae59cf52159bdc1ad3637da8654524cb7b4b4b6) --- src/crimson/os/seastore/async_cleaner.cc | 16 ++++++++++++++++ src/crimson/os/seastore/async_cleaner.h | 18 +++++++++++++++++- .../crimson/seastore/test_btree_lba_manager.cc | 6 ++++++ src/test/crimson/seastore/test_cbjournal.cc | 8 +++++++- .../crimson/seastore/test_seastore_journal.cc | 6 ++++++ 5 files changed, 52 insertions(+), 2 deletions(-) diff --git a/src/crimson/os/seastore/async_cleaner.cc b/src/crimson/os/seastore/async_cleaner.cc index aa6fcd8a95d6c..a9d22b1269caa 100644 --- a/src/crimson/os/seastore/async_cleaner.cc +++ b/src/crimson/os/seastore/async_cleaner.cc @@ -443,6 +443,18 @@ void JournalTrimmerImpl::set_journal_head(journal_seq_t head) background_callback->maybe_wake_background(); } +void JournalTrimmerImpl::set_journal_head_sequence( + segment_seq_t seq) +{ + if (journal_head != JOURNAL_SEQ_NULL) { + ceph_assert(seq == journal_head.segment_seq + 1); + } + if (journal_head_seq != NULL_SEG_SEQ) { + ceph_assert(seq == journal_head_seq + 1); + } + journal_head_seq = seq; +} + void JournalTrimmerImpl::update_journal_tails( journal_seq_t dirty_tail, journal_seq_t alloc_tail) @@ -976,6 +988,10 @@ segment_id_t SegmentCleaner::allocate_segment( if (segment_info.is_empty()) { auto old_usage = calc_utilization(seg_id); segments.mark_open(seg_id, seq, type, category, generation); + if (type == segment_type_t::JOURNAL) { + assert(trimmer != nullptr); + trimmer->set_journal_head_sequence(seq); + } background_callback->maybe_wake_background(); auto new_usage = calc_utilization(seg_id); 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 1c94745d820c2..2f8ae53022e86 100644 --- a/src/crimson/os/seastore/async_cleaner.h +++ b/src/crimson/os/seastore/async_cleaner.h @@ -416,6 +416,12 @@ public: // set the committed journal head virtual void set_journal_head(journal_seq_t) = 0; + // get the opened journal head sequence + virtual segment_seq_t get_journal_head_sequence() const = 0; + + // set the opened journal head sequence + virtual void set_journal_head_sequence(segment_seq_t) = 0; + // get the committed journal dirty tail virtual journal_seq_t get_dirty_tail() const = 0; @@ -455,7 +461,9 @@ public: } assert(get_journal_head().segment_seq >= get_journal_tail().segment_seq); - return get_journal_head().segment_seq + 1 - + assert(get_journal_head_sequence() >= + get_journal_head().segment_seq); + return get_journal_head_sequence() + 1 - get_journal_tail().segment_seq; } }; @@ -510,6 +518,12 @@ public: void set_journal_head(journal_seq_t) final; + segment_seq_t get_journal_head_sequence() const final { + return journal_head_seq; + } + + void set_journal_head_sequence(segment_seq_t) final; + journal_seq_t get_dirty_tail() const final { return journal_dirty_tail; } @@ -539,6 +553,7 @@ public: } void reset() { + journal_head_seq = NULL_SEG_SEQ; journal_head = JOURNAL_SEQ_NULL; journal_dirty_tail = JOURNAL_SEQ_NULL; journal_alloc_tail = JOURNAL_SEQ_NULL; @@ -618,6 +633,7 @@ private: device_off_t roll_start; device_off_t roll_size; + segment_seq_t journal_head_seq = NULL_SEG_SEQ; journal_seq_t journal_head; journal_seq_t journal_dirty_tail; journal_seq_t journal_alloc_tail; diff --git a/src/test/crimson/seastore/test_btree_lba_manager.cc b/src/test/crimson/seastore/test_btree_lba_manager.cc index 977b8c3865069..d584d6c22b68d 100644 --- a/src/test/crimson/seastore/test_btree_lba_manager.cc +++ b/src/test/crimson/seastore/test_btree_lba_manager.cc @@ -55,6 +55,12 @@ struct btree_test_base : void set_journal_head(journal_seq_t) final {} + segment_seq_t get_journal_head_sequence() const final { + return NULL_SEG_SEQ; + } + + void set_journal_head_sequence(segment_seq_t) final {} + journal_seq_t get_dirty_tail() const final { return dummy_tail; } journal_seq_t get_alloc_tail() const final { return dummy_tail; } diff --git a/src/test/crimson/seastore/test_cbjournal.cc b/src/test/crimson/seastore/test_cbjournal.cc index bacb3cd2f78ec..3c7007a5024d8 100644 --- a/src/test/crimson/seastore/test_cbjournal.cc +++ b/src/test/crimson/seastore/test_cbjournal.cc @@ -147,10 +147,14 @@ struct cbjournal_test_t : public seastar_test_suite_t, JournalTrimmer /* * JournalTrimmer interfaces */ - journal_seq_t get_journal_head() const { + journal_seq_t get_journal_head() const final { return JOURNAL_SEQ_NULL; } + segment_seq_t get_journal_head_sequence() const final { + return NULL_SEG_SEQ; + } + journal_seq_t get_dirty_tail() const final { return JOURNAL_SEQ_NULL; } @@ -161,6 +165,8 @@ struct cbjournal_test_t : public seastar_test_suite_t, JournalTrimmer void set_journal_head(journal_seq_t head) final {} + void set_journal_head_sequence(segment_seq_t) final {} + void update_journal_tails( journal_seq_t dirty_tail, journal_seq_t alloc_tail) final {} diff --git a/src/test/crimson/seastore/test_seastore_journal.cc b/src/test/crimson/seastore/test_seastore_journal.cc index ddd894349d21e..e275213047b6c 100644 --- a/src/test/crimson/seastore/test_seastore_journal.cc +++ b/src/test/crimson/seastore/test_seastore_journal.cc @@ -94,6 +94,12 @@ struct journal_test_t : seastar_test_suite_t, SegmentProvider, JournalTrimmer { void set_journal_head(journal_seq_t) final {} + segment_seq_t get_journal_head_sequence() const final { + return NULL_SEG_SEQ; + } + + void set_journal_head_sequence(segment_seq_t) final {} + journal_seq_t get_dirty_tail() const final { return dummy_tail; } journal_seq_t get_alloc_tail() const final { return dummy_tail; } -- 2.39.5