crimson/os/seastore: add is_data_stable() to allow delta-overwrite on EXIST_CLEAN

Signed-off-by: Myoungwon Oh <myoungwon.oh@samsung.com>
(cherry picked from commit c20a5d8b53789fd53fb98bb784a180f4d349035a)

diff --git a/src/crimson/os/seastore/btree/btree_range_pin.cc b/src/crimson/os/seastore/btree/btree_range_pin.cc
index 1c34e99586064d5a10b341a87dc4ab6613cadee0..1e72f3da75f386f421d76a4aa6db582766d3831d 100644 (file)
--- a/src/crimson/os/seastore/btree/btree_range_pin.cc
+++ b/src/crimson/os/seastore/btree/btree_range_pin.cc
@@ -36,6 +36,16 @@ bool BtreeNodeMapping<key_t, val_t>::is_stable() const
   return p.is_child_stable(ctx, pos);
 }
 
+template <typename key_t, typename val_t>
+bool BtreeNodeMapping<key_t, val_t>::is_data_stable() const
+{
+  assert(parent);
+  assert(parent->is_valid());
+  assert(pos != std::numeric_limits<uint16_t>::max());
+  auto &p = (FixedKVNode<key_t>&)*parent;
+  return p.is_child_data_stable(ctx, pos);
+}
+
 template class BtreeNodeMapping<laddr_t, paddr_t>;
 template class BtreeNodeMapping<paddr_t, laddr_t>;
 } // namespace crimson::os::seastore

diff --git a/src/crimson/os/seastore/btree/btree_range_pin.h b/src/crimson/os/seastore/btree/btree_range_pin.h
index b23a50bf4badf7691f857bd96068056b5c5af2c3..a2d74558733548ff7fbed2e1521699faf041c257 100644 (file)
--- a/src/crimson/os/seastore/btree/btree_range_pin.h
+++ b/src/crimson/os/seastore/btree/btree_range_pin.h
@@ -196,6 +196,7 @@ public:
 
   get_child_ret_t<LogicalCachedExtent> get_logical_extent(Transaction&) final;
   bool is_stable() const final;
+  bool is_data_stable() const final;
 };
 
 }

diff --git a/src/crimson/os/seastore/btree/fixed_kv_node.h b/src/crimson/os/seastore/btree/fixed_kv_node.h
index f4e7f6239a20f6819893335ba66464c0f7f3a45f..14a1703abbc81a54ea98681466416b07479c674e 100644 (file)
--- a/src/crimson/os/seastore/btree/fixed_kv_node.h
+++ b/src/crimson/os/seastore/btree/fixed_kv_node.h
@@ -227,6 +227,7 @@ struct FixedKVNode : ChildableCachedExtent {
   }
 
   virtual bool is_child_stable(op_context_t<node_key_t>, uint16_t pos) const = 0;
+  virtual bool is_child_data_stable(op_context_t<node_key_t>, uint16_t pos) const = 0;
 
   template <typename T>
   get_child_ret_t<T> get_child(
@@ -597,6 +598,10 @@ struct FixedKVInternalNode
     ceph_abort("impossible");
     return false;
   }
+  bool is_child_data_stable(op_context_t<NODE_KEY>, uint16_t pos) const final {
+    ceph_abort("impossible");
+    return false;
+  }
 
   bool validate_stable_children() final {
     LOG_PREFIX(FixedKVInternalNode::validate_stable_children);
@@ -986,6 +991,13 @@ struct FixedKVLeafNode
   //
   // For reserved mappings, the return values are undefined.
   bool is_child_stable(op_context_t<NODE_KEY> c, uint16_t pos) const final {
+    return _is_child_stable(c, pos);
+  }
+  bool is_child_data_stable(op_context_t<NODE_KEY> c, uint16_t pos) const final {
+    return _is_child_stable(c, pos, true);
+  }
+
+  bool _is_child_stable(op_context_t<NODE_KEY> c, uint16_t pos, bool data_only = false) const {
     auto child = this->children[pos];
     if (is_reserved_ptr(child)) {
       return true;
@@ -994,7 +1006,11 @@ struct FixedKVLeafNode
       ceph_assert(
        child->is_pending_in_trans(c.trans.get_trans_id())
        || this->is_stable_written());
-      return c.cache.is_viewable_extent_stable(c.trans, child);
+      if (data_only) {
+       return c.cache.is_viewable_extent_data_stable(c.trans, child);
+      } else {
+       return c.cache.is_viewable_extent_stable(c.trans, child);
+      }
     } else if (this->is_pending()) {
       auto key = this->iter_idx(pos).get_key();
       auto &sparent = this->get_stable_for_key(key);
@@ -1002,7 +1018,11 @@ struct FixedKVLeafNode
       auto child = sparent.children[spos];
       if (is_valid_child_ptr(child)) {
        ceph_assert(child->is_logical());
-       return c.cache.is_viewable_extent_stable(c.trans, child);
+       if (data_only) {
+         return c.cache.is_viewable_extent_data_stable(c.trans, child);
+       } else {
+         return c.cache.is_viewable_extent_stable(c.trans, child);
+       }
       } else {
        return true;
       }

diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index a3becb78ee9aadf43acede0fd9cf26287124b218..0bf03abf6a9093a5b506f3057483695c5b405060 100644 (file)
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -452,6 +452,15 @@ public:
     return view->is_stable();
   }
 
+  bool is_viewable_extent_data_stable(
+    Transaction &t,
+    CachedExtentRef extent)
+  {
+    assert(extent);
+    auto view = extent->get_transactional_view(t);
+    return view->is_data_stable();
+  }
+
   using get_extent_ertr = base_ertr;
   get_extent_ertr::future<CachedExtentRef>
   get_extent_viewable_by_trans(

diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h
index f264b3a707c368d8bcb91559a3d28be2f1d543c5..942be72323161aea34593ecbe78106ed04319667 100644 (file)
--- a/src/crimson/os/seastore/cached_extent.h
+++ b/src/crimson/os/seastore/cached_extent.h
@@ -436,6 +436,10 @@ public:
             is_pending_io());
   }
 
+  bool is_data_stable() const {
+    return is_stable() || is_exist_clean();
+  }
+
   /// Returns true if extent has a pending delta
   bool is_mutation_pending() const {
     return state == extent_state_t::MUTATION_PENDING;
@@ -1087,6 +1091,7 @@ public:
   // For reserved mappings, the return values are
   // undefined although it won't crash
   virtual bool is_stable() const = 0;
+  virtual bool is_data_stable() const = 0;
   virtual bool is_clone() const = 0;
   bool is_zero_reserved() const {
     return !get_val().is_real();

diff --git a/src/crimson/os/seastore/object_data_handler.cc b/src/crimson/os/seastore/object_data_handler.cc
index b6d5a86dd6d6936019daac351a122a843cabb2e9..4af71c27ae33820f190a76c16e270b6d721c4ada 100644 (file)
--- a/src/crimson/os/seastore/object_data_handler.cc
+++ b/src/crimson/os/seastore/object_data_handler.cc
@@ -297,14 +297,13 @@ overwrite_ops_t prepare_ops_list(
   interval_set<uint64_t> pre_alloc_addr_removed, pre_alloc_addr_remapped;
   if (delta_based_overwrite_max_extent_size) {
     for (auto &r : ops.to_remove) {
-      // TODO: Introduce LBAMapping::is_data_stable() to include EXIST_CLEAN extents
-      if (r->is_stable() && !r->is_zero_reserved()) {
+      if (r->is_data_stable() && !r->is_zero_reserved()) {
        pre_alloc_addr_removed.insert(r->get_key(), r->get_length());
 
       }
     }
     for (auto &r : ops.to_remap) {
-      if (r.pin && r.pin->is_stable() && !r.pin->is_zero_reserved()) {
+      if (r.pin && r.pin->is_data_stable() && !r.pin->is_zero_reserved()) {
        pre_alloc_addr_remapped.insert(r.pin->get_key(), r.pin->get_length());
       }
     }

diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h
index c7e4121263a9002c9efdc1c2f3cac37daa03837f..783fea1724ed39730973525df6dd9898a24fae0b 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.h
+++ b/src/crimson/os/seastore/transaction_manager.h
@@ -365,7 +365,7 @@ public:
   read_extent_ret<T> get_mutable_extent_by_laddr(Transaction &t, laddr_t laddr, extent_len_t len) {
     return get_pin(t, laddr
     ).si_then([this, &t, len](auto pin) {
-      ceph_assert(pin->is_stable() && !pin->is_zero_reserved());
+      ceph_assert(pin->is_data_stable() && !pin->is_zero_reserved());
       ceph_assert(!pin->is_clone());
       ceph_assert(pin->get_length() == len);
       return this->read_pin<T>(t, std::move(pin));

diff --git a/src/test/crimson/seastore/test_object_data_handler.cc b/src/test/crimson/seastore/test_object_data_handler.cc
index 2200be8a4e778e54845e1fb8a5fce08f222c69e2..0e258b9a36ca1825f27f98fc7adfe67a6e825f9c 100644 (file)
--- a/src/test/crimson/seastore/test_object_data_handler.cc
+++ b/src/test/crimson/seastore/test_object_data_handler.cc
@@ -800,22 +800,9 @@ TEST_P(object_data_handler_test_t, overwrite_then_read_within_transaction) {
       auto pin1 = remap_pin(*t, std::move(pins.front()), 4096, 8192);
       auto ext = get_extent(*t, base + 4096, 4096 * 2);
       ASSERT_TRUE(ext->is_exist_clean());
-      ext = tm->get_mutable_extent(*t, ext)->cast<ObjectDataBlock>();
-
-      auto l = 4096;
-      memset(
-       known_contents.c_str() + base + 4096,
-       'z',
-       l);
-      bufferlist bl;
-      bl.append(
-       bufferptr(
-         known_contents,
-         base + 4096,
-         l));
-
-      ext->overwrite(0, bl);
+      write(*t, base + 4096, 4096, 'y');
       ASSERT_TRUE(ext->is_exist_mutation_pending());
+      write(*t, base + 8092, 4096, 'z');
     }
     submit_transaction(std::move(t));
     read(base + 4096, 4096);