crimson/os/seastore/segment_manager_group: cleanups

* track valid device ids
* return valid segment managers

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>

diff --git a/src/crimson/os/seastore/segment_cleaner.cc b/src/crimson/os/seastore/segment_cleaner.cc
index 085fe74994ffecd685bd74d1a709b55b1b4f2768..82d164f9184a93072a6e8cf2fe27476f949c31cd 100644 (file)
--- 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 df7c225ef8a508530e0fd0b09ff8d49b4a149180..b5a53ba87ff0abd3759b6c15bc99b3f485c27ef6 100644 (file)
--- 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<SegmentManager*> sms) {
+  SpaceTrackerSimple(const std::vector<SegmentManager*> &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<SegmentManager*> sms)
+  SpaceTrackerDetailed(const std::vector<SegmentManager*> &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 3e53021d0e9454f5624d1a554b347709903c89d9..b691e5100583d27f061c3e19d43cac8741dcb56d 100644 (file)
--- 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<int64_t>(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 522eed1557dd0eb5bdefeda9d43d97b3c1129d48..cc715e470866db369692acc99f883333f823e23b 100644 (file)
--- 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 <set>
+
 #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<SegmentManager*>& get_segment_managers() {
-    return segment_managers;
+  SegmentManagerGroup() {
+    segment_managers.resize(DEVICE_ID_MAX, nullptr);
   }
 
-  using read_ertr = SegmentManager::read_ertr;
-  SegmentManagerGroup() {
+  const std::set<device_id_t>& get_device_ids() const {
+    return device_ids;
+  }
+
+  std::vector<SegmentManager*> get_segment_managers() const {
+    assert(device_ids.size());
+    std::vector<SegmentManager*> 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<extent, extent_info>
    * 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<crimson::ct_error::enodata>;
   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<SegmentManager*> 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<SegmentManager*> segment_managers;
+  std::set<device_id_t> device_ids;
 };
 
 using SegmentManagerGroupRef = std::unique_ptr<SegmentManagerGroup>;