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..a0e599d65f1f2a8c00f3a708fc837d7d44e5a25a 100644 (file)
--- a/src/crimson/os/seastore/cached_extent.cc
+++ b/src/crimson/os/seastore/cached_extent.cc
@@ -434,12 +434,28 @@ 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) {
+      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) {
+          auto it = merged.find(buf.key);
+          if (it != merged.end()) {
+            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 c0faa45b880cfd8afa203dee31bb1005ed8a4610..addbabd74eeca34aae536e0d61ac922fb2c13a8f 100644 (file)
--- a/src/crimson/os/seastore/lba/lba_btree_node.h
+++ b/src/crimson/os/seastore/lba/lba_btree_node.h
@@ -288,45 +288,63 @@ struct LBALeafNode
 
   std::ostream &print_detail(std::ostream &out) const final;
 
+  std::map<laddr_t, pladdr_t> merge_content_to(
+    Transaction &t,
+    LBALeafNode &pending_version,
+    iterator &iter)
+  {
+    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;
+      }
+      if (auto child = pending_version.children[it->get_offset()];
+          is_valid_child_ptr(child) && child->_is_pending_io()) {
+        // skip the ones that the pending version is also modifying
+        it++;
+        continue;
+      }
+      if (it->get_key() == iter->get_key()) {
+        if (v2.pladdr != v1.pladdr) {
+          auto m_v2 = v2;
+          m_v2.pladdr = v1.pladdr;
+          m_v2.checksum = v1.checksum;
+          it->set_val(m_v2);
+          auto [_it, inserted] = modified.emplace(it->get_key(), v1.pladdr);
+          ceph_assert(inserted);
+        }
+        it++;
+        iter++;
+      } else if (it->get_key() > iter->get_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);
-      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;
-        }
-        if (auto child = pending_version.children[it->get_offset()];
-            is_valid_child_ptr(child) && child->_is_pending_io()) {
-          // skip the ones that the pending version is also modifying
-          it++;
-          continue;
-        }
-        if (it->get_key() == iter->get_key()) {
-          it->set_val(v1);
-          it++;
-          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());
-      }
+      std::ignore = this->merge_content_to(t, pending_version, iter);
     }
   }
 
@@ -342,6 +360,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>;