crimson/os/seastore: introduce modified_region in DATA_BLOCK to keep track of modifie...

It has a limitation to keep track of the modified region using the existing
deltas because we can not get the correct region in two cases: 1) a case where replay
is done and 2) duplicate_for_write. This commit introduces modified region
to solve the problem.

Signed-off-by: Myoungwon Oh <myoungwon.oh@samsung.com>

diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc
index fe42efa74ad70ec4af7fb6c8b05cc04a070535e0..24fa9788fe89ee1abae29cac951093283592d7e5 100644 (file)
--- a/src/crimson/os/seastore/cache.cc
+++ b/src/crimson/os/seastore/cache.cc
@@ -1288,7 +1288,7 @@ record_t Cache::prepare_record(
     i->dirty_from_or_retired_at = JOURNAL_SEQ_MIN;
     i->state = CachedExtent::extent_state_t::CLEAN;
     assert(i->is_logical());
-    i->cast<LogicalCachedExtent>()->clear_delta();
+    i->clear_modified_region();
     touch_extent(*i);
     DEBUGT("inplace rewrite ool block is commmitted -- {}", t, *i);
   }

diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h
index a72011db27521e23ad6d02cfafebd132d3bdd8f3..487b0f3555e1f111d302f5e390f10c1d32a43961 100644 (file)
--- a/src/crimson/os/seastore/cached_extent.h
+++ b/src/crimson/os/seastore/cached_extent.h
@@ -1242,8 +1242,6 @@ public:
 
   void on_replace_prior(Transaction &t) final;
 
-  virtual void clear_delta() {}
-
   struct modified_region_t {
     extent_len_t offset;
     extent_len_t len;
@@ -1252,6 +1250,8 @@ public:
     return std::nullopt;
   }
 
+  virtual void clear_modified_region() {}
+
   virtual ~LogicalCachedExtent();
 protected:
 

