From 82311ad109db92ecaad1121e22eb5e3cfd288a27 Mon Sep 17 00:00:00 2001 From: Xuehan Xu Date: Thu, 11 Dec 2025 16:11:08 +0800 Subject: [PATCH] fixup! crimson/os/seastore/cache: add facilities to synchronize data and states between rewriting trasactions and others when committing --- src/crimson/os/seastore/btree/fixed_kv_node.h | 12 ++---------- src/crimson/os/seastore/cache.cc | 4 ++-- src/crimson/os/seastore/cached_extent.cc | 14 ++++++++++---- src/crimson/os/seastore/cached_extent.h | 5 ----- .../collection_manager/collection_flat_node.h | 6 ------ src/crimson/os/seastore/logical_child_node.h | 3 +++ src/crimson/os/seastore/transaction.h | 2 +- 7 files changed, 18 insertions(+), 28 deletions(-) diff --git a/src/crimson/os/seastore/btree/fixed_kv_node.h b/src/crimson/os/seastore/btree/fixed_kv_node.h index b11031a6ea1..fdd67e49656 100644 --- a/src/crimson/os/seastore/btree/fixed_kv_node.h +++ b/src/crimson/os/seastore/btree/fixed_kv_node.h @@ -210,11 +210,7 @@ struct FixedKVInternalNode } void on_data_commit() final { - auto &prior = static_cast(*this->get_prior_instance()); - if (this->is_mutation_pending()) { - prior.delta_buffer = std::move(delta_buffer); - } - prior.set_layout_buf(prior.get_bptr().c_str()); + this->set_layout_buf(this->get_bptr().c_str()); } void on_invalidated(Transaction &t) final { @@ -633,11 +629,7 @@ struct FixedKVLeafNode } void on_data_commit() final { - auto &prior = static_cast(*this->get_prior_instance()); - if (this->is_mutation_pending()) { - prior.delta_buffer = std::move(delta_buffer); - } - prior.set_layout_buf(prior.get_bptr().c_str()); + this->set_layout_buf(this->get_bptr().c_str()); } void sync_layout_buf() { diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index 3704790de5c..4bef9db84e5 100644 --- a/src/crimson/os/seastore/cache.cc +++ b/src/crimson/os/seastore/cache.cc @@ -1952,9 +1952,9 @@ void Cache::complete_commit( committer.commit_and_share_paddr(); if (is_lba_backref_node(i->get_type())) { committer.commit_data(); - committer.share_prior_data_to_pending(); } touch_extent_fully(prior, &t_src, t.get_cache_hint()); + committer.sync_version(); committer.unblock_trans(t); prior.complete_io(); i->committer.reset(); @@ -2038,8 +2038,8 @@ void Cache::complete_commit( ceph_assert(prior.is_valid()); if (is_lba_backref_node(i->get_type())) { committer.commit_data(); - committer.share_prior_data_to_pending(); } + committer.sync_version(); prior.complete_io(); prior.clear_delta(); i->committer.reset(); diff --git a/src/crimson/os/seastore/cached_extent.cc b/src/crimson/os/seastore/cached_extent.cc index 3063c7cb01b..d5abf2e0853 100644 --- a/src/crimson/os/seastore/cached_extent.cc +++ b/src/crimson/os/seastore/cached_extent.cc @@ -379,8 +379,11 @@ void ExtentCommitter::sync_checksum() { void ExtentCommitter::commit_data() { assert(extent.prior_instance); // extent and its prior are sharing the same bptr content - extent.prior_instance->set_bptr(extent.get_bptr()); - extent.on_data_commit(); + auto &prior = *extent.prior_instance; + prior.set_bptr(extent.get_bptr()); + prior.on_data_commit(); + _share_prior_data_to_mutations(); + _share_prior_data_to_pending_versions(); } void ExtentCommitter::commit_state() { @@ -402,8 +405,6 @@ void ExtentCommitter::commit_state() { prior.version = extent.version; prior.user_hint = extent.user_hint; prior.rewrite_generation = extent.rewrite_generation; - prior.last_touch_end = extent.last_touch_end; - prior.cache_state = extent.cache_state; prior.state = extent.state; extent.on_state_commit(); } @@ -429,17 +430,22 @@ void ExtentCommitter::commit_and_share_paddr() { } void ExtentCommitter::_share_prior_data_to_mutations() { + LOG_PREFIX(ExtentCommitter::_share_prior_data_to_mutations); + ceph_assert(is_lba_backref_node(extent.get_type())); auto &prior = *extent.prior_instance; for (auto &mext : prior.mutation_pending_extents) { auto &mextent = static_cast(mext); + TRACE("{} -> {}", extent, mextent); extent.get_bptr().copy_out( 0, extent.get_length(), mextent.get_bptr().c_str()); + mextent.on_data_commit(); mextent.reapply_delta(); } } void ExtentCommitter::_share_prior_data_to_pending_versions() { + ceph_assert(is_lba_backref_node(extent.get_type())); auto &prior = *extent.prior_instance; switch (extent.get_type()) { case extent_types_t::LADDR_LEAF: diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h index 2cb3ebe4321..e53ce32b707 100644 --- a/src/crimson/os/seastore/cached_extent.h +++ b/src/crimson/os/seastore/cached_extent.h @@ -294,11 +294,6 @@ public: void sync_version(); - void share_prior_data_to_pending() { - _share_prior_data_to_mutations(); - _share_prior_data_to_pending_versions(); - } - void commit_and_share_paddr(); private: diff --git a/src/crimson/os/seastore/collection_manager/collection_flat_node.h b/src/crimson/os/seastore/collection_manager/collection_flat_node.h index 9f8cd53d70b..a1c2c0ceb0f 100644 --- a/src/crimson/os/seastore/collection_manager/collection_flat_node.h +++ b/src/crimson/os/seastore/collection_manager/collection_flat_node.h @@ -107,12 +107,6 @@ struct CollectionNode : LogicalChildNode { coll_map_t decoded; delta_buffer_t delta_buffer; - void do_on_state_commit() final { - auto &prior = static_cast(*get_prior_instance()); - prior.delta_buffer = std::move(delta_buffer); - prior.decoded = std::move(decoded); - } - CachedExtentRef duplicate_for_write(Transaction&) final { assert(delta_buffer.empty()); return CachedExtentRef(new CollectionNode(*this)); diff --git a/src/crimson/os/seastore/logical_child_node.h b/src/crimson/os/seastore/logical_child_node.h index 092ee084a2e..e143e323755 100644 --- a/src/crimson/os/seastore/logical_child_node.h +++ b/src/crimson/os/seastore/logical_child_node.h @@ -136,6 +136,9 @@ protected: do_on_replace_prior(t); } virtual void do_on_replace_prior(Transaction &t) {} + void on_data_commit() final { + ceph_abort("impossible"); + } }; using LogicalChildNodeRef = TCachedExtentRef; } // namespace crimson::os::seastore diff --git a/src/crimson/os/seastore/transaction.h b/src/crimson/os/seastore/transaction.h index 723c87eb66d..9a119b3a3fd 100644 --- a/src/crimson/os/seastore/transaction.h +++ b/src/crimson/os/seastore/transaction.h @@ -411,7 +411,7 @@ public: std::pair pre_stable_extent_paddr_mod( read_set_item_t &item) { - LOG_PREFIX(Transaction::sync_extent_state); + LOG_PREFIX(Transaction::pre_stable_extent_paddr_mod); SUBTRACET(seastore_t, "{}", *this, *item.ref); #ifndef NDEBUG auto [existed, it] = lookup_trans_from_read_extent(item.ref); -- 2.47.3