From f8f6ba5c0494062fd5dabcb37066d838713f92a2 Mon Sep 17 00:00:00 2001 From: Yingxin Cheng Date: Thu, 31 Mar 2022 09:54:20 +0800 Subject: [PATCH] crimson/os/seastore/segment_manager_group: cleanups * track valid device ids * return valid segment managers Signed-off-by: Yingxin Cheng --- src/crimson/os/seastore/segment_cleaner.cc | 8 +-- src/crimson/os/seastore/segment_cleaner.h | 16 +----- .../os/seastore/segment_manager_group.cc | 5 ++ .../os/seastore/segment_manager_group.h | 54 +++++++++++++------ 4 files changed, 47 insertions(+), 36 deletions(-) diff --git a/src/crimson/os/seastore/segment_cleaner.cc b/src/crimson/os/seastore/segment_cleaner.cc index 085fe74994f..82d164f9184 100644 --- a/src/crimson/os/seastore/segment_cleaner.cc +++ b/src/crimson/os/seastore/segment_cleaner.cc @@ -505,7 +505,7 @@ SegmentCleaner::gc_reclaim_space_ret SegmentCleaner::gc_reclaim_space() SegmentCleaner::mount_ret SegmentCleaner::mount( device_id_t pdevice_id) { - auto& sms = sm_group->get_segment_managers(); + const auto& sms = sm_group->get_segment_managers(); logger().debug( "SegmentCleaner::mount: {} segment managers", sms.size()); init_complete = false; @@ -524,12 +524,6 @@ SegmentCleaner::mount_ret SegmentCleaner::mount( segments.clear(); for (auto sm : sms) { - // sms is a vector that is indexed by device id and - // always has "max_device" elements, some of which - // may be null. - if (!sm) { - continue; - } segments.add_segment_manager(*sm); stats.empty_segments += sm->get_num_segments(); } diff --git a/src/crimson/os/seastore/segment_cleaner.h b/src/crimson/os/seastore/segment_cleaner.h index df7c225ef8a..b5a53ba87ff 100644 --- a/src/crimson/os/seastore/segment_cleaner.h +++ b/src/crimson/os/seastore/segment_cleaner.h @@ -357,14 +357,8 @@ class SpaceTrackerSimple : public SpaceTrackerI { } public: SpaceTrackerSimple(const SpaceTrackerSimple &) = default; - SpaceTrackerSimple(std::vector sms) { + SpaceTrackerSimple(const std::vector &sms) { for (auto sm : sms) { - if (!sm) { - // sms is a vector that is indexed by device id and - // always has "max_device" elements, some of which - // may be null. - continue; - } live_bytes_by_segment.add_device( sm->get_device_id(), sm->get_num_segments(), @@ -466,16 +460,10 @@ class SpaceTrackerDetailed : public SpaceTrackerI { public: SpaceTrackerDetailed(const SpaceTrackerDetailed &) = default; - SpaceTrackerDetailed(std::vector sms) + SpaceTrackerDetailed(const std::vector &sms) { block_size_by_segment_manager.resize(DEVICE_ID_MAX, 0); for (auto sm : sms) { - // sms is a vector that is indexed by device id and - // always has "max_device" elements, some of which - // may be null. - if (!sm) { - continue; - } segment_usage.add_device( sm->get_device_id(), sm->get_num_segments(), diff --git a/src/crimson/os/seastore/segment_manager_group.cc b/src/crimson/os/seastore/segment_manager_group.cc index 3e53021d0e9..b691e510058 100644 --- a/src/crimson/os/seastore/segment_manager_group.cc +++ b/src/crimson/os/seastore/segment_manager_group.cc @@ -12,6 +12,7 @@ namespace crimson::os::seastore { SegmentManagerGroup::read_segment_tail_ret SegmentManagerGroup::read_segment_tail(segment_id_t segment) { + assert(has_device(segment.device_id())); auto& segment_manager = *segment_managers[segment.device_id()]; return segment_manager.read( paddr_t::make_seg_paddr( @@ -54,6 +55,7 @@ SegmentManagerGroup::read_segment_tail(segment_id_t segment) SegmentManagerGroup::read_segment_header_ret SegmentManagerGroup::read_segment_header(segment_id_t segment) { + assert(has_device(segment.device_id())); auto& segment_manager = *segment_managers[segment.device_id()]; return segment_manager.read( paddr_t::make_seg_paddr(segment, 0), @@ -160,6 +162,7 @@ SegmentManagerGroup::scan_valid_records( found_record_handler_t &handler) { LOG_PREFIX(SegmentManagerGroup::scan_valid_records); + assert(has_device(cursor.get_segment_id().device_id())); auto& segment_manager = *segment_managers[cursor.get_segment_id().device_id()]; if (cursor.get_segment_offset() == 0) { @@ -260,6 +263,7 @@ SegmentManagerGroup::read_validate_record_metadata( { LOG_PREFIX(SegmentManagerGroup::read_validate_record_metadata); auto& seg_addr = start.as_seg_paddr(); + assert(has_device(seg_addr.get_segment_id().device_id())); auto& segment_manager = *segment_managers[seg_addr.get_segment_id().device_id()]; auto block_size = segment_manager.get_block_size(); auto segment_size = static_cast(segment_manager.get_segment_size()); @@ -335,6 +339,7 @@ SegmentManagerGroup::read_validate_data( const record_group_header_t &header) { LOG_PREFIX(SegmentManagerGroup::read_validate_data); + assert(has_device(record_base.get_device_id())); auto& segment_manager = *segment_managers[record_base.get_device_id()]; auto data_addr = record_base.add_offset(header.mdlength); TRACE("reading record group data blocks {}~{}", data_addr, header.dlength); diff --git a/src/crimson/os/seastore/segment_manager_group.h b/src/crimson/os/seastore/segment_manager_group.h index 522eed1557d..cc715e47086 100644 --- a/src/crimson/os/seastore/segment_manager_group.h +++ b/src/crimson/os/seastore/segment_manager_group.h @@ -3,6 +3,8 @@ #pragma once +#include + #include "crimson/common/errorator.h" #include "crimson/os/seastore/seastore_types.h" #include "crimson/os/seastore/segment_manager.h" @@ -11,14 +13,38 @@ namespace crimson::os::seastore { class SegmentManagerGroup { public: - std::vector& get_segment_managers() { - return segment_managers; + SegmentManagerGroup() { + segment_managers.resize(DEVICE_ID_MAX, nullptr); } - using read_ertr = SegmentManager::read_ertr; - SegmentManagerGroup() { + const std::set& get_device_ids() const { + return device_ids; + } + + std::vector get_segment_managers() const { + assert(device_ids.size()); + std::vector ret; + for (auto& device_id : device_ids) { + auto segment_manager = segment_managers[device_id]; + assert(segment_manager->get_device_id() == device_id); + ret.emplace_back(segment_manager); + } + return ret; + } + + void add_segment_manager(SegmentManager* segment_manager) { + auto device_id = segment_manager->get_device_id(); + ceph_assert(!has_device(device_id)); + segment_managers[device_id] = segment_manager; + device_ids.insert(device_id); + } + + void reset() { + segment_managers.clear(); segment_managers.resize(DEVICE_ID_MAX, nullptr); + device_ids.clear(); } + using read_segment_header_ertr = crimson::errorator< crimson::ct_error::enoent, crimson::ct_error::enodata, @@ -47,6 +73,7 @@ public: * Returns list * cursor.is_complete() will be true when no further extents exist in segment. */ + using read_ertr = SegmentManager::read_ertr; using scan_extents_cursor = scan_valid_records_cursor; using scan_extents_ertr = read_ertr::extend; using scan_extents_ret_bare = @@ -76,22 +103,16 @@ public: using release_ertr = SegmentManager::release_ertr; release_ertr::future<> release_segment(segment_id_t id) { - assert(segment_managers[id.device_id()] != nullptr); + assert(has_device(id.device_id())); return segment_managers[id.device_id()]->release(id); } - void add_segment_manager(SegmentManager* segment_manager) { - ceph_assert(!segment_managers[segment_manager->get_device_id()]); - segment_managers[segment_manager->get_device_id()] = segment_manager; - } - - void reset() { - segment_managers.clear(); - segment_managers.resize(DEVICE_ID_MAX, nullptr); +private: + bool has_device(device_id_t id) const { + assert(id <= DEVICE_ID_MAX_VALID); + return device_ids.count(id) >= 1; } -private: - std::vector segment_managers; /// read record metadata for record starting at start using read_validate_record_metadata_ertr = read_ertr; using read_validate_record_metadata_ret = @@ -116,6 +137,9 @@ private: scan_valid_records_cursor& cursor, found_record_handler_t& handler, std::size_t& budget_used); + + std::vector segment_managers; + std::set device_ids; }; using SegmentManagerGroupRef = std::unique_ptr; -- 2.39.5