From: Yingxin Cheng Date: Fri, 25 Sep 2020 07:57:44 +0000 (+0800) Subject: crimson/onode-staged-tree: fix insert_stage calculation X-Git-Tag: v16.1.0~359^2~28 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=a3827220c183fedbe57627563a6789b6fcc8e4b3;p=ceph.git crimson/onode-staged-tree: fix insert_stage calculation Fix incorrect insert_stage calculation when insert into the front of leaf node. Signed-off-by: Yingxin Cheng --- diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc b/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc index 28db24a28f49..36900606443a 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc @@ -125,13 +125,13 @@ node_future, bool>> Node::insert( MatchHistory(), [this, c, &key, &value](auto& history) { return lower_bound_tracked(c, key, history ).safe_then([c, &key, &value, &history](auto result) { - if (result.match == MatchKindBS::EQ) { + if (result.match() == MatchKindBS::EQ) { return node_ertr::make_ready_future, bool>>( std::make_pair(result.p_cursor, false)); } else { auto leaf_node = result.p_cursor->get_leaf_node(); return leaf_node->insert_value( - c, key, value, result.p_cursor->get_position(), history + c, key, value, result.p_cursor->get_position(), history, result.mstat ).safe_then([](auto p_cursor) { return node_ertr::make_ready_future, bool>>( std::make_pair(p_cursor, true)); @@ -564,7 +564,7 @@ LeafNode::lower_bound_tracked( cursor = get_or_track_cursor(result.position, index_key, result.p_value); } return node_ertr::make_ready_future( - search_result_t{cursor, result.match()}); + search_result_t{cursor, result.mstat}); } node_future<> LeafNode::test_clone_root( @@ -586,7 +586,8 @@ node_future<> LeafNode::test_clone_root( node_future> LeafNode::insert_value( context_t c, const key_hobj_t& key, const onode_t& value, - const search_position_t& pos, const MatchHistory& history) { + const search_position_t& pos, const MatchHistory& history, + match_stat_t mstat) { #ifndef NDEBUG if (pos.is_end()) { assert(impl->is_level_tail()); @@ -594,6 +595,7 @@ node_future> LeafNode::insert_value( #endif std::cout << "leaf insert at pos(" << pos << "), " << key << ", " << value << ", " << history + << ", mstat(" << (int)mstat << ")" << std::endl; #if 0 std::cout << "before insert:" << std::endl; @@ -602,7 +604,7 @@ node_future> LeafNode::insert_value( search_position_t insert_pos = pos; auto [insert_stage, insert_size] = impl->evaluate_insert( - key, value, history, insert_pos); + key, value, history, mstat, insert_pos); auto free_size = impl->free_size(); if (free_size >= insert_size) { // insert diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node.h index 97f7cfad02d2..328fe454bc5f 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/node.h +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node.h @@ -93,7 +93,12 @@ class Node struct search_result_t { bool is_end() const { return p_cursor->is_end(); } Ref p_cursor; - MatchKindBS match; + match_stat_t mstat; + + MatchKindBS match() const { + assert(mstat >= MSTAT_MIN && mstat <= MSTAT_MAX); + return (mstat == MSTAT_EQ ? MatchKindBS::EQ : MatchKindBS::NE); + } }; virtual ~Node(); @@ -283,7 +288,8 @@ class LeafNode final : public Node { LeafNode(LeafNodeImpl*, NodeImplURef&&); node_future> insert_value( context_t, const key_hobj_t&, const onode_t&, - const search_position_t&, const MatchHistory&); + const search_position_t&, const MatchHistory&, + match_stat_t mstat); static node_future> allocate_root(context_t, RootNodeTracker&); friend class Node; diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_impl.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node_impl.h index a2027dd74c98..d752bae70a01 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_impl.h +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_impl.h @@ -125,7 +125,7 @@ class LeafNodeImpl : public NodeImpl { search_position_t&, key_view_t&, const onode_t**) const = 0; virtual std::tuple evaluate_insert( const key_hobj_t&, const onode_t&, - const MatchHistory&, search_position_t&) const = 0; + const MatchHistory&, match_stat_t, search_position_t&) const = 0; struct fresh_impl_t { LeafNodeImplURef impl; diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_layout.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node_layout.h index dc98943b2e6a..eae101db1e26 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_layout.h +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_layout.h @@ -411,11 +411,12 @@ class NodeLayoutT final : public InternalNodeImpl, public LeafNodeImpl { } std::tuple evaluate_insert( - const key_hobj_t& key, const onode_t& value, const MatchHistory& history, + const key_hobj_t& key, const onode_t& value, + const MatchHistory& history, match_stat_t mstat, search_position_t& insert_pos) const override { if constexpr (NODE_TYPE == node_type_t::LEAF) { return STAGE_T::evaluate_insert( - key, value, history, cast_down(insert_pos)); + key, value, history, mstat, cast_down(insert_pos)); } else { assert(false && "impossible path"); } diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_types.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node_types.h index c1b4d190da58..512dcaece94b 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_types.h +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_types.h @@ -47,4 +47,15 @@ struct laddr_packed_t { laddr_t value; } __attribute__((packed)); +using match_stat_t = int8_t; +constexpr match_stat_t MSTAT_END = -2; // index is search_position_t::end() +constexpr match_stat_t MSTAT_EQ = -1; // key == index +constexpr match_stat_t MSTAT_NE0 = 0; // key == index [pool/shard crush ns/oid]; key < index [snap/gen] +constexpr match_stat_t MSTAT_NE1 = 1; // key == index [pool/shard crush]; key < index [ns/oid] +constexpr match_stat_t MSTAT_NE2 = 2; // key < index [pool/shard crush ns/oid] || + // key == index [pool/shard]; key < index [crush] +constexpr match_stat_t MSTAT_NE3 = 3; // key < index [pool/shard] +constexpr match_stat_t MSTAT_MIN = MSTAT_END; +constexpr match_stat_t MSTAT_MAX = MSTAT_NE3; + } diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/stages/stage.h b/src/crimson/os/seastore/onode_manager/staged-fltree/stages/stage.h index 0a6ba4c25b36..3c4482613405 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/stages/stage.h +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/stages/stage.h @@ -1091,7 +1091,7 @@ struct staged { template > static std::enable_if_t evaluate_insert( const full_key_t& key, const onode_t& value, - const MatchHistory& history, position_t& position) { + const MatchHistory& history, match_stat_t mstat, position_t& position) { match_stage_t insert_stage = STAGE_TOP; while (*history.get_by_stage(insert_stage) == MatchKindCMP::EQ) { assert(insert_stage != STAGE_BOTTOM && "insert conflict!"); @@ -1104,14 +1104,18 @@ struct staged { assert(insert_stage <= STAGE && "impossible insert stage"); } else if (position == position_t::begin()) { // I must be short-circuited by staged::smallest_result() - // in staged::lower_bound() - + // in staged::lower_bound(), so we need to rely on mstat instead + assert(mstat >= MSTAT_NE0 && mstat <= MSTAT_NE3); + if (mstat == MSTAT_NE0) { + insert_stage = STAGE_RIGHT; + } else if (mstat == MSTAT_NE1) { + insert_stage = STAGE_STRING; + } else { + insert_stage = STAGE_LEFT; + } // XXX(multi-type): need to upgrade node type before inserting an // incompatible index at front. assert(insert_stage <= STAGE && "incompatible insert"); - - // insert at begin and at the top stage - insert_stage = STAGE; } else { assert(insert_stage <= STAGE && "impossible insert stage"); bool ret = compensate_insert_position_at(insert_stage, position); diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/stages/stage_types.h b/src/crimson/os/seastore/onode_manager/staged-fltree/stages/stage_types.h index 4aef1ede1f5a..55144e10f0c8 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/stages/stage_types.h +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/stages/stage_types.h @@ -315,25 +315,10 @@ template<> struct value_type { using type = onode_t; }; template using value_type_t = typename value_type::type; -using match_stat_t = int8_t; -constexpr match_stat_t MSTAT_END = -2; // index is search_position_t::end() -constexpr match_stat_t MSTAT_EQ = -1; // key == index -constexpr match_stat_t MSTAT_NE0 = 0; // key == index [pool/shard crush ns/oid]; key < index [snap/gen] -constexpr match_stat_t MSTAT_NE1 = 1; // key == index [pool/shard crush]; key < index [ns/oid] -constexpr match_stat_t MSTAT_NE2 = 2; // key < index [pool/shard crush ns/oid] || - // key == index [pool/shard]; key < index [crush] -constexpr match_stat_t MSTAT_NE3 = 3; // key < index [pool/shard] -constexpr match_stat_t MSTAT_MIN = MSTAT_END; -constexpr match_stat_t MSTAT_MAX = MSTAT_NE3; - template struct staged_result_t { using me_t = staged_result_t; bool is_end() const { return position.is_end(); } - MatchKindBS match() const { - assert(mstat >= MSTAT_MIN && mstat <= MSTAT_MAX); - return (mstat == MSTAT_EQ ? MatchKindBS::EQ : MatchKindBS::NE); - } static me_t end() { return {staged_position_t::end(), nullptr, MSTAT_END}; diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/tree.cc b/src/crimson/os/seastore/onode_manager/staged-fltree/tree.cc index b5ccc60c23b6..0e20f567ecdb 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/tree.cc +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/tree.cc @@ -105,7 +105,7 @@ Btree::contains(Transaction& t, const ghobject_t& obj) { // TODO: improve lower_bound() return root->lower_bound(get_context(t), key); }).safe_then([](auto result) { - return MatchKindBS::EQ == result.match; + return MatchKindBS::EQ == result.match(); }); } ); @@ -120,7 +120,7 @@ Btree::find(Transaction& t, const ghobject_t& obj) { // TODO: improve lower_bound() return root->lower_bound(get_context(t), key); }).safe_then([this](auto result) { - if (result.match == MatchKindBS::EQ) { + if (result.match() == MatchKindBS::EQ) { return Cursor(this, result.p_cursor); } else { return Cursor::make_end(this);