From: Matan Breizman Date: Mon, 9 Feb 2026 08:49:39 +0000 (+0000) Subject: Revert "crimson/os/seastore: disable linked tree node operations when committing" X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=c7f22e4496488d52642eea2bc8581f7d10231b88;p=ceph.git Revert "crimson/os/seastore: disable linked tree node operations when committing" This reverts commit 21d76b9673264ac5817d01e9e11401fbba20e895. Signed-off-by: Matan Breizman --- diff --git a/src/crimson/os/seastore/btree/fixed_kv_node.h b/src/crimson/os/seastore/btree/fixed_kv_node.h index fdd67e49656..ce1124e66a3 100644 --- a/src/crimson/os/seastore/btree/fixed_kv_node.h +++ b/src/crimson/os/seastore/btree/fixed_kv_node.h @@ -185,10 +185,8 @@ struct FixedKVInternalNode this->set_layout_buf(this->get_bptr().c_str()); } - void prepare_commit(Transaction &t) final { - if (!is_rewrite_transaction(t.get_src())) { - parent_node_t::prepare_commit(); - } + void prepare_commit() final { + parent_node_t::prepare_commit(); } virtual ~FixedKVInternalNode() { @@ -248,14 +246,12 @@ struct FixedKVInternalNode delta_buffer.clear(); } - void on_replace_prior(Transaction &t) final { - if (!is_rewrite_transaction(t.get_src())) { - this->parent_node_t::on_replace_prior(); - if (this->is_btree_root()) { - this->root_node_t::on_replace_prior(); - } else { - this->child_node_t::on_replace_prior(); - } + void on_replace_prior() final { + this->parent_node_t::on_replace_prior(); + if (this->is_btree_root()) { + this->root_node_t::on_replace_prior(); + } else { + this->child_node_t::on_replace_prior(); } } @@ -676,23 +672,19 @@ struct FixedKVLeafNode } virtual void do_prepare_commit() = 0; - void prepare_commit(Transaction &t) final { - if (!is_rewrite_transaction(t.get_src())) { - do_prepare_commit(); - } + void prepare_commit() final { + do_prepare_commit(); modifications = 0; } virtual void do_on_replace_prior() = 0; - void on_replace_prior(Transaction &t) final { + void on_replace_prior() final { ceph_assert(!this->is_rewrite()); - if (!is_rewrite_transaction(t.get_src())) { - do_on_replace_prior(); - if (this->is_btree_root()) { - this->root_node_t::on_replace_prior(); - } else { - this->child_node_t::on_replace_prior(); - } + do_on_replace_prior(); + if (this->is_btree_root()) { + this->root_node_t::on_replace_prior(); + } else { + this->child_node_t::on_replace_prior(); } modifications = 0; } diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index 1bb973c8c00..f161930fc5a 100644 --- a/src/crimson/os/seastore/cache.cc +++ b/src/crimson/os/seastore/cache.cc @@ -1348,10 +1348,10 @@ record_t Cache::prepare_record( } i->prepare_write(); - i->prepare_commit(t); + i->prepare_commit(); if (i->is_mutation_pending()) { - i->on_replace_prior(t); + i->on_replace_prior(); } // else, is_exist_mutation_pending(): // - it doesn't have prior_instance to replace @@ -1414,12 +1414,12 @@ record_t Cache::prepare_record( i->trans_view_hook.unlink(); } - t.for_each_finalized_fresh_block([&t](auto &e) { + t.for_each_finalized_fresh_block([](auto &e) { // fresh blocks' `prepare_commit` must be invoked before // retiering extents, this is because logical linked tree // nodes needs to access their prior instances in this // phase if they are rewritten. - e->prepare_commit(t); + e->prepare_commit(); }); /* diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h index a0ef8b49d66..ed521561111 100644 --- a/src/crimson/os/seastore/cached_extent.h +++ b/src/crimson/os/seastore/cached_extent.h @@ -399,7 +399,7 @@ public: * Called prior to committing the transaction in which this extent * is living. */ - virtual void prepare_commit(Transaction &) {} + virtual void prepare_commit() {} /** * on_initial_write @@ -450,7 +450,7 @@ public: * with the states of Cache and can't wait till transaction * completes. */ - virtual void on_replace_prior(Transaction &) {} + virtual void on_replace_prior() {} /** * on_invalidated diff --git a/src/crimson/os/seastore/logical_child_node.h b/src/crimson/os/seastore/logical_child_node.h index e143e323755..1fc28d35489 100644 --- a/src/crimson/os/seastore/logical_child_node.h +++ b/src/crimson/os/seastore/logical_child_node.h @@ -128,14 +128,12 @@ public: return (get_laddr() + get_length()).checked_to_laddr(); } protected: - void on_replace_prior(Transaction &t) final { + void on_replace_prior() final { assert(is_seen_by_users()); - if (!is_rewrite_transaction(t.get_src())) { - lba_child_node_t::on_replace_prior(); - } - do_on_replace_prior(t); + lba_child_node_t::on_replace_prior(); + do_on_replace_prior(); } - virtual void do_on_replace_prior(Transaction &t) {} + virtual void do_on_replace_prior() {} void on_data_commit() final { ceph_abort("impossible"); } diff --git a/src/crimson/os/seastore/object_data_handler.h b/src/crimson/os/seastore/object_data_handler.h index 73c0953668a..ffb22246b4f 100644 --- a/src/crimson/os/seastore/object_data_handler.h +++ b/src/crimson/os/seastore/object_data_handler.h @@ -294,10 +294,7 @@ struct ObjectDataBlock : crimson::os::seastore::LogicalChildNode { modified_region.clear(); } - void prepare_commit(Transaction &t) final { - if (is_rewrite_transaction(t.get_src())) { - return; - } + void prepare_commit() final { if (has_mutation()) { ceph_assert(!cached_overwrites.is_empty()); if (cached_overwrites.has_cached_bptr()) { 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 4d238ff185c..42f127b1a29 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 @@ -80,10 +80,7 @@ struct OMapInnerNode } } - void prepare_commit(Transaction &t) final { - if (is_rewrite_transaction(t.get_src())) { - return; - } + void prepare_commit() final { if (unlikely(!is_seen_by_users())) { ceph_assert(is_rewrite()); auto &prior = *get_prior_instance()->template cast(); @@ -114,10 +111,7 @@ struct OMapInnerNode } } - void do_on_replace_prior(Transaction &t) final { - if (is_rewrite_transaction(t.get_src())) { - return; - } + void do_on_replace_prior() final { this->parent_node_t::on_replace_prior(); if (!this->is_btree_root()) { auto &prior = *get_prior_instance()->template cast(); @@ -343,10 +337,7 @@ struct OMapLeafNode this->child_node_t::on_invalidated(); } - void prepare_commit(Transaction &t) final { - if (is_rewrite_transaction(t.get_src())) { - return; - } + void prepare_commit() final { if (unlikely(!is_seen_by_users())) { ceph_assert(is_rewrite()); auto &prior = *get_prior_instance()->template cast(); @@ -376,9 +367,9 @@ struct OMapLeafNode } } - void do_on_replace_prior(Transaction &t) final { + void do_on_replace_prior() final { ceph_assert(!this->is_rewrite()); - if (!this->is_btree_root() && !is_rewrite_transaction(t.get_src())) { + if (!this->is_btree_root()) { auto &prior = *get_prior_instance()->template cast(); assert(prior.base_child_t::has_parent_tracker()); this->child_node_t::on_replace_prior(); diff --git a/src/crimson/os/seastore/root_block.cc b/src/crimson/os/seastore/root_block.cc index 574b500a860..3f61b92fdaa 100644 --- a/src/crimson/os/seastore/root_block.cc +++ b/src/crimson/os/seastore/root_block.cc @@ -8,11 +8,8 @@ namespace crimson::os::seastore { -void RootBlock::on_replace_prior(Transaction &t) { - if (!lba_root_node || - // for rewrite transactions, we keep the prior extents instead of - // the new ones. - is_rewrite_transaction(t.get_src())) { +void RootBlock::on_replace_prior() { + if (!lba_root_node) { auto &prior = static_cast(*get_prior_instance()); if (prior.lba_root_node) { RootBlockRef this_ref = this; @@ -32,10 +29,7 @@ void RootBlock::on_replace_prior(Transaction &t) { } } } - if (!backref_root_node || - // for rewrite transactions, we keep the prior extents instead of - // the new ones. - is_rewrite_transaction(t.get_src())) { + if (!backref_root_node) { auto &prior = static_cast(*get_prior_instance()); if (prior.backref_root_node) { RootBlockRef this_ref = this; diff --git a/src/crimson/os/seastore/root_block.h b/src/crimson/os/seastore/root_block.h index ad0cc40a900..3913be52415 100644 --- a/src/crimson/os/seastore/root_block.h +++ b/src/crimson/os/seastore/root_block.h @@ -61,7 +61,7 @@ struct RootBlock : CachedExtent { return extent_types_t::ROOT; } - void on_replace_prior(Transaction &t) final; + void on_replace_prior() final; /// dumps root as delta ceph::bufferlist get_delta() final {