From 3b62cbc1447131e596728ad4b79732492c088d63 Mon Sep 17 00:00:00 2001 From: Yingxin Cheng Date: Sun, 7 Aug 2022 20:12:07 +0800 Subject: [PATCH] crimson/os/seastore: decouple segment release from transaction manager Signed-off-by: Yingxin Cheng --- src/crimson/os/seastore/async_cleaner.cc | 59 +++++++------------ src/crimson/os/seastore/async_cleaner.h | 3 - src/crimson/os/seastore/transaction.h | 13 ---- .../os/seastore/transaction_manager.cc | 3 - 4 files changed, 22 insertions(+), 56 deletions(-) diff --git a/src/crimson/os/seastore/async_cleaner.cc b/src/crimson/os/seastore/async_cleaner.cc index 35b8b0ed7e5..70e590ec240 100644 --- a/src/crimson/os/seastore/async_cleaner.cc +++ b/src/crimson/os/seastore/async_cleaner.cc @@ -1013,9 +1013,6 @@ AsyncCleaner::gc_reclaim_space_ret AsyncCleaner::gc_reclaim_space() }); }); }).si_then([this, &t] { - if (reclaim_state->is_complete()) { - t.mark_segment_to_release(reclaim_state->get_segment_id()); - } return ecb->submit_transaction_direct(t); }); }); @@ -1028,14 +1025,32 @@ AsyncCleaner::gc_reclaim_space_ret AsyncCleaner::gc_reclaim_space() auto d = seastar::lowres_system_clock::now() - start; DEBUG("duration: {}, pavail_ratio before: {}, repeats: {}", d, pavail_ratio, runs); if (reclaim_state->is_complete()) { - INFO("reclaim {} finish, reclaimed alive/total={}, usage={}", - reclaim_state->get_segment_id(), - stats.reclaiming_bytes/(double)segments.get_segment_size(), - space_tracker->calc_utilization(reclaim_state->get_segment_id())); + auto segment_to_release = reclaim_state->get_segment_id(); + INFO("reclaim {} finish, reclaimed alive/total={}", + segment_to_release, + stats.reclaiming_bytes/(double)segments.get_segment_size()); stats.reclaimed_bytes += stats.reclaiming_bytes; stats.reclaimed_segment_bytes += segments.get_segment_size(); stats.reclaiming_bytes = 0; reclaim_state.reset(); + return sm_group->release_segment(segment_to_release + ).safe_then([this, FNAME, segment_to_release] { + auto old_usage = calc_utilization(segment_to_release); + if(unlikely(old_usage != 0)) { + space_tracker->dump_usage(segment_to_release); + ERRORT("segment {} old_usage {} != 0", + segment_to_release, old_usage); + ceph_abort(); + } + segments.mark_empty(segment_to_release); + auto new_usage = calc_utilization(segment_to_release); + adjust_segment_util(old_usage, new_usage); + INFO("released {}, {}", + segment_to_release, gc_stat_printer_t{this, false}); + maybe_wake_gc_blocked_io(); + }); + } else { + return SegmentManager::release_ertr::now(); } }); }); @@ -1189,36 +1204,6 @@ AsyncCleaner::scan_extents_ret AsyncCleaner::scan_no_tail_segment( }); } -AsyncCleaner::release_ertr::future<> -AsyncCleaner::maybe_release_segment(Transaction &t) -{ - auto to_release = t.get_segment_to_release(); - if (to_release != NULL_SEG_ID) { - LOG_PREFIX(AsyncCleaner::maybe_release_segment); - INFOT("releasing segment {}", t, to_release); - return sm_group->release_segment(to_release - ).safe_then([this, FNAME, &t, to_release] { - auto old_usage = calc_utilization(to_release); - if(unlikely(old_usage != 0)) { - space_tracker->dump_usage(to_release); - 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); - INFOT("released, {}", t, gc_stat_printer_t{this, false}); - if (space_tracker->get_usage(to_release) != 0) { - space_tracker->dump_usage(to_release); - ceph_abort(); - } - maybe_wake_gc_blocked_io(); - }); - } else { - return SegmentManager::release_ertr::now(); - } -} - void AsyncCleaner::complete_init() { LOG_PREFIX(AsyncCleaner::complete_init); diff --git a/src/crimson/os/seastore/async_cleaner.h b/src/crimson/os/seastore/async_cleaner.h index a107694a55c..963efe3a805 100644 --- a/src/crimson/os/seastore/async_cleaner.h +++ b/src/crimson/os/seastore/async_cleaner.h @@ -861,9 +861,6 @@ public: void update_journal_tails( journal_seq_t dirty_tail, journal_seq_t alloc_tail) final; - using release_ertr = SegmentManagerGroup::release_ertr; - release_ertr::future<> maybe_release_segment(Transaction &t); - void adjust_segment_util(double old_usage, double new_usage) { auto old_index = get_bucket_index(old_usage); auto new_index = get_bucket_index(new_usage); diff --git a/src/crimson/os/seastore/transaction.h b/src/crimson/os/seastore/transaction.h index 2917065b178..e38d3c56f17 100644 --- a/src/crimson/os/seastore/transaction.h +++ b/src/crimson/os/seastore/transaction.h @@ -226,15 +226,6 @@ public: } } - void mark_segment_to_release(segment_id_t segment) { - assert(to_release == NULL_SEG_ID); - to_release = segment; - } - - segment_id_t get_segment_to_release() const { - return to_release; - } - auto get_delayed_alloc_list() { std::list ret; for (auto& extent : delayed_alloc_list) { @@ -363,7 +354,6 @@ public: backref_tree_stats = {}; ool_write_stats = {}; rewrite_version_stats = {}; - to_release = NULL_SEG_ID; conflicted = false; if (!has_reset) { has_reset = true; @@ -519,9 +509,6 @@ private: ool_write_stats_t ool_write_stats; version_stat_t rewrite_version_stats; - ///< if != NULL_SEG_ID, release this segment after completion - segment_id_t to_release = NULL_SEG_ID; - bool conflicted = false; bool has_reset = false; diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc index f5dee399be7..806e646daff 100644 --- a/src/crimson/os/seastore/transaction_manager.cc +++ b/src/crimson/os/seastore/transaction_manager.cc @@ -389,9 +389,6 @@ TransactionManager::submit_transaction_direct( async_cleaner->update_journal_tails( cache->get_oldest_dirty_from().value_or(start_seq), cache->get_oldest_backref_dirty_from().value_or(start_seq)); - return async_cleaner->maybe_release_segment(tref); - }).safe_then([FNAME, &tref] { - SUBTRACET(seastore_t, "completed", tref); return tref.get_handle().complete(); }).handle_error( submit_transaction_iertr::pass_further{}, -- 2.39.5