From: Yingxin Cheng <yingxin.cheng@intel.com>
Date: Fri, 18 Mar 2022 01:41:04 +0000 (+0800)
Subject: test/crimson/seastore: fix metrics registration conflicts
X-Git-Tag: v18.0.0~1193^2
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=99c896d9f2b0860ae86d1c0f7e9306602c375e09;p=ceph.git

test/crimson/seastore: fix metrics registration conflicts

The conflicts are from adding segment manager to transaction manager
repeatedly. Fixed the repeat adding issue, add ceph_assert() to detect
this issue as early as possible, and consolidate initiation code into
make_transaction_manager().

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

diff --git a/src/crimson/os/seastore/extent_reader.h b/src/crimson/os/seastore/extent_reader.h
index ffd1ad4f42cda..1016f50f23846 100644
--- a/src/crimson/os/seastore/extent_reader.h
+++ b/src/crimson/os/seastore/extent_reader.h
@@ -81,8 +81,7 @@ public:
   ); ///< @return used budget
 
   void add_segment_manager(SegmentManager* segment_manager) {
-    assert(!segment_managers[segment_manager->get_device_id()] ||
-      segment_manager == segment_managers[segment_manager->get_device_id()]);
+    ceph_assert(!segment_managers[segment_manager->get_device_id()]);
     segment_managers[segment_manager->get_device_id()] = segment_manager;
   }
 
@@ -94,6 +93,11 @@ public:
     return segment_managers[addr.get_device_id()]->read(addr, len, out);
   }
 
+  void reset() {
+    segment_managers.clear();
+    segment_managers.resize(DEVICE_ID_MAX, nullptr);
+  }
+
 private:
   std::vector<SegmentManager*> segment_managers;
 