diff --git a/src/crimson/os/seastore/extent_placement_manager.cc b/src/crimson/os/seastore/extent_placement_manager.cc
index 45f51a401e0810c8daacd65c53e91c9e1476a2e9..0562d975337cf93ebb02c809d47dd1d279233907 100644 (file)
--- a/src/crimson/os/seastore/extent_placement_manager.cc
+++ b/src/crimson/os/seastore/extent_placement_manager.cc
@@ -794,14 +794,11 @@ RandomBlockOolWriter::do_write(
     bufferptr bp;
     if (can_inplace_rewrite(t, ex)) {
       auto r = ex->get_modified_region();
-      if (r.has_value() && r->len > rbm->get_block_size()) {
-       offset = p2align(r->offset, rbm->get_block_size());
-       extent_len_t len =
-         p2roundup(r->offset + r->len, rbm->get_block_size()) - offset;
-       bp = ceph::bufferptr(ex->get_bptr(), offset, len);
-      } else {
-       bp = ex->get_bptr();
-      }
+      ceph_assert(r.has_value());
+      offset = p2align(r->offset, rbm->get_block_size());
+      extent_len_t len =
+       p2roundup(r->offset + r->len, rbm->get_block_size()) - offset;
+      bp = ceph::bufferptr(ex->get_bptr(), offset, len);
     } else {
       bp = ex->get_bptr();
     }

diff --git a/src/crimson/os/seastore/object_data_handler.cc b/src/crimson/os/seastore/object_data_handler.cc
index 3333fcfad9d4cc6aefeb9b32fd767bee41d3c443..5e49a48a18809cf0ed120805ca9d73dd5f088020 100644 (file)
--- a/src/crimson/os/seastore/object_data_handler.cc
+++ b/src/crimson/os/seastore/object_data_handler.cc
@@ -423,6 +423,7 @@ void ObjectDataBlock::apply_delta(const ceph::bufferlist &bl) {
   for (auto &&d : deltas) {
     auto iter = d.bl.cbegin();
     iter.copy(d.len, get_bptr().c_str() + d.offset);
+    modified_region.union_insert(d.offset, d.len);
   }
 }
 

diff --git a/src/crimson/os/seastore/object_data_handler.h b/src/crimson/os/seastore/object_data_handler.h
index 2b01089214c271786ec2357d5e24d66df459db9e..783d0919ce5dba7327e319c3c831f33def61cdab 100644 (file)
--- a/src/crimson/os/seastore/object_data_handler.h
+++ b/src/crimson/os/seastore/object_data_handler.h
@@ -35,10 +35,12 @@ struct ObjectDataBlock : crimson::os::seastore::LogicalCachedExtent {
 
   std::vector<block_delta_t> delta = {};
 
+  interval_set<extent_len_t> modified_region;
+
   explicit ObjectDataBlock(ceph::bufferptr &&ptr)
     : LogicalCachedExtent(std::move(ptr)) {}
   explicit ObjectDataBlock(const ObjectDataBlock &other)
-    : LogicalCachedExtent(other) {}
+    : LogicalCachedExtent(other), modified_region(other.modified_region) {}
   explicit ObjectDataBlock(extent_len_t length)
     : LogicalCachedExtent(length) {}
 
@@ -55,28 +57,27 @@ struct ObjectDataBlock : crimson::os::seastore::LogicalCachedExtent {
     auto iter = bl.cbegin();
     iter.copy(bl.length(), get_bptr().c_str() + offset);
     delta.push_back({offset, bl.length(), bl});
+    modified_region.union_insert(offset, bl.length());
   }
 
   ceph::bufferlist get_delta() final;
 
   void apply_delta(const ceph::bufferlist &bl) final;
 
-  void clear_delta() final {
-    delta.clear();
-  }
-
   std::optional<modified_region_t> get_modified_region() final {
-    interval_set<extent_len_t> range;
-    for (auto &p : delta) {
-      if (p.len > 0) {
-       range.union_insert(p.offset, p.len);
-      }
-    }
-    if (range.empty()) {
+    if (modified_region.empty()) {
       return std::nullopt;
     }
-    return modified_region_t{range.range_start(),
-      range.range_end() - range.range_start()};
+    return modified_region_t{modified_region.range_start(),
+      modified_region.range_end() - modified_region.range_start()};
+  }
+
+  void clear_modified_region() final {
+    modified_region.clear();
+  }
+
+  void logical_on_delta_write() final {
+    delta.clear();
   }
 };
 using ObjectDataBlockRef = TCachedExtentRef<ObjectDataBlock>;

diff --git a/src/test/crimson/seastore/test_block.cc b/src/test/crimson/seastore/test_block.cc
index f7a39b0ef59cc25e90d782b9c7d6170441f84c14..7d673d8c23629597e1aa7f406308fc0d33d70d7e 100644 (file)
--- a/src/test/crimson/seastore/test_block.cc
+++ b/src/test/crimson/seastore/test_block.cc
@@ -19,6 +19,7 @@ void TestBlock::apply_delta(const ceph::bufferlist &bl) {
   decode(deltas, biter);
   for (auto &&d : deltas) {
     set_contents(d.val, d.offset, d.len);
+    modified_region.union_insert(d.offset, d.len);
   }
 }
 

diff --git a/src/test/crimson/seastore/test_block.h b/src/test/crimson/seastore/test_block.h
index 8ddb3880b4f59d82e9d8d2e12f2ef0f8b97e64da..2c70b7dbb876667a19adf1f88fa5e163fa9f2600 100644 (file)
--- a/src/test/crimson/seastore/test_block.h
+++ b/src/test/crimson/seastore/test_block.h
@@ -49,10 +49,12 @@ struct TestBlock : crimson::os::seastore::LogicalCachedExtent {
 
   std::vector<test_block_delta_t> delta = {};
 
+  interval_set<extent_len_t> modified_region;
+
   TestBlock(ceph::bufferptr &&ptr)
     : LogicalCachedExtent(std::move(ptr)) {}
   TestBlock(const TestBlock &other)
-    : LogicalCachedExtent(other) {}
+    : LogicalCachedExtent(other), modified_region(other.modified_region) {}
 
   CachedExtentRef duplicate_for_write(Transaction&) final {
     return CachedExtentRef(new TestBlock(*this));
@@ -68,6 +70,7 @@ struct TestBlock : crimson::os::seastore::LogicalCachedExtent {
   void set_contents(char c, uint16_t offset, uint16_t len) {
     ::memset(get_bptr().c_str() + offset, c, len);
     delta.push_back({c, offset, len});
+    modified_region.union_insert(offset, len);
   }
 
   void set_contents(char c) {
@@ -80,22 +83,20 @@ struct TestBlock : crimson::os::seastore::LogicalCachedExtent {
 
   void apply_delta(const ceph::bufferlist &bl) final;
 
-  void clear_delta() final {
-    delta.clear();
-  }
-
   std::optional<modified_region_t> get_modified_region() final {
-    interval_set<extent_len_t> range;
-    for (auto &p : delta) {
-      if (p.len > 0) {
-       range.union_insert(p.offset, p.len);
-      }
-    }
-    if (range.empty()) {
+    if (modified_region.empty()) {
       return std::nullopt;
     }
-    return modified_region_t{range.range_start(),
-      range.range_end() - range.range_start()};
+    return modified_region_t{modified_region.range_start(),
+      modified_region.range_end() - modified_region.range_start()};
+  }
+
+  void clear_modified_region() final {
+    modified_region.clear();
+  }
+
+  void logical_on_delta_write() final {
+    delta.clear();
   }
 };
 using TestBlockRef = TCachedExtentRef<TestBlock>;