From: Yingxin Cheng Date: Wed, 19 May 2021 07:36:53 +0000 (+0800) Subject: crimson/onode-staged-tree: distinguish extent state between retired and invalid X-Git-Tag: v17.1.0~1880^2~9 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=167560f5a57cd52b54bd40746ed6d2a5a720a5e4;p=ceph.git crimson/onode-staged-tree: distinguish extent state between retired and invalid 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 fdcdc8668fa3..57fd5b1e69c4 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc @@ -284,13 +284,13 @@ Node::~Node() // 2. unlink parent/super --ptr-> me // 3. unlink me --ref-> parent/super // 4. extent is retired - assert(!impl->is_extent_valid()); + assert(impl->is_extent_retired()); // TODO: maybe its possible when eagain happens internally, we should // revisit to make sure tree operations can be aborted normally, // without resource leak or hitting unexpected asserts. } else { - assert(impl->is_extent_valid()); + assert(!impl->is_extent_retired()); if (is_root()) { super->do_untrack_root(*this); } else { @@ -2058,6 +2058,7 @@ void LeafNode::validate_cursor(const tree_cursor_t& cursor) const #ifndef NDEBUG assert(this == cursor.get_leaf_node().get()); assert(cursor.is_tracked()); + assert(!impl->is_extent_retired()); auto [key, p_value_header] = get_kv(cursor.get_position()); auto magic = p_value_header->magic; assert(key.compare_to(cursor.get_key_view(magic)) == MatchKindCMP::EQ); 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 37eee930b770..ec42c4f569e1 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 @@ -301,6 +301,7 @@ class NodeExtentAccessorT { extent->get_recorder()->is_empty()); recorder = nullptr; } else { + // extent is invalid or retired ceph_abort("impossible path"); } #ifndef NDEBUG @@ -319,25 +320,26 @@ class NodeExtentAccessorT { const node_stage_t& read() const { return node_stage; } laddr_t get_laddr() const { return extent->get_laddr(); } nextent_state_t get_state() const { - assert(extent->is_valid()); + assert(!is_retired()); // we cannot rely on the underlying extent state because // FRESH/MUTATION_PENDING can become DIRTY after transaction submission. return state; } - bool is_valid() const { + bool is_retired() const { if (extent) { - assert(extent->is_valid()); - return true; - } else { + // XXX SeaStore extent cannot distinguish between invalid and retired. + // assert(extent->is_valid()); return false; + } else { + return true; } } // must be called before any mutate attempes. // for the safety of mixed read and mutate, call before read. void prepare_mutate(context_t c) { - assert(extent->is_valid()); + assert(!is_retired()); if (state == nextent_state_t::READ_ONLY) { assert(!extent->is_pending()); auto ref_recorder = recorder_t::create_for_encode(c.vb); @@ -494,7 +496,7 @@ class NodeExtentAccessorT { eagain_future rebuild(context_t c) { LOG_PREFIX(OTree::Extent::rebuild); - assert(extent->is_valid()); + assert(!is_retired()); if (state == nextent_state_t::FRESH) { assert(extent->is_initial_pending()); // already fresh and no need to record @@ -552,7 +554,7 @@ class NodeExtentAccessorT { eagain_future<> retire(context_t c) { LOG_PREFIX(OTree::Extent::retire); - assert(extent->is_valid()); + assert(!is_retired()); auto addr = extent->get_laddr(); return c.nm.retire_extent(c.t, std::move(extent) ).handle_error( 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 07cbeffe89b4..bd3432ba0612 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 @@ -82,7 +82,7 @@ class NodeImpl { virtual level_t level() const = 0; virtual node_offset_t free_size() const = 0; virtual node_offset_t total_size() const = 0; - virtual bool is_extent_valid() const = 0; + virtual bool is_extent_retired() const = 0; virtual std::optional get_pivot_index() const = 0; virtual bool is_size_underflow() const = 0; 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 33c27cb4ec0e..a54bd056b6b2 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 @@ -120,7 +120,7 @@ class NodeLayoutT final : public InternalNodeImpl, public LeafNodeImpl { level_t level() const override { return extent.read().level(); } node_offset_t free_size() const override { return extent.read().free_size(); } node_offset_t total_size() const override { return extent.read().total_size(); } - bool is_extent_valid() const override { return extent.is_valid(); } + bool is_extent_retired() const override { return extent.is_retired(); } std::optional get_pivot_index() const override { if (is_level_tail()) { diff --git a/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc b/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc index 54dab1ca9def..5d11f8c9764e 100644 --- a/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc +++ b/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc @@ -828,7 +828,7 @@ class DummyChildPool { laddr_t laddr() const override { return _laddr; } bool is_level_tail() const override { return _is_level_tail; } std::optional get_pivot_index() const override { return {key_view}; } - bool is_extent_valid() const override { return _is_extent_valid; } + bool is_extent_retired() const override { return _is_extent_retired; } const std::string& get_name() const override { return name; } search_position_t make_tail() override { _is_level_tail = true; @@ -836,7 +836,8 @@ class DummyChildPool { return search_position_t::end(); } eagain_future<> retire_extent(context_t) override { - _is_extent_valid = false; + assert(!_is_extent_retired); + _is_extent_retired = true; return seastar::now(); } @@ -898,7 +899,7 @@ class DummyChildPool { bool _is_level_tail; laddr_t _laddr; std::string name; - bool _is_extent_valid = true; + bool _is_extent_retired = false; key_view_t key_view; void* p_mem_key_view;