diff --git a/src/crimson/os/seastore/seastore.cc b/src/crimson/os/seastore/seastore.cc
index 707bd42560168..2ac23ccc13819 100644
--- a/src/crimson/os/seastore/seastore.cc
+++ b/src/crimson/os/seastore/seastore.cc
@@ -1625,27 +1625,7 @@ seastar::future<std::unique_ptr<SeaStore>> make_seastore(
   return SegmentManager::get_segment_manager(
     device
   ).then([&device](auto sm) {
-    auto scanner = std::make_unique<ExtentReader>();
-    auto& scanner_ref = *scanner.get();
-    auto segment_cleaner = std::make_unique<SegmentCleaner>(
-      SegmentCleaner::config_t::get_default(),
-      std::move(scanner),
-      false /* detailed */);
-
-    auto journal = journal::make_segmented(*sm, scanner_ref, *segment_cleaner);
-    auto epm = std::make_unique<ExtentPlacementManager>();
-    auto cache = std::make_unique<Cache>(scanner_ref, *epm);
-    auto lba_manager = lba_manager::create_lba_manager(*sm, *cache);
-
-    auto tm = std::make_unique<TransactionManager>(
-      *sm,
-      std::move(segment_cleaner),
-      std::move(journal),
-      std::move(cache),
-      std::move(lba_manager),
-      std::move(epm),
-      scanner_ref);
-
+    auto tm = make_transaction_manager(*sm, false /* detailed */);
     auto cm = std::make_unique<collection_manager::FlatCollectionManager>(*tm);
     return std::make_unique<SeaStore>(
       device,
diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc
index 266a064857e86..7021293d204bd 100644
--- a/src/crimson/os/seastore/transaction_manager.cc
+++ b/src/crimson/os/seastore/transaction_manager.cc
@@ -161,6 +161,7 @@ TransactionManager::close_ertr::future<> TransactionManager::close() {
     cache->dump_contents();
     return journal->close();
   }).safe_then([this] {
+    scanner.reset();
     return epm->close();
   }).safe_then([FNAME] {
     INFO("completed");
@@ -554,4 +555,29 @@ TransactionManager::get_extent_if_live_ret TransactionManager::get_extent_if_liv
 
 TransactionManager::~TransactionManager() {}
 
+TransactionManagerRef make_transaction_manager(
+    SegmentManager& sm,
+    bool detailed)
+{
+  auto scanner = std::make_unique<ExtentReader>();
+  auto& scanner_ref = *scanner.get();
+  auto segment_cleaner = std::make_unique<SegmentCleaner>(
+    SegmentCleaner::config_t::get_default(),
+    std::move(scanner),
+    detailed);
+  auto journal = journal::make_segmented(sm, scanner_ref, *segment_cleaner);
+  auto epm = std::make_unique<ExtentPlacementManager>();
+  auto cache = std::make_unique<Cache>(scanner_ref, *epm);
+  auto lba_manager = lba_manager::create_lba_manager(sm, *cache);
+
+  return std::make_unique<TransactionManager>(
+    sm,
+    std::move(segment_cleaner),
+    std::move(journal),
+    std::move(cache),
+    std::move(lba_manager),
+    std::move(epm),
+    scanner_ref);
+}
+
 }
diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h
index b9192214af962..f1a5745322ad6 100644
--- a/src/crimson/os/seastore/transaction_manager.h
+++ b/src/crimson/os/seastore/transaction_manager.h
@@ -585,4 +585,8 @@ public:
 };
 using TransactionManagerRef = std::unique_ptr<TransactionManager>;
 
+TransactionManagerRef make_transaction_manager(
+    SegmentManager& sm,
+    bool detailed);
+
 }
diff --git a/src/crimson/tools/store_nbd/tm_driver.cc b/src/crimson/tools/store_nbd/tm_driver.cc
index a87846e44672e..851228e2ad429 100644
--- a/src/crimson/tools/store_nbd/tm_driver.cc
+++ b/src/crimson/tools/store_nbd/tm_driver.cc
@@ -3,6 +3,8 @@
 
 #include "tm_driver.h"
 
+#include "crimson/os/seastore/segment_manager/block.h"
+
 using namespace crimson;
 using namespace crimson::os;
 using namespace crimson::os::seastore;
@@ -132,33 +134,8 @@ seastar::future<bufferlist> TMDriver::read(
 
 void TMDriver::init()
 {
-  auto scanner = std::make_unique<ExtentReader>();
-  scanner->add_segment_manager(segment_manager.get());
-  auto& scanner_ref = *scanner.get();
-  auto segment_cleaner = std::make_unique<SegmentCleaner>(
-    SegmentCleaner::config_t::get_default(),
-    std::move(scanner),
-    false /* detailed */);
-  auto journal = journal::make_segmented(
-    *segment_manager, *scanner, *segment_cleaner);
-  auto epm = std::make_unique<ExtentPlacementManager>();
-  auto cache = std::make_unique<Cache>(scanner_ref, *epm);
-  auto lba_manager = lba_manager::create_lba_manager(*segment_manager, *cache);
-
-  epm->add_allocator(
-    device_type_t::SEGMENTED,
-    std::make_unique<SegmentedAllocator>(
-      *segment_cleaner,
-      *segment_manager));
-
-  tm = std::make_unique<TransactionManager>(
-    *segment_manager,
-    std::move(segment_cleaner),
-    std::move(journal),
-    std::move(cache),
-    std::move(lba_manager),
-    std::move(epm),
-    scanner_ref);
+  tm = make_transaction_manager(*segment_manager, false /* detailed */);
+  tm->add_segment_manager(segment_manager.get());
 }
 
 void TMDriver::clear()
diff --git a/src/crimson/tools/store_nbd/tm_driver.h b/src/crimson/tools/store_nbd/tm_driver.h
index c0e1fc480468c..f6d7c9b8d5cbd 100644
--- a/src/crimson/tools/store_nbd/tm_driver.h
+++ b/src/crimson/tools/store_nbd/tm_driver.h
@@ -6,7 +6,6 @@
 #include "crimson/os/seastore/cache.h"
 #include "crimson/os/seastore/segment_cleaner.h"
 #include "crimson/os/seastore/segment_manager.h"
-#include "crimson/os/seastore/segment_manager/block.h"
 #include "crimson/os/seastore/transaction_manager.h"
 #include "test/crimson/seastore/test_block.h"
 
@@ -36,8 +35,8 @@ public:
 private:
   const config_t config;
 
-  using BlockSegmentManager = crimson::os::seastore::segment_manager::block::BlockSegmentManager;
-  std::unique_ptr<BlockSegmentManager> segment_manager;
+  using SegmentManagerRef = crimson::os::seastore::SegmentManagerRef;
+  SegmentManagerRef segment_manager;
 
   using TransactionManager = crimson::os::seastore::TransactionManager;
   using TransactionManagerRef = crimson::os::seastore::TransactionManagerRef;
diff --git a/src/test/crimson/seastore/onode_tree/test_fltree_onode_manager.cc b/src/test/crimson/seastore/onode_tree/test_fltree_onode_manager.cc
index 047e695a4100a..77578f3625f88 100644
--- a/src/test/crimson/seastore/onode_tree/test_fltree_onode_manager.cc
+++ b/src/test/crimson/seastore/onode_tree/test_fltree_onode_manager.cc
@@ -84,6 +84,7 @@ struct fltree_onode_manager_test_t
   virtual FuturizedStore::mkfs_ertr::future<> _mkfs() final {
     return TMTestState::_mkfs(
     ).safe_then([this] {
+      tm->add_segment_manager(segment_manager.get());
       return tm->mount(
       ).safe_then([this] {
 	return repeat_eagain([this] {
diff --git a/src/test/crimson/seastore/transaction_manager_test_state.h b/src/test/crimson/seastore/transaction_manager_test_state.h
index 9f7d3aebb1777..7c0d3df0878d4 100644
--- a/src/test/crimson/seastore/transaction_manager_test_state.h
+++ b/src/test/crimson/seastore/transaction_manager_test_state.h
@@ -70,41 +70,8 @@ protected:
   }
 };
 
-auto get_transaction_manager(
-  SegmentManager &segment_manager) {
-  auto scanner = std::make_unique<ExtentReader>();
-  scanner->add_segment_manager(&segment_manager);
-  auto& scanner_ref = *scanner.get();
-  auto segment_cleaner = std::make_unique<SegmentCleaner>(
-    SegmentCleaner::config_t::get_default(),
-    std::move(scanner),
-    true);
-  auto journal = journal::make_segmented(
-    segment_manager,
-    scanner_ref,
-    *segment_cleaner);
-  auto epm = std::make_unique<ExtentPlacementManager>();
-  auto cache = std::make_unique<Cache>(scanner_ref, *epm);
-  auto lba_manager = lba_manager::create_lba_manager(segment_manager, *cache);
-
-  epm->add_allocator(
-    device_type_t::SEGMENTED,
-    std::make_unique<SegmentedAllocator>(
-      *segment_cleaner,
-      segment_manager));
-
-  return std::make_unique<TransactionManager>(
-    segment_manager,
-    std::move(segment_cleaner),
-    std::move(journal),
-    std::move(cache),
-    std::move(lba_manager),
-    std::move(epm),
-    scanner_ref);
-}
-
 auto get_seastore(SeaStore::MDStoreRef mdstore, SegmentManagerRef sm) {
-  auto tm = get_transaction_manager(*sm);
+  auto tm = make_transaction_manager(*sm, true);
   auto cm = std::make_unique<collection_manager::FlatCollectionManager>(*tm);
   return std::make_unique<SeaStore>(
     "",
@@ -125,7 +92,8 @@ protected:
   TMTestState() : EphemeralTestState() {}
 
   virtual void _init() override {
-    tm = get_transaction_manager(*segment_manager);
+    tm = make_transaction_manager(*segment_manager, true);
+    tm->add_segment_manager(segment_manager.get());
     segment_cleaner = tm->get_segment_cleaner();
     lba_manager = tm->get_lba_manager();
   }