From: Yingxin Cheng <yingxin.cheng@intel.com>
Date: Thu, 22 Apr 2021 08:46:44 +0000 (+0800)
Subject: crimson/onode-staged-tree: pass origin_left_addr to apply_children_merge()
X-Git-Tag: v17.1.0~2110^2~6
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=9f821d0ee21a450e4940cd25e075ab7da2fcbd6d;p=ceph.git

crimson/onode-staged-tree: pass origin_left_addr to apply_children_merge()

rebuild_extent() may change the address of the left child, we need to
pass the origin_left_addr in order to validate if it is correct.

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
---

diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc b/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc
index a4594b3f5c7d..2096fa5eecbd 100644
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc
@@ -555,14 +555,16 @@ Node::try_merge_adjacent(context_t c, bool update_parent_index)
           update_index_after_merge = update_parent_index;
         }
         logger().info("OTree::Node::MergeAdjacent: merge {} and {} "
-                      "at merge_stage={}, merge_size={}B, update_index={} ...",
+                      "at merge_stage={}, merge_size={}B, update_index={}, is_left={} ...",
                       left_for_merge->get_name(), right_for_merge->get_name(),
-                      merge_stage, merge_size, update_index_after_merge);
+                      merge_stage, merge_size, update_index_after_merge, is_left);
         // we currently cannot generate delta depends on another extent content,
         // so use rebuild_extent() as a workaround to rebuild the node from a
         // fresh extent, thus no need to generate delta.
+        auto left_addr = left_for_merge->impl->laddr();
         return left_for_merge->rebuild_extent(c
         ).safe_then([c, merge_stage, merge_size, update_index_after_merge,
+                     left_addr,
                      left_for_merge = std::move(left_for_merge),
                      right_for_merge = std::move(right_for_merge)] (auto left_mut) mutable {
           if (left_for_merge->impl->node_type() == node_type_t::LEAF) {
@@ -573,7 +575,7 @@ Node::try_merge_adjacent(context_t c, bool update_parent_index)
               left_mut, *right_for_merge->impl, merge_stage, merge_size);
           left_for_merge->track_merge(right_for_merge, merge_stage, left_last_pos);
           return left_for_merge->parent_info().ptr->apply_children_merge(
-              c, std::move(left_for_merge),
+              c, std::move(left_for_merge), left_addr,
               std::move(right_for_merge), update_index_after_merge);
         });
       } else {
@@ -946,16 +948,16 @@ node_future<> InternalNode::fix_index(context_t c, Ref<Node> child)
 
 template <bool FORCE_MERGE>
 node_future<> InternalNode::apply_children_merge(
-    context_t c, Ref<Node>&& left_child,
+    context_t c, Ref<Node>&& left_child, laddr_t origin_left_addr,
     Ref<Node>&& right_child, bool update_index)
 {
   auto left_pos = left_child->parent_info().position;
   auto left_addr = left_child->impl->laddr();
   auto& right_pos = right_child->parent_info().position;
   auto right_addr = right_child->impl->laddr();
-  logger().debug("OTree::Internal::ApplyChildMerge: apply {}'s child "
-                 "{} at pos({}), to merge with {} at pos({}), update_index={} ...",
-                 get_name(), left_child->get_name(), left_pos,
+  logger().debug("OTree::Internal::ApplyChildMerge: apply {}'s child {} (was {:#x}) "
+                 "at pos({}), to merge with {} at pos({}), update_index={} ...",
+                 get_name(), left_child->get_name(), origin_left_addr, left_pos,
                  right_child->get_name(), right_pos, update_index);
 
 #ifndef NDEBUG
@@ -963,7 +965,7 @@ node_future<> InternalNode::apply_children_merge(
   assert(!left_pos.is_end());
   const laddr_packed_t* p_value_left;
   impl->get_slot(left_pos, nullptr, &p_value_left);
-  assert(p_value_left->value == left_addr);
+  assert(p_value_left->value == origin_left_addr);
 
   assert(right_child->use_count() == 1);
   assert(right_child->parent_info().ptr == this);
@@ -1031,9 +1033,9 @@ node_future<> InternalNode::apply_children_merge(
   });
 }
 template node_future<> InternalNode::apply_children_merge<true>(
-    context_t, Ref<Node>&&, Ref<Node>&&, bool);
+    context_t, Ref<Node>&&, laddr_t, Ref<Node>&&, bool);
 template node_future<> InternalNode::apply_children_merge<false>(
-    context_t, Ref<Node>&&, Ref<Node>&&, bool);
+    context_t, Ref<Node>&&, laddr_t, Ref<Node>&&, bool);
 
 node_future<std::pair<Ref<Node>, Ref<Node>>> InternalNode::get_child_peers(
     context_t c, const search_position_t& pos)
diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node.h
index 94f1062d02a9..2f4df94c825c 100644
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/node.h
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node.h
@@ -524,7 +524,7 @@ class InternalNode final : public Node {
 
   template <bool FORCE_MERGE = false>
   node_future<> apply_children_merge(
-      context_t, Ref<Node>&& left, Ref<Node>&& right, bool update_index);
+      context_t, Ref<Node>&& left, laddr_t, Ref<Node>&& right, bool update_index);
 
   void validate_child_tracked(const Node& child) const {
     validate_child(child);
diff --git a/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc b/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc
index 35bc3be56bd9..e81e3c0421bc 100644
--- a/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc
+++ b/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc
@@ -1071,7 +1071,7 @@ class DummyChildPool {
       }
       left->impl->reset(*p_keys, left_is_tail);
       return left->parent_info().ptr->apply_children_merge<true>(
-          c, std::move(left), std::move(right), !stole_key);
+          c, std::move(left), left->impl->laddr(), std::move(right), !stole_key);
     }
 
     DummyChildImpl* impl;