crimson/onode-staged-tree: distinguish extent state between retired and invalid

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>

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 fdcdc8668fa3e673a1eeb3c779b1ac7b45c0fb56..57fd5b1e69c4c129a0123fbcb04391f0c3b74b68 100644 (file)
--- 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 37eee930b77094d9bccf7ba6ebf75e6877acea3a..ec42c4f569e1842990603b0012d8c36a3f54cb7e 100644 (file)
--- 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<NodeExtentMutable> 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 07cbeffe89b4a62227ac5b808d8f11d19317d0c4..bd3432ba0612008c9b20055c4181acc32c4c164f 100644 (file)
--- 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<key_view_t> 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 33c27cb4ec0e7650d0ae257984d4a5432ead566d..a54bd056b6b2f87331c77e73b5914b79d8d43cc8 100644 (file)
--- 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<key_view_t> 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 54dab1ca9def8fb4ffed1e76a820fc0a4126c3c0..5d11f8c9764e19af47234803e08410e0994eca6a 100644 (file)
--- 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<key_view_t> 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;