From f02f20c2aecfbcb24d4a2d49db159a15b63487f4 Mon Sep 17 00:00:00 2001 From: Xinyu Huang Date: Fri, 9 Jun 2023 15:25:48 +0800 Subject: [PATCH] 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 --- .../os/seastore/btree/fixed_kv_btree.h | 49 ++++++++++--------- src/crimson/os/seastore/cached_extent.h | 7 +++ 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/src/crimson/os/seastore/btree/fixed_kv_btree.h b/src/crimson/os/seastore/btree/fixed_kv_btree.h index 2aaf1620fcb4d..197c198787f76 100644 --- 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(); - 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(); 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(); + 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 871ad8f08e15e..2bf6279133eb0 100644 --- 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; -- 2.39.5