From 593b3a9b9119886787ddcf1122797f8b3655bd8e Mon Sep 17 00:00:00 2001
From: Yingxin Cheng <yingxin.cheng@intel.com>
Date: Tue, 15 Mar 2022 09:18:08 +0800
Subject: [PATCH] crimson/os/seastore/EPM: coalesce writes using the same
 writer

After integrated with RecordSubmitter, Writer is able to coalesce
concurrent writes into record groups. There is no need to distribute
writes to multiple Writers for both large and small write sizes.

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
---
 src/common/options/crimson.yaml.in            |  5 ----
 .../os/seastore/extent_placement_manager.cc   | 14 +++------
 .../os/seastore/extent_placement_manager.h    | 30 ++++++++++++-------
 3 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/src/common/options/crimson.yaml.in b/src/common/options/crimson.yaml.in
index 7f906a88958..ee9ee653ec7 100644
--- a/src/common/options/crimson.yaml.in
+++ b/src/common/options/crimson.yaml.in
@@ -40,11 +40,6 @@ options:
   default: true
   see_also:
   - seastore_device_size
-- name: seastore_init_write_segments_num_per_device
-  type: uint
-  level: dev
-  desc: Initial number of segments for writing new extents per device
-  default: 6
 - name: seastore_journal_batch_capacity
   type: uint
   level: dev
diff --git a/src/crimson/os/seastore/extent_placement_manager.cc b/src/crimson/os/seastore/extent_placement_manager.cc
index fcb9cb39478..660e8d554f4 100644
--- a/src/crimson/os/seastore/extent_placement_manager.cc
+++ b/src/crimson/os/seastore/extent_placement_manager.cc
@@ -12,22 +12,16 @@ namespace crimson::os::seastore {
 SegmentedAllocator::SegmentedAllocator(
   SegmentProvider& sp,
   SegmentManager& sm)
-  : rewriter(sp, sm)
+  : cold_writer{"COLD", sp, sm},
+    rewrite_writer{"REWRITE", sp, sm}
 {
-  std::generate_n(
-    std::back_inserter(writers),
-    crimson::common::get_conf<uint64_t>(
-      "seastore_init_write_segments_num_per_device"),
-    [&] {
-      return Writer{sp, sm};
-    }
-  );
 }
 
 SegmentedAllocator::Writer::Writer(
+  std::string name,
   SegmentProvider& sp,
   SegmentManager& sm)
-  : segment_allocator("OOL", segment_type_t::OOL, sp, sm),
+  : segment_allocator(name, segment_type_t::OOL, sp, sm),
     record_submitter(crimson::common::get_conf<uint64_t>(
                        "seastore_journal_iodepth_limit"),
                      crimson::common::get_conf<uint64_t>(
diff --git a/src/crimson/os/seastore/extent_placement_manager.h b/src/crimson/os/seastore/extent_placement_manager.h
index 263dc0cf421..0d1c4e49cfb 100644
--- a/src/crimson/os/seastore/extent_placement_manager.h
+++ b/src/crimson/os/seastore/extent_placement_manager.h
@@ -75,7 +75,7 @@ class SegmentProvider;
 class SegmentedAllocator : public ExtentAllocator {
   class Writer : public ExtentOolWriter {
   public:
-    Writer(SegmentProvider& sp, SegmentManager& sm);
+    Writer(std::string name, SegmentProvider& sp, SegmentManager& sm);
     Writer(Writer &&) = default;
 
     open_ertr::future<> open() final;
@@ -112,16 +112,20 @@ public:
     SegmentManager& sm);
 
   Writer &get_writer(placement_hint_t hint) {
-    if (hint == placement_hint_t::REWRITE) {
-      return rewriter;
+    assert(hint >= placement_hint_t::COLD);
+    assert(hint < placement_hint_t::NUM_HINTS);
+    if (hint == placement_hint_t::COLD) {
+      return cold_writer;
     } else {
-      return writers[std::rand() % writers.size()];
+      assert(hint == placement_hint_t::REWRITE);
+      return rewrite_writer;
     }
   }
 
   open_ertr::future<> open() {
-    return crimson::do_for_each(writers, [](auto& writer) {
-      return writer.open();
+    return cold_writer.open(
+    ).safe_then([this] {
+      return rewrite_writer.open();
     });
   }
 
@@ -146,13 +150,17 @@ public:
   }
 
   stop_ertr::future<> stop() {
-    return crimson::do_for_each(writers, [](auto& writer) {
-      return writer.stop();
+    return cold_writer.stop(
+    ).safe_then([this] {
+      return rewrite_writer.stop();
     });
   }
 private:
-  Writer rewriter;
-  std::vector<Writer> writers;
+  // TODO:
+  // - hot_writer
+  // - a map of hint -> writer
+  Writer cold_writer;
+  Writer rewrite_writer;
 };
 
 class ExtentPlacementManager {
@@ -202,7 +210,7 @@ public:
               std::move(bp)};
     }
 
-    // FIXME: set delay for COLD extent when the record overhead is low
+    // FIXME: set delay for COLD extent and improve GC
     // NOTE: delay means to delay the decision about whether to write the
     // extent as inline or out-of-line extents.
     bool delay = (hint > placement_hint_t::COLD &&
-- 
2.39.5