Merge pull request ceph#63376 from xxhdx1985126/wip-seastore-omap-root-fix

crimson/os/seastore/omap_manager: handle the cases in which omap nodes are rewritten before seen by users

Reviewed-by: Yingxin Cheng <[email protected]>

Author: cyx1231st

src/crimson/os/seastore/omap_manager/btree/omap_btree_node.h

@@ -137,6 +137,10 @@ struct OMapNode : LogicalChildNode {
     return end;
   }
   bool is_btree_root() const { return root; }
+protected:
+  void set_root(bool is_root) {
+    root = is_root;
+  }
 private:
   bool root = false;
   std::string begin;

src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.h

@@ -76,19 +76,29 @@ struct OMapInnerNode
   }
 
   void prepare_commit() final {
+    if (unlikely(!is_seen_by_users())) {
+      ceph_assert(is_rewrite());
+      auto &prior = *get_prior_instance()->template cast<OMapInnerNode>();
+      if (!prior.is_seen_by_users()) {
+	return;
+      }
+      set_seen_by_users();
+    }
     this->parent_node_t::prepare_commit();
-    if (is_rewrite() && !is_btree_root()) {
+    if (is_rewrite()) {
       auto &prior = *get_prior_instance()->template cast<OMapInnerNode>();
-      if (prior.base_child_t::has_parent_tracker()) {
-        assert(prior.is_seen_by_users());
+      assert(prior.is_seen_by_users());
+      // Chances are that this transaction is in parallel with another
+      // user transaction that set the prior's root to true, so we need
+      // to do this.
+      set_root(prior.is_btree_root());
+      if (!is_btree_root()) {
+	assert(prior.base_child_t::has_parent_tracker());
+	assert(prior.is_seen_by_users());
 	// unlike fixed-kv nodes, rewriting child nodes of the omap tree
 	// won't affect parent nodes, so we have to manually take prior
 	// instances' parent trackers here.
 	this->child_node_t::take_parent_from_prior();
-      } else {
-        // dirty omap extent may not be accessed yet during rewrite,
-        // this means the extent may not be initalized yet as linked.
-        assert(!prior.is_seen_by_users());
       }
     }
   }
@@ -292,18 +302,28 @@ struct OMapLeafNode
   }
 
   void prepare_commit() final {
-    if (is_rewrite() && !is_btree_root()) {
+    if (unlikely(!is_seen_by_users())) {
+      ceph_assert(is_rewrite());
+      auto &prior = *get_prior_instance()->template cast<OMapLeafNode>();
+      if (!prior.is_seen_by_users()) {
+	return;
+      }
+      set_seen_by_users();
+    }
+    if (is_rewrite()) {
       auto &prior = *get_prior_instance()->template cast<OMapLeafNode>();
-      if (prior.base_child_t::has_parent_tracker()) {
-        assert(prior.is_seen_by_users());
+      assert(prior.is_seen_by_users());
+      // Chances are that this transaction is in parallel with another
+      // user transaction that set the prior's root to true, so we need
+      // to do this.
+      set_root(prior.is_btree_root());
+      if (!is_btree_root()) {
+	assert(prior.base_child_t::has_parent_tracker());
+	assert(prior.is_seen_by_users());
 	// unlike fixed-kv nodes, rewriting child nodes of the omap tree
 	// won't affect parent nodes, so we have to manually take prior
 	// instances' parent trackers here.
 	this->child_node_t::take_parent_from_prior();
-      } else {
-        // dirty omap extent may not be accessed yet during rewrite,
-        // this means the extent may not be initalized yet as linked.
-        assert(!prior.is_seen_by_users());
       }
     }
   }