crimson/os/seastore/lba: TRIM/CLEANER trans to adjust deltas of

LBALeafNodes when committing them.

This is to deal with the following scenario:
1. A client transaction modifies the value of the LBALeafNode, but not
   the pladdr but other field;
2. A TRIM/CLEANER transaction modifies the pladdr for the same laddr_t
   concurrently

In the old approach, the client trans may override the pladdr with the
outdated value after the TRIM/CLEANER transaction commits

Signed-off-by: Xuehan Xu <xuxuehan@qianxin.com>

diff --git a/src/crimson/common/fixed_kv_node_layout.h b/src/crimson/common/fixed_kv_node_layout.h
index 7adce7158b3e3e4ecf2adbf808962db7d0832c8c..a4aa8efd129a7dcab2e08dc7519bc4bc885a773f 100644 (file)
--- a/src/crimson/common/fixed_kv_node_layout.h
+++ b/src/crimson/common/fixed_kv_node_layout.h
@@ -310,6 +310,12 @@ public:
     void clear() {
       buffer.clear();
     }
+    template <typename Func>
+    void for_each(Func &&f) {
+      for (auto &i : buffer) {
+        std::invoke(std::forward<Func>(f), i);
+      }
+    }
   };
 
   void journal_insert(

diff --git a/src/crimson/os/seastore/cached_extent.cc b/src/crimson/os/seastore/cached_extent.cc
index d5abf2e085326f8075a4e7b2c159d548e31e42d3..06b64e49da52d35047dda8ac1cd7b2cdbb421e49 100644 (file)
--- a/src/crimson/os/seastore/cached_extent.cc
+++ b/src/crimson/os/seastore/cached_extent.cc
@@ -434,12 +434,37 @@ void ExtentCommitter::_share_prior_data_to_mutations() {
   ceph_assert(is_lba_backref_node(extent.get_type()));
   auto &prior = *extent.prior_instance;
   for (auto &mext : prior.mutation_pending_extents) {
-    auto &mextent = static_cast<CachedExtent&>(mext);
-    TRACE("{} -> {}", extent, mextent);
-    extent.get_bptr().copy_out(
-      0, extent.get_length(), mextent.get_bptr().c_str());
-    mextent.on_data_commit();
-    mextent.reapply_delta();
+    if (extent.get_type() == extent_types_t::LADDR_LEAF) {
+      // LBA leaf mappings contains other fields than just pladdr, which
+      // may also be modified. In this case, we can just overwrite the
+      // whole contents of the leaf node and reapply deltas like what
+      // we do for internal nodes.
+      auto &mextent = static_cast<lba::LBALeafNode&>(mext);
+      auto &me = static_cast<lba::LBALeafNode&>(extent);
+      TRACE("{} -> {}", me, mextent);
+      auto iter = me.begin();
+      auto merged = me.merge_content_to(t, mextent, iter);
+      mextent.adjust_delta([&](auto &buf) {
+        if (buf.op == lba::LBALeafNode::delta_t::op_t::UPDATE ||
+            // only remapping extents can create a delta with op
+            // INSERT and the corresponding mapping in "merged"
+            buf.op == lba::LBALeafNode::delta_t::op_t::INSERT) {
+          auto it = merged.find(buf.key);
+          if (it != merged.end()) {
+            TRACE("{} -> {}, {} -> {}",
+              me, mextent, (pladdr_t)buf.val.pladdr, it->second);
+            buf.val.pladdr = pladdr_le_t(it->second);
+          }
+        }
+      });
+    } else {
+      auto &mextent = static_cast<CachedExtent&>(mext);
+      TRACE("{} -> {}", extent, mextent);
+      extent.get_bptr().copy_out(
+        0, extent.get_length(), mextent.get_bptr().c_str());
+      mextent.on_data_commit();
+      mextent.reapply_delta();
+    }
   }
 }
 

diff --git a/src/crimson/os/seastore/lba/lba_btree_node.h b/src/crimson/os/seastore/lba/lba_btree_node.h
index b6831f6a210f1f6d5fcbc09d7ca7eab1000292d2..8f6a04f2aedfc267f32ab21934c0045f47b8cf9c 100644 (file)
--- a/src/crimson/os/seastore/lba/lba_btree_node.h
+++ b/src/crimson/os/seastore/lba/lba_btree_node.h
@@ -288,47 +288,96 @@ struct LBALeafNode
 
   std::ostream &print_detail(std::ostream &out) const final;
 
-  template <template <typename...> typename Container, typename... T>
-  void merge_content_to(Transaction &t, Container<T...> &container) {
-    auto iter = this->begin();
-    for (auto &copy_dest : container) {
-      auto &pending_version = static_cast<LBALeafNode&>(*copy_dest);
-      auto it = pending_version.begin();
-      while (it != pending_version.end() && iter != this->end()) {
-        const auto &v1 = iter->get_val();
-        if (v1.pladdr.is_laddr() ||
-            v1.pladdr.get_paddr().is_zero()) {
-          iter++;
-          continue;
-        }
-        if (const auto &v2 = it->get_val();
-            v2.pladdr.is_laddr() || v2.pladdr.get_paddr().is_zero()) {
-          it++;
-          continue;
+  std::map<laddr_t, pladdr_t> merge_content_to(
+    Transaction &t,
+    LBALeafNode &pending_version,
+    iterator &iter)
+  {
+    LOG_PREFIX(LBALeafNode::merge_content_to);
+    std::map<laddr_t, pladdr_t> modified;
+    auto it = pending_version.begin();
+    while (it != pending_version.end() && iter != this->end()) {
+      const auto &v1 = iter->get_val();
+      if (v1.pladdr.is_laddr() ||
+          v1.pladdr.get_paddr().is_zero()) {
+        iter++;
+        continue;
+      }
+      const auto &v2 = it->get_val();
+      if (v2.pladdr.is_laddr() || v2.pladdr.get_paddr().is_zero()) {
+        it++;
+        continue;
+      }
+      auto child = pending_version.children[it->get_offset()];
+      if (unlikely(is_reserved_ptr(child))) {
+        SUBERRORT(seastore_lba, "unexpected reserved ptr for {}, {}",
+          t, it->get_key(), pending_version);
+        ceph_abort();
+      }
+      if (is_valid_child_ptr(child) &&
+          (child->_is_mutable() || child->_is_pending_io())) {
+        // skip the ones that the pending version is also modifying
+        it++;
+        continue;
+      }
+      auto pending_key = it->get_key();
+      auto stable_key = iter->get_key();
+      auto stable_end = stable_key + v1.len;
+      if (pending_key < stable_end && pending_key >= stable_key) {
+        auto pending_end = pending_key + v2.len;
+        assert(pending_end <= stable_end);
+        if (pending_key != stable_key) {
+          assert(v2.pladdr != v1.pladdr);
+          assert(!is_valid_child_ptr(child) ||
+                 !child->_is_exist_clean() ||
+                 !child->_is_exist_mutation_pending());
         }
-        if (auto child = pending_version.children[it->get_offset()];
-            is_valid_child_ptr(child) &&
-            (child->_is_mutable() || child->_is_pending_io())) {
-          // skip the ones that the pending version is also modifying
-          it++;
-          continue;
+        if (v2.pladdr != v1.pladdr) {
+          auto m_v2 = v2;
+          auto off = pending_key.get_byte_distance<extent_len_t>(stable_key);
+          auto paddr = v1.pladdr.get_paddr();
+          paddr = paddr + off;
+          m_v2.pladdr = paddr;
+          SUBTRACET(seastore_lba, "merging to {}, paddr: {} -> {}",
+            t, pending_version, m_v2.pladdr, paddr);
+          if (!is_valid_child_ptr(child) ||
+              (!child->_is_exist_clean() &&
+               !child->_is_exist_mutation_pending())) {
+            // exclude the mappings whose children are EXIST_CLEAN ones
+            SUBTRACET(seastore_lba, "merging to {}, checksum: {} -> {}",
+              t, pending_version, m_v2.checksum, v1.checksum);
+            m_v2.checksum = v1.checksum;
+          }
+          it->set_val(m_v2);
+          auto [_it, inserted] = modified.emplace(it->get_key(), paddr);
+          ceph_assert(inserted);
         }
-        if (it->get_key() == iter->get_key()) {
-          it->set_val(v1);
-          it++;
+        it++;
+        if (pending_end == stable_end) {
           iter++;
-        } else if (it->get_key() > iter->get_key()) {
-          iter++;
-        } else {
-          it++;
         }
-      }
-      if (pending_version.get_last_committed_crc()) {
-        // if pending_version has already calculated its crc,
-        // calculate it again.
-        pending_version.set_last_committed_crc(pending_version.calc_crc32c());
+      } else if (pending_key > stable_key) {
+        iter++;
+      } else {
+        it++;
       }
     }
+    if (pending_version.is_initial_pending() &&
+        pending_version.get_last_committed_crc()) {
+      // if pending_version has already calculated its crc,
+      // calculate it again.
+      pending_version.set_last_committed_crc(pending_version.calc_crc32c());
+    }
+    return modified;
+  }
+
+  template <template <typename...> typename Container, typename... T>
+  void merge_content_to(Transaction &t, Container<T...> &container) {
+    auto iter = this->begin();
+    for (auto &copy_dest : container) {
+      auto &pending_version = static_cast<LBALeafNode&>(*copy_dest);
+      std::ignore = this->merge_content_to(t, pending_version, iter);
+    }
   }
 
   void merge_content_to_pending_versions(Transaction &t) {
@@ -343,6 +392,12 @@ struct LBALeafNode
       this->merge_content_to(t, copy_dests.dests_by_key);
     });
   }
+
+  template <typename Func>
+  void adjust_delta(Func &&f) {
+    ceph_assert(this->is_mutation_pending());
+    this->delta_buffer.for_each(std::forward<Func>(f));
+  }
 };
 using LBALeafNodeRef = TCachedExtentRef<LBALeafNode>;