crimson/onode-staged-tree: cleanup and introduce has_single_value()

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 e5b1b13838d6dc3006b2e703b1037ccc3a915b74..66fb3731279ff98269724a1ef11561fc3433d7fa 100644 (file)
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc
@@ -878,9 +878,7 @@ eagain_future<> InternalNode::erase_child(context_t c, Ref<Node>&& child_ref)
     return child_ref->retire(c, std::move(child_ref)
     ).safe_then([c, this, child_pos, FNAME,
                  this_ref = std::move(this_ref)] () mutable {
-      if ((impl->is_level_tail() && impl->is_keys_empty()) ||
-          (!impl->is_level_tail() && impl->is_keys_one())) {
-        // there is only one value left
+      if (impl->has_single_value()) {
         // fast path without mutating the extent
         DEBUGT("{} has one value left, erase ...", c.t, get_name());
 #ifndef NDEBUG
@@ -1767,11 +1765,11 @@ LeafNode::erase(context_t c, const search_position_t& pos, bool get_next)
         [c, &pos, this_ref = std::move(this_ref), this, FNAME] () mutable {
 #ifndef NDEBUG
       assert(!impl->is_keys_empty());
-      if (impl->is_keys_one()) {
+      if (impl->has_single_value()) {
         assert(pos == search_position_t::begin());
       }
 #endif
-      if (!is_root() && impl->is_keys_one()) {
+      if (!is_root() && impl->has_single_value()) {
         // we need to keep the root as an empty leaf node
         // fast path without mutating the extent
         // track_erase

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 bd3432ba0612008c9b20055c4181acc32c4c164f..08648658746e596b1ebf99dc0f9380d29a8b050d 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
@@ -76,8 +76,8 @@ class NodeImpl {
    */
   virtual void validate_non_empty() const = 0;
   virtual bool is_keys_empty() const = 0;
-  // under the assumption that keys are not empty, check whether num_keys == 1
-  virtual bool is_keys_one() const = 0;
+  // under the assumption that node is not empty
+  virtual bool has_single_value() const = 0;
 
   virtual level_t level() const = 0;
   virtual node_offset_t free_size() 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 a54bd056b6b2f87331c77e73b5914b79d8d43cc8..5e156995cf81079824a9a3b3c673cc101fc8a7cb 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
@@ -111,10 +111,17 @@ class NodeLayoutT final : public InternalNodeImpl, public LeafNodeImpl {
     }
     assert(!is_keys_empty());
   }
+
   bool is_keys_empty() const override { return extent.read().keys() == 0; }
-  bool is_keys_one() const override {
-    assert(!is_keys_empty());
-    return STAGE_T::is_keys_one(extent.read());
+
+  bool has_single_value() const override {
+    validate_non_empty();
+    if constexpr (NODE_TYPE == node_type_t::INTERNAL) {
+      return ((is_level_tail() && is_keys_empty()) ||
+              (!is_level_tail() && STAGE_T::is_keys_one(extent.read())));
+    } else {
+      return STAGE_T::is_keys_one(extent.read());
+    }
   }
 
   level_t level() const override { return extent.read().level(); }

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 250e1ddcd9aab35e4bdf1522188fcc1945eb3e73..1fe6ff3a4d8e0526f7bf016462404ba5de70d876 100644 (file)
--- a/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc
+++ b/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc
@@ -855,7 +855,7 @@ class DummyChildPool {
       ceph_abort("impossible path"); }
     bool is_keys_empty() const override {
       ceph_abort("impossible path"); }
-    bool is_keys_one() const override {
+    bool has_single_value() const override {
       ceph_abort("impossible path"); }
     node_offset_t free_size() const override {
       ceph_abort("impossible path"); }