crimson/os/seastore: fix bug in check_node

EXIST_CLEAN and EXIST_MUTATION_PENDING shuold not be treated as
CLEAN in check_node because they are transaction private and the
leafnode has been duplicated for write.

fix: https://tracker.ceph.com/issues/61626

Signed-off-by: Xinyu Huang <xinyu.huang@intel.com>

diff --git a/src/crimson/os/seastore/btree/fixed_kv_btree.h b/src/crimson/os/seastore/btree/fixed_kv_btree.h
index 2aaf1620fcb4d3ac9c05075ad2f74a762a8339f3..197c198787f766e3773cc6b6874f3a982d725b88 100644 (file)
--- a/src/crimson/os/seastore/btree/fixed_kv_btree.h
+++ b/src/crimson/os/seastore/btree/fixed_kv_btree.h
@@ -506,28 +506,7 @@ public:
         }
       }
       if (ret == Transaction::get_extent_ret::PRESENT) {
-        if (child_node->is_mutation_pending()) {
-          auto &prior = (child_node_t &)*child_node->prior_instance;
-          assert(prior.is_valid());
-          assert(prior.is_parent_valid());
-          if (node->is_mutation_pending()) {
-            auto &n = node->get_stable_for_key(i->get_key());
-            assert(prior.get_parent_node().get() == &n);
-            auto pos = n.lower_bound_offset(i->get_key());
-            assert(pos < n.get_node_size());
-            assert(n.children[pos] == &prior);
-          } else {
-            assert(prior.get_parent_node().get() == node.get());
-            assert(node->children[i->get_offset()] == &prior);
-          }
-        } else if (child_node->is_initial_pending()) {
-          auto cnode = child_node->template cast<child_node_t>();
-          auto pos = node->find(i->get_key()).get_offset();
-          auto child = node->children[pos];
-          assert(child);
-          assert(child == cnode.get());
-          assert(cnode->is_parent_valid());
-        } else {
+        if (child_node->is_stable()) {
           assert(child_node->is_valid());
           auto cnode = child_node->template cast<child_node_t>();
           assert(cnode->has_parent_tracker());
@@ -541,6 +520,32 @@ public:
             assert(cnode->get_parent_node().get() == node.get());
             assert(node->children[i->get_offset()] == cnode.get());
           }
+        } else if (child_node->is_pending()) {
+          if (child_node->is_mutation_pending()) {
+            auto &prior = (child_node_t &)*child_node->prior_instance;
+            assert(prior.is_valid());
+            assert(prior.is_parent_valid());
+            if (node->is_mutation_pending()) {
+              auto &n = node->get_stable_for_key(i->get_key());
+              assert(prior.get_parent_node().get() == &n);
+              auto pos = n.lower_bound_offset(i->get_key());
+              assert(pos < n.get_node_size());
+              assert(n.children[pos] == &prior);
+            } else {
+              assert(prior.get_parent_node().get() == node.get());
+              assert(node->children[i->get_offset()] == &prior);
+            }
+          } else {
+            auto cnode = child_node->template cast<child_node_t>();
+            auto pos = node->find(i->get_key()).get_offset();
+            auto child = node->children[pos];
+            assert(child);
+            assert(child == cnode.get());
+            assert(cnode->is_parent_valid());
+          }
+        } else {
+          ceph_assert(!child_node->is_valid());
+          ceph_abort("impossible");
         }
       } else if (ret == Transaction::get_extent_ret::ABSENT) {
         ChildableCachedExtent* child = nullptr;

diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h
index 871ad8f08e15e6b9849b2a38e9c04bf05e54969e..2bf6279133eb0091439573491f4c1412f8f9f000 100644 (file)
--- a/src/crimson/os/seastore/cached_extent.h
+++ b/src/crimson/os/seastore/cached_extent.h
@@ -415,6 +415,13 @@ public:
     return is_mutable() || state == extent_state_t::EXIST_CLEAN;
   }
 
+  /// Returns true if extent is stable and shared among transactions
+  bool is_stable() const {
+    return state == extent_state_t::CLEAN_PENDING ||
+      state == extent_state_t::CLEAN ||
+      state == extent_state_t::DIRTY;
+  }
+
   /// Returns true if extent has a pending delta
   bool is_mutation_pending() const {
     return state == extent_state_t::MUTATION_PENDING;