From 865285a53cf3a4a2880c2b7b6a35b5b20a55553d Mon Sep 17 00:00:00 2001 From: Yingxin Cheng Date: Mon, 27 Mar 2023 17:38:17 +0800 Subject: [PATCH] crimson/os/seastore/cache: use CachedExtent::is_mutable() where appropriate CachedExtent::is_mutable() should only be used to check whether need to call duplicate_for_write(extent). Signed-off-by: Yingxin Cheng --- src/crimson/os/seastore/btree/fixed_kv_btree.h | 10 +++++----- src/crimson/os/seastore/btree/fixed_kv_node.h | 2 +- src/crimson/os/seastore/cached_extent.h | 3 ++- .../collection_manager/collection_flat_node.cc | 6 +++--- .../omap_manager/btree/omap_btree_node_impl.cc | 8 ++++---- .../staged-fltree/node_extent_accessor.h | 18 +++++++++--------- .../staged-fltree/node_extent_manager.h | 2 +- src/crimson/os/seastore/transaction_manager.h | 2 +- 8 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/crimson/os/seastore/btree/fixed_kv_btree.h b/src/crimson/os/seastore/btree/fixed_kv_btree.h index 779b2ad06b9c..e2e94f0454c4 100644 --- a/src/crimson/os/seastore/btree/fixed_kv_btree.h +++ b/src/crimson/os/seastore/btree/fixed_kv_btree.h @@ -525,7 +525,7 @@ public: return handle_split( c, ret ).si_then([c, laddr, val, &ret] { - if (!ret.leaf.node->is_pending()) { + if (!ret.leaf.node->is_mutable()) { CachedExtentRef mut = c.cache.duplicate_for_write( c.trans, ret.leaf.node ); @@ -581,7 +581,7 @@ public: "update element at {}", c.trans, iter.is_end() ? min_max_t::max : iter.get_key()); - if (!iter.leaf.node->is_pending()) { + if (!iter.leaf.node->is_mutable()) { CachedExtentRef mut = c.cache.duplicate_for_write( c.trans, iter.leaf.node ); @@ -622,7 +622,7 @@ public: return seastar::do_with( iter, [this, c](auto &ret) { - if (!ret.leaf.node->is_pending()) { + if (!ret.leaf.node->is_mutable()) { CachedExtentRef mut = c.cache.duplicate_for_write( c.trans, ret.leaf.node ); @@ -1460,7 +1460,7 @@ private: for (; split_from > 0; --split_from) { auto &parent_pos = iter.get_internal(split_from + 1); - if (!parent_pos.node->is_pending()) { + if (!parent_pos.node->is_mutable()) { parent_pos.node = c.cache.duplicate_for_write( c.trans, parent_pos.node )->template cast(); @@ -1632,7 +1632,7 @@ private: node_position_t &pos) { LOG_PREFIX(FixedKVBtree::merge_level); - if (!parent_pos.node->is_pending()) { + if (!parent_pos.node->is_mutable()) { parent_pos.node = c.cache.duplicate_for_write( c.trans, parent_pos.node )->template cast(); diff --git a/src/crimson/os/seastore/btree/fixed_kv_node.h b/src/crimson/os/seastore/btree/fixed_kv_node.h index 1aed9fb200c9..5658d0bf5d40 100644 --- a/src/crimson/os/seastore/btree/fixed_kv_node.h +++ b/src/crimson/os/seastore/btree/fixed_kv_node.h @@ -206,7 +206,7 @@ struct FixedKVInternalNode /** * Internal relative addresses on read or in memory prior to commit * are either record or block relative depending on whether this - * physical node is is_initial_pending() or just is_pending(). + * physical node is is_initial_pending() or just is_mutable(). * * User passes appropriate base depending on lifecycle and * resolve_relative_addrs fixes up relative internal references diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h index e4c3ec50f13f..5ccafc5fa196 100644 --- a/src/crimson/os/seastore/cached_extent.h +++ b/src/crimson/os/seastore/cached_extent.h @@ -280,6 +280,7 @@ public: return TCachedExtentRef(static_cast(this)); } + /// Returns true if extent can be mutated in an open transaction bool is_mutable() const { return state == extent_state_t::INITIAL_WRITE_PENDING || state == extent_state_t::MUTATION_PENDING || @@ -479,7 +480,7 @@ private: /// number of deltas since initial write extent_version_t version = 0; - /// address of original block -- relative iff is_pending() and is_clean() + /// address of original block -- record relative iff is_initial_pending() paddr_t poffset; /// relative address before ool write, used to update mapping diff --git a/src/crimson/os/seastore/collection_manager/collection_flat_node.cc b/src/crimson/os/seastore/collection_manager/collection_flat_node.cc index 6181582702cd..9eaeefc72a67 100644 --- a/src/crimson/os/seastore/collection_manager/collection_flat_node.cc +++ b/src/crimson/os/seastore/collection_manager/collection_flat_node.cc @@ -62,7 +62,7 @@ CollectionNode::create(coll_context_t cc, coll_t coll, unsigned bits) { read_to_local(); logger().debug("CollectionNode:{}", __func__); - if (!is_pending()) { + if (!is_mutable()) { auto mut = cc.tm.get_mutable_extent(cc.t, this)->cast(); return mut->create(cc, coll, bits); } @@ -90,7 +90,7 @@ CollectionNode::update(coll_context_t cc, coll_t coll, unsigned bits) { read_to_local(); logger().debug("CollectionNode:{}", __func__); - if (!is_pending()) { + if (!is_mutable()) { auto mut = cc.tm.get_mutable_extent(cc.t, this)->cast(); return mut->update(cc, coll, bits); } @@ -107,7 +107,7 @@ CollectionNode::remove(coll_context_t cc, coll_t coll) { read_to_local(); logger().debug("CollectionNode:{}", __func__); - if (!is_pending()) { + if (!is_mutable()) { auto mut = cc.tm.get_mutable_extent(cc.t, this)->cast(); return mut->remove(cc, coll); } diff --git a/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.cc b/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.cc index 7f0af20f6d1b..ee22b00b7a2c 100644 --- a/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.cc +++ b/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.cc @@ -87,7 +87,7 @@ OMapInnerNode::handle_split( { LOG_PREFIX(OMapInnerNode::handle_split); DEBUGT("this: {}", oc.t, *this); - if (!is_pending()) { + if (!is_mutable()) { auto mut = oc.tm.get_mutable_extent(oc.t, this)->cast(); auto mut_iter = mut->iter_idx(iter.get_index()); return mut->handle_split(oc, mut_iter, mresult); @@ -379,7 +379,7 @@ OMapInnerNode::merge_entry( { LOG_PREFIX(OMapInnerNode::merge_entry); DEBUGT("{}, parent: {}", oc.t, *entry, *this); - if (!is_pending()) { + if (!is_mutable()) { auto mut = oc.tm.get_mutable_extent(oc.t, this)->cast(); auto mut_iter = mut->iter_idx(iter->get_index()); return mut->merge_entry(oc, mut_iter, entry); @@ -520,7 +520,7 @@ OMapLeafNode::insert( DEBUGT("{} -> {}, this: {}", oc.t, key, value, *this); bool overflow = extent_will_overflow(key.size(), value.length()); if (!overflow) { - if (!is_pending()) { + if (!is_mutable()) { auto mut = oc.tm.get_mutable_extent(oc.t, this)->cast(); return mut->insert(oc, key, value); } @@ -579,7 +579,7 @@ OMapLeafNode::rm_key(omap_context_t oc, const std::string &key) LOG_PREFIX(OMapLeafNode::rm_key); DEBUGT("{}, this: {}", oc.t, key, *this); auto rm_pt = find_string_key(key); - if (!is_pending() && rm_pt != iter_end()) { + if (!is_mutable() && rm_pt != iter_end()) { auto mut = oc.tm.get_mutable_extent(oc.t, this)->cast(); return mut->rm_key(oc, key); } diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_accessor.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_accessor.h index c800acc69346..1a03036d3c3e 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_accessor.h +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_accessor.h @@ -305,7 +305,7 @@ class NodeExtentAccessorT { assert(p_recorder->node_type() == NODE_TYPE); assert(p_recorder->field_type() == FIELD_TYPE); recorder = static_cast(p_recorder); - } else if (!extent->is_pending() && extent->is_valid()) { + } else if (!extent->is_mutable() && extent->is_valid()) { state = nextent_state_t::READ_ONLY; // mut is empty assert(extent->get_recorder() == nullptr || @@ -355,7 +355,7 @@ class NodeExtentAccessorT { void prepare_mutate(context_t c) { assert(!is_retired()); if (state == nextent_state_t::READ_ONLY) { - assert(!extent->is_pending()); + assert(!extent->is_mutable()); auto ref_recorder = recorder_t::create_for_encode(c.vb); recorder = static_cast(ref_recorder.get()); extent = extent->mutate(c, std::move(ref_recorder)); @@ -366,7 +366,7 @@ class NodeExtentAccessorT { assert(recorder == static_cast(extent->get_recorder())); mut.emplace(extent->get_mutable()); } - assert(extent->is_pending()); + assert(extent->is_mutable()); } template @@ -376,7 +376,7 @@ class NodeExtentAccessorT { position_t& insert_pos, match_stage_t& insert_stage, node_offset_t& insert_size) { - assert(extent->is_pending()); + assert(extent->is_mutable()); assert(state != nextent_state_t::READ_ONLY); if (state == nextent_state_t::MUTATION_PENDING) { recorder->template encode_insert( @@ -397,7 +397,7 @@ class NodeExtentAccessorT { } void split_replayable(StagedIterator& split_at) { - assert(extent->is_pending()); + assert(extent->is_mutable()); assert(state != nextent_state_t::READ_ONLY); if (state == nextent_state_t::MUTATION_PENDING) { recorder->encode_split(split_at, read().p_start()); @@ -420,7 +420,7 @@ class NodeExtentAccessorT { position_t& insert_pos, match_stage_t& insert_stage, node_offset_t& insert_size) { - assert(extent->is_pending()); + assert(extent->is_mutable()); assert(state != nextent_state_t::READ_ONLY); if (state == nextent_state_t::MUTATION_PENDING) { recorder->template encode_split_insert( @@ -444,7 +444,7 @@ class NodeExtentAccessorT { void update_child_addr_replayable( const laddr_t new_addr, laddr_packed_t* p_addr) { - assert(extent->is_pending()); + assert(extent->is_mutable()); assert(state != nextent_state_t::READ_ONLY); if (state == nextent_state_t::MUTATION_PENDING) { recorder->encode_update_child_addr( @@ -462,7 +462,7 @@ class NodeExtentAccessorT { } std::tuple erase_replayable(const position_t& pos) { - assert(extent->is_pending()); + assert(extent->is_mutable()); assert(state != nextent_state_t::READ_ONLY); if (state == nextent_state_t::MUTATION_PENDING) { recorder->encode_erase(pos); @@ -479,7 +479,7 @@ class NodeExtentAccessorT { } position_t make_tail_replayable() { - assert(extent->is_pending()); + assert(extent->is_mutable()); assert(state != nextent_state_t::READ_ONLY); if (state == nextent_state_t::MUTATION_PENDING) { recorder->encode_make_tail(); diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager.h index 5593e2311a07..f8772929c6ad 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager.h +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager.h @@ -32,7 +32,7 @@ class NodeExtent : public LogicalCachedExtent { return get_bptr().c_str(); } NodeExtentMutable get_mutable() { - assert(is_pending()); + assert(is_mutable()); return do_get_mutable(); } diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index fddd1e2eecec..47dcfc712944 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -261,7 +261,7 @@ public: "extent is already duplicated -- {}", t, *ref); - assert(ref->is_pending()); + assert(ref->is_mutable()); assert(&*ref == &*ret); } return ret; -- 2.47.3