From 9a2d281886fca126308e06f5838af0c5b81efc39 Mon Sep 17 00:00:00 2001 From: Xuehan Xu Date: Wed, 21 May 2025 11:50:23 +0800 Subject: [PATCH] crimson/os/seastore/omap_manager: handle the cases in which omap nodes are rewritten before seen by users Fixes: https://tracker.ceph.com/issues/71383 Signed-off-by: Xuehan Xu --- .../omap_manager/btree/omap_btree_node.h | 4 ++ .../omap_manager/btree/omap_btree_node_impl.h | 48 +++++++++++++------ 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/src/crimson/os/seastore/omap_manager/btree/omap_btree_node.h b/src/crimson/os/seastore/omap_manager/btree/omap_btree_node.h index cef4830258d..87ae7c5f8fd 100644 --- a/src/crimson/os/seastore/omap_manager/btree/omap_btree_node.h +++ b/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; diff --git a/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.h b/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.h index 46d8f1bc7f3..e77f8dec65c 100644 --- a/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.h +++ b/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(); + 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(); - 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(); + if (!prior.is_seen_by_users()) { + return; + } + set_seen_by_users(); + } + if (is_rewrite()) { auto &prior = *get_prior_instance()->template cast(); - 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()); } } } -- 2.39.5