From: Xuehan Xu Date: Thu, 21 May 2026 13:52:46 +0000 (+0800) Subject: crimson/os/seastore/cached_extent: let pending cached extents hold pointers to the... X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=a238cd65537c6a89178f1a04404fcbf8d3b5dd55;p=ceph-ci.git crimson/os/seastore/cached_extent: let pending cached extents hold pointers to the transactions This is necessary for the promote transaction to find the transactions that are retiring the extents promoted Signed-off-by: Xuehan Xu --- diff --git a/src/crimson/os/seastore/btree/fixed_kv_node.h b/src/crimson/os/seastore/btree/fixed_kv_node.h index 69c40c1acad..30a46749a7e 100644 --- a/src/crimson/os/seastore/btree/fixed_kv_node.h +++ b/src/crimson/os/seastore/btree/fixed_kv_node.h @@ -85,7 +85,7 @@ struct FixedKVNode : CachedExtent { void on_delta_write(paddr_t record_block_offset) final { // All in-memory relative addrs are necessarily record-relative assert(get_prior_instance()); - assert(pending_for_transaction); + assert(t != nullptr); resolve_relative_addrs(record_block_offset); } @@ -264,8 +264,9 @@ struct FixedKVInternalNode paddr_t addr, base_child_t* nextent) { LOG_PREFIX(FixedKVInternalNode::update); + assert(this->t != nullptr); SUBTRACE(seastore_fixedkv_tree, "trans.{}, pos {}, {}", - this->pending_for_transaction, + this->t->get_trans_id(), iter.get_offset(), (void*)nextent); this->update_child_ptr(iter.get_offset(), nextent); @@ -281,8 +282,9 @@ struct FixedKVInternalNode paddr_t addr, base_child_t* nextent) { LOG_PREFIX(FixedKVInternalNode::insert); + assert(this->t != nullptr); SUBTRACE(seastore_fixedkv_tree, "trans.{}, pos {}, key {}, {}", - this->pending_for_transaction, + this->t->get_trans_id(), iter.get_offset(), pivot, (void*)nextent); @@ -296,8 +298,9 @@ struct FixedKVInternalNode void remove(internal_const_iterator_t iter) { LOG_PREFIX(FixedKVInternalNode::remove); + assert(this->t != nullptr); SUBTRACE(seastore_fixedkv_tree, "trans.{}, pos {}, key {}", - this->pending_for_transaction, + this->t->get_trans_id(), iter.get_offset(), iter.get_key()); this->remove_child_ptr(iter.get_offset()); @@ -312,8 +315,9 @@ struct FixedKVInternalNode paddr_t addr, base_child_t* nextent) { LOG_PREFIX(FixedKVInternalNode::replace); + assert(this->t != nullptr); SUBTRACE(seastore_fixedkv_tree, "trans.{}, pos {}, old key {}, key {}, {}", - this->pending_for_transaction, + this->t->get_trans_id(), iter.get_offset(), iter.get_key(), pivot, diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index 033fc4b4a02..2db2a7057d3 100644 --- a/src/crimson/os/seastore/cache.cc +++ b/src/crimson/os/seastore/cache.cc @@ -1374,7 +1374,7 @@ CachedExtentRef Cache::duplicate_for_write( } auto ret = i->duplicate_for_write(t); - ret->pending_for_transaction = t.get_trans_id(); + ret->t = &t; ret->set_prior_instance(i); if (!is_root_type(ret->get_type())) { assert(ret->get_paddr().is_absolute()); @@ -1780,7 +1780,7 @@ record_t Cache::prepare_record( } assert(i->state == CachedExtent::extent_state_t::DIRTY); assert(i->version > 0); - assert(i->pending_for_transaction == TRANS_ID_NULL); + assert(i->t == nullptr); assert(!i->prior_instance); remove_from_dirty(i, &trans_src); // set the version to zero because the extent state is now clean @@ -1813,7 +1813,7 @@ record_t Cache::prepare_record( assert(!i->prior_instance || t.get_src() == transaction_type_t::DEMOTE); // no set_io_wait(), skip complete_commit() assert(!i->is_pending_io()); - i->pending_for_transaction = TRANS_ID_NULL; + i->t = nullptr; i->state = CachedExtent::extent_state_t::CLEAN; } else { assert(i->is_exist_mutation_pending()); @@ -2094,7 +2094,7 @@ void Cache::complete_commit( assert(i->get_last_committed_crc() == CRC_NULL); } #endif - i->pending_for_transaction = TRANS_ID_NULL; + i->t = nullptr; i->on_initial_write(); const auto t_src = t.get_src(); if (should_use_no_conflict_publish(t, i->get_type())) { @@ -2105,7 +2105,7 @@ void Cache::complete_commit( TRACET("committing rewritten extent into " "existing, inline={} -- {}, prior={}", t, is_inline, *i, prior); - prior.pending_for_transaction = TRANS_ID_NULL; + prior.t = nullptr; committer.commit_state(); committer.sync_checksum(); committer.commit_and_share_paddr(); @@ -2203,7 +2203,7 @@ void Cache::complete_commit( committer.sync_checksum(); committer.unblock_trans(t); auto &prior = *i->prior_instance; - prior.pending_for_transaction = TRANS_ID_NULL; + prior.t = nullptr; ceph_assert(prior.is_valid()); if (is_lba_backref_node(i->get_type())) { committer.commit_data(); @@ -2215,7 +2215,7 @@ void Cache::complete_commit( prior.committer.reset(); } - i->pending_for_transaction = TRANS_ID_NULL; + i->t = nullptr; i->reset_prior_instance(); assert(i->version > 0); i->complete_io(); @@ -2238,7 +2238,7 @@ void Cache::complete_commit( TRACET("committing rewritten extent into " "existing -- {}, prior={}", t, *i, prior); - prior.pending_for_transaction = TRANS_ID_NULL; + prior.t = nullptr; if (auto shadow = prior.get_shadow(); shadow) { committer.commit_shadow_demote(t); prior.reset_shadow(); @@ -2295,7 +2295,7 @@ void Cache::init() P_ADDR_ROOT, PLACEMENT_HINT_NULL, NULL_GENERATION, - TRANS_ID_NULL, + nullptr, write_policy_t::WRITE_BACK); root->set_modify_time(seastar::lowres_system_clock::now()); INFO("init root -- {}", *root); @@ -2720,7 +2720,7 @@ Cache::_get_absent_extent_by_type( offset, PLACEMENT_HINT_NULL, NULL_GENERATION, - TRANS_ID_NULL, + nullptr, write_policy_t::WRITE_BACK); DEBUGT("{} length=0x{:x} is absent, add extent ... -- {}", t, type, length, *ret); diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index 119f71bfc2e..01b1e6742b7 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -866,7 +866,7 @@ private: offset, PLACEMENT_HINT_NULL, NULL_GENERATION, - TRANS_ID_NULL, + nullptr, write_policy_t::WRITE_BACK); SUBDEBUGT(seastore_cache, "{} {}~0x{:x} is absent, add extent and reading range 0x{:x}~0x{:x} ... -- {}", @@ -906,7 +906,7 @@ private: offset, PLACEMENT_HINT_NULL, NULL_GENERATION, - TRANS_ID_NULL, + nullptr, write_policy_t::WRITE_BACK); SUBDEBUG(seastore_cache, "{} {}~0x{:x} is absent, add extent and reading range 0x{:x}~0x{:x} ... -- {}", @@ -1208,7 +1208,7 @@ public: result->paddr, opt.hint, result->gen, - t.get_trans_id(), + &t, write_policy_t::WRITE_BACK); t.add_fresh_extent(ret); SUBDEBUGT(seastore_cache, @@ -1252,7 +1252,7 @@ public: result.paddr, opt.hint, result.gen, - t.get_trans_id(), + &t, opt.write_policy); t.add_fresh_extent(ret); SUBDEBUGT(seastore_cache, @@ -1295,7 +1295,7 @@ public: remap_paddr, PLACEMENT_HINT_NULL, NULL_GENERATION, - t.get_trans_id(), + &t, write_policy_t::WRITE_BACK); auto extent = ext->template cast(); diff --git a/src/crimson/os/seastore/cached_extent.cc b/src/crimson/os/seastore/cached_extent.cc index 622a331f2e0..6cef967c0f7 100644 --- a/src/crimson/os/seastore/cached_extent.cc +++ b/src/crimson/os/seastore/cached_extent.cc @@ -89,6 +89,34 @@ std::ostream &operator<<(std::ostream &out, const CachedExtent &ext) return ext.print(out); } +trans_spec_view_t::trans_spec_view_t( + Transaction &t) : t(&t) {} + +bool trans_spec_view_t::cmp_t::operator()( + const trans_spec_view_t &lhs, + const trans_spec_view_t &rhs) const +{ + transaction_id_t l = ((lhs.t == nullptr) ? 0 : lhs.t->get_trans_id()); + transaction_id_t r = ((rhs.t == nullptr) ? 0 : rhs.t->get_trans_id()); + return l < r; +} + +bool trans_spec_view_t::cmp_t::operator()( + const transaction_id_t &lhs, + const trans_spec_view_t &rhs) const +{ + transaction_id_t r = ((rhs.t == nullptr) ? 0 : rhs.t->get_trans_id()); + return lhs < r; +} + +bool trans_spec_view_t::cmp_t::operator()( + const trans_spec_view_t &lhs, + const transaction_id_t &rhs) const +{ + transaction_id_t l = ((lhs.t == nullptr) ? 0 : lhs.t->get_trans_id()); + return l < rhs; +} + CachedExtent::~CachedExtent() { if (parent_index) { @@ -124,6 +152,44 @@ CachedExtent* CachedExtent::maybe_get_transactional_view(Transaction &t) { return this; } +bool CachedExtent::is_pending_in_trans(transaction_id_t id) const { + auto trans_id = ((t == nullptr) ? 0 : t->get_trans_id()); + return is_pending() && trans_id == id; +} + +std::ostream &CachedExtent::print(std::ostream &out) const { + std::string prior_poffset_str = prior_poffset + ? fmt::format("{}", *prior_poffset) + : "nullopt"; + out << "CachedExtent(addr=" << this + << ", type=" << get_type() + << ", trans=" << ((t == nullptr) ? 0 : t->get_trans_id()) + << ", version=" << version + << ", dirty_from=" << dirty_from + << ", modify_time=" << sea_time_point_printer_t{modify_time} + << ", paddr=" << get_paddr() + << ", prior_paddr=" << prior_poffset_str + << std::hex << ", length=0x" << get_length() + << ", loaded=0x" << get_loaded_length() << std::dec + << ", state=" << state + << ", pin_state=" << pin_state + << ", last_committed_crc=" << last_committed_crc + << ", refcount=" << use_count() + << ", user_hint=" << user_hint + << ", write_policy=" << write_policy + << ", rewrite_gen=" << rewrite_gen_printer_t{rewrite_generation} + << ", pending_io="; + if (is_pending_io()) { + out << io_wait->from_state; + } else { + out << "N/A"; + } + if (is_valid() && is_fully_loaded() && !is_stable_clean_pending()) { + print_detail(out); + } + return out << ")"; +} + std::ostream &LogicalCachedExtent::print_detail(std::ostream &out) const { out << ", laddr=" << laddr @@ -410,7 +476,7 @@ void ExtentCommitter::commit_state() { SUBTRACET(seastore_cache, "{} prior={}", t, extent, *extent.prior_instance); auto &prior = *extent.prior_instance; - prior.pending_for_transaction = extent.pending_for_transaction; + prior.t = extent.t; prior.modify_time = extent.modify_time; prior.last_committed_crc = extent.last_committed_crc; prior.dirty_from = extent.dirty_from; diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h index 7115494d167..2b3084a52a7 100644 --- a/src/crimson/os/seastore/cached_extent.h +++ b/src/crimson/os/seastore/cached_extent.h @@ -141,32 +141,22 @@ template using read_trans_set_t = typename read_set_item_t::trans_set_t; struct trans_spec_view_t { - // if the extent is pending, contains the id of the owning transaction; - // TRANS_ID_NULL otherwise - transaction_id_t pending_for_transaction = TRANS_ID_NULL; + // if the extent is pending, this points to the transaction + Transaction *t = nullptr; trans_spec_view_t() = default; - trans_spec_view_t(transaction_id_t id) : pending_for_transaction(id) {} + trans_spec_view_t(Transaction &t); virtual ~trans_spec_view_t() = default; struct cmp_t { bool operator()( const trans_spec_view_t &lhs, - const trans_spec_view_t &rhs) const - { - return lhs.pending_for_transaction < rhs.pending_for_transaction; - } + const trans_spec_view_t &rhs) const; bool operator()( const transaction_id_t &lhs, - const trans_spec_view_t &rhs) const - { - return lhs < rhs.pending_for_transaction; - } + const trans_spec_view_t &rhs) const; bool operator()( const trans_spec_view_t &lhs, - const transaction_id_t &rhs) const - { - return lhs.pending_for_transaction < rhs; - } + const transaction_id_t &rhs) const; }; using trans_view_hook_t = @@ -364,13 +354,13 @@ public: paddr_t paddr, placement_hint_t hint, rewrite_gen_t gen, - transaction_id_t trans_id, + Transaction *trans, write_policy_t policy) { state = _state; set_paddr(paddr); user_hint = hint; rewrite_generation = gen; - pending_for_transaction = trans_id; + t = trans; write_policy = policy; } @@ -524,38 +514,7 @@ public: friend std::ostream &operator<<(std::ostream &, extent_state_t); virtual std::ostream &print_detail(std::ostream &out) const { return out; } - std::ostream &print(std::ostream &out) const { - std::string prior_poffset_str = prior_poffset - ? fmt::format("{}", *prior_poffset) - : "nullopt"; - out << "CachedExtent(addr=" << this - << ", type=" << get_type() - << ", trans=" << pending_for_transaction - << ", version=" << version - << ", dirty_from=" << dirty_from - << ", modify_time=" << sea_time_point_printer_t{modify_time} - << ", paddr=" << get_paddr() - << ", prior_paddr=" << prior_poffset_str - << std::hex << ", length=0x" << get_length() - << ", loaded=0x" << get_loaded_length() << std::dec - << ", state=" << state - << ", pin_state=" << pin_state - << ", last_committed_crc=" << last_committed_crc - << ", refcount=" << use_count() - << ", user_hint=" << user_hint - << ", write_policy=" << write_policy - << ", rewrite_gen=" << rewrite_gen_printer_t{rewrite_generation} - << ", pending_io="; - if (is_pending_io()) { - out << io_wait->from_state; - } else { - out << "N/A"; - } - if (is_valid() && is_fully_loaded() && !is_stable_clean_pending()) { - print_detail(out); - } - return out << ")"; - } + std::ostream &print(std::ostream &out) const; /** * get_delta @@ -854,9 +813,7 @@ public: } /// Returns true if the extent part of the open transaction - bool is_pending_in_trans(transaction_id_t id) const { - return is_pending() && pending_for_transaction == id; - } + bool is_pending_in_trans(transaction_id_t id) const; enum class viewable_state_t { stable, // viewable @@ -1331,8 +1288,8 @@ struct trans_retired_extent_link_t { // Otherwise, we have to search through each extent's "retired_transactions" // to remove the transaction trans_spec_view_t trans_view; - trans_retired_extent_link_t(CachedExtentRef extent, transaction_id_t id) - : extent(extent), trans_view{id} + trans_retired_extent_link_t(CachedExtentRef extent, Transaction &t) + : extent(extent), trans_view{t} { assert(extent->is_stable()); extent->retired_transactions.insert(trans_view); diff --git a/src/crimson/os/seastore/lba/lba_btree_node.cc b/src/crimson/os/seastore/lba/lba_btree_node.cc index 1c97d078a11..c94f262c364 100644 --- a/src/crimson/os/seastore/lba/lba_btree_node.cc +++ b/src/crimson/os/seastore/lba/lba_btree_node.cc @@ -50,8 +50,9 @@ void LBALeafNode::update( modification_t mod) { LOG_PREFIX(LBALeafNode::update); + assert(this->t != nullptr); SUBTRACE(seastore_fixedkv_tree, "trans.{}, pos {}", - this->pending_for_transaction, + this->t->get_trans_id(), iter.get_offset()); if (likely(mod == modification_t::USER_MODIFY)) { this->on_modify(); @@ -71,8 +72,9 @@ LBALeafNode::internal_const_iterator_t LBALeafNode::insert( lba_map_val_t val) { LOG_PREFIX(LBALeafNode::insert); + assert(this->t != nullptr); SUBTRACE(seastore_fixedkv_tree, "trans.{}, pos {}, key {}", - this->pending_for_transaction, + this->t->get_trans_id(), iter.get_offset(), addr); this->on_modify(); diff --git a/src/crimson/os/seastore/lba/lba_btree_node.h b/src/crimson/os/seastore/lba/lba_btree_node.h index 49df7813c01..0baa8a28830 100644 --- a/src/crimson/os/seastore/lba/lba_btree_node.h +++ b/src/crimson/os/seastore/lba/lba_btree_node.h @@ -165,8 +165,9 @@ struct LBALeafNode laddr_t pivot, lba_map_val_t val) { LOG_PREFIX(FixedKVInternalNode::replace); + assert(this->t != nullptr); SUBTRACE(seastore_fixedkv_tree, "trans.{}, pos {}, old key {}, key {}", - this->pending_for_transaction, + this->t->get_trans_id(), iter.get_offset(), iter.get_key(), pivot); @@ -180,8 +181,9 @@ struct LBALeafNode void remove(internal_const_iterator_t iter) final { LOG_PREFIX(LBALeafNode::remove); + assert(this->t != nullptr); SUBTRACE(seastore_fixedkv_tree, "trans.{}, pos {}, key {}", - this->pending_for_transaction, + this->t->get_trans_id(), iter.get_offset(), iter.get_key()); assert(iter != this->end()); diff --git a/src/crimson/os/seastore/linked_tree_node.h b/src/crimson/os/seastore/linked_tree_node.h index 1b136c8a6a2..f6dc6f828db 100644 --- a/src/crimson/os/seastore/linked_tree_node.h +++ b/src/crimson/os/seastore/linked_tree_node.h @@ -117,7 +117,7 @@ protected: auto pi = me.get_prior_instance(); auto &prior = *pi->template cast(); ceph_assert(prior.parent_of_root); - ceph_assert(me.pending_for_transaction); + ceph_assert(me.t != nullptr); parent_of_root = prior.parent_of_root; TreeRootLinker::link_root(parent_of_root, &me); return; @@ -441,7 +441,7 @@ public: // see Transaction::views struct copy_dests_t : trans_spec_view_t { std::set, Comparator> dests_by_key; - copy_dests_t(Transaction &t) : trans_spec_view_t{t.get_trans_id()} {} + copy_dests_t(Transaction &t) : trans_spec_view_t(t) {} ~copy_dests_t() { LOG_PREFIX(~copy_dests_t); SUBTRACE(seastore_fixedkv_tree, "copy_dests_t destroyed"); @@ -480,8 +480,9 @@ protected: auto mut_iter = me.mutation_pending_extents.find( t.get_trans_id(), trans_spec_view_t::cmp_t()); assert(mut_iter != me.mutation_pending_extents.end()); - assert(copy_dests_by_trans.find(t.get_trans_id()) == - copy_dests_by_trans.end()); + assert(copy_dests_by_trans.find( + t.get_trans_id(), trans_spec_view_t::cmp_t()) == + copy_dests_by_trans.end()); return static_cast(&(*mut_iter)); } ceph_assert(hint == CachedExtent::viewable_state_t::stable_become_retired); @@ -501,7 +502,8 @@ protected: template void for_each_copy_dest_set(Transaction &t, Func &&f) { for (auto &dests : copy_dests_by_trans) { - if (dests.pending_for_transaction == t.get_trans_id()) { + assert(dests.t != nullptr); + if (dests.t->get_trans_id() == t.get_trans_id()) { continue; } auto ©_dests = static_cast(dests); @@ -515,8 +517,11 @@ protected: auto tid = t.get_trans_id(); auto iter = copy_dests_by_trans.lower_bound( tid, trans_spec_view_t::cmp_t()); + if (iter != copy_dests_by_trans.end()) { + assert(iter->t != nullptr); + } if (iter == copy_dests_by_trans.end() || - iter->pending_for_transaction != tid) { + iter->t->get_trans_id() != tid) { iter = copy_dests_by_trans.insert_before( iter, t.add_transactional_view(t)); } @@ -548,8 +553,9 @@ protected: auto &me = down_cast(); LOG_PREFIX(ParentNode::remove_child_ptr); auto raw_children = children.data(); + assert(me.t != nullptr); SUBTRACE(seastore_fixedkv_tree, "trans.{}, pos {}, total size {}, extent {}", - me.pending_for_transaction, + me.t->get_trans_id(), offset, me.get_size(), (void*)raw_children[offset]); diff --git a/src/crimson/os/seastore/transaction.h b/src/crimson/os/seastore/transaction.h index ed1af43bd94..432e57737bd 100644 --- a/src/crimson/os/seastore/transaction.h +++ b/src/crimson/os/seastore/transaction.h @@ -162,7 +162,7 @@ public: ref->set_invalid(*this); write_set.erase(*ref); assert(ref->prior_instance); - retired_set.emplace(ref->prior_instance, trans_id); + retired_set.emplace(ref->prior_instance, *this); assert(read_set.count(ref->prior_instance->get_paddr(), extent_cmp_t{})); ref->reset_prior_instance(); } else { @@ -171,7 +171,7 @@ public: // XXX: prevent double retire -- retired_set.count(ref->get_paddr()) == 0 // If it's already in the set, insert here will be a noop, // which is what we want. - retired_set.emplace(ref, trans_id); + retired_set.emplace(ref, *this); } } @@ -408,7 +408,7 @@ public: bool retired) { read_set.insert(item); if (retired) { - retired_set.emplace(item.ref, trans_id); + retired_set.emplace(item.ref, *this); } } void maybe_update_pending_paddr(