crimson/os/seastore/omap: adjust omap_list_config_t

Add inclusive behavior for both first and last in omap_list_config_t,
and optimize the usage.
Set first closed last open for range for omap range remove.

Signed-off-by: Xinyu Huang <xinyu.huang@intel.com>

diff --git a/src/crimson/os/seastore/omap_manager.h b/src/crimson/os/seastore/omap_manager.h
index ad17b2f007789fa7feaa1494696cda1478457977..2eb5d89f4ae1f9964a04a343b838b33983bb0ac4 100644 (file)
--- a/src/crimson/os/seastore/omap_manager.h
+++ b/src/crimson/os/seastore/omap_manager.h
@@ -114,37 +114,48 @@ public:
     size_t max_result_size = 128;
 
     /// true denotes behavior like lower_bound, upper_bound otherwise
-    bool inclusive = false;
+    /// range start behavior
+    bool first_inclusive = false;
+    /// range end behavior
+    bool last_inclusive = false;
 
     omap_list_config_t(
       size_t max_result_size,
-      bool inclusive)
+      bool first_inclusive,
+      bool last_inclusive)
       : max_result_size(max_result_size),
-       inclusive(inclusive) {}
+       first_inclusive(first_inclusive),
+       last_inclusive(last_inclusive) {}
     omap_list_config_t() {}
     omap_list_config_t(const omap_list_config_t &) = default;
     omap_list_config_t(omap_list_config_t &&) = default;
     omap_list_config_t &operator=(const omap_list_config_t &) = default;
     omap_list_config_t &operator=(omap_list_config_t &&) = default;
 
-    static omap_list_config_t with_max(size_t max) {
-      omap_list_config_t ret{};
-      ret.max_result_size = max;
-      return ret;
+    auto with_max(size_t max) {
+      this->max_result_size = max;
+      return *this;
     }
 
-    static omap_list_config_t with_inclusive(bool inclusive) {
-      omap_list_config_t ret{};
-      ret.inclusive = inclusive;
-      return ret;
+    auto without_max() {
+      this->max_result_size = std::numeric_limits<size_t>::max();
+      return *this;
+    }
+
+    auto with_inclusive(
+      bool first_inclusive,
+      bool last_inclusive) {
+      this->first_inclusive = first_inclusive;
+      this->last_inclusive = last_inclusive;
+      return *this;
     }
 
     auto with_reduced_max(size_t reduced_by) const {
       assert(reduced_by <= max_result_size);
       return omap_list_config_t(
        max_result_size - reduced_by,
-       inclusive
-      );
+       first_inclusive,
+       last_inclusive);
     }
   };
   using omap_list_iertr = base_iertr;

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 38b9dd66fadd34f85eee61736e7d9eb840cef9b2..7f0af20f6d1b17be440d7f99e40464aea2888278 100644 (file)
--- 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
@@ -263,7 +263,7 @@ OMapInnerNode::list(
               assert(child_result.begin()->first > result.rbegin()->first);
             }
             if (child_result.size() && first && iter == fiter) {
-             if (config.inclusive) {
+             if (config.first_inclusive) {
                assert(child_result.begin()->first >= *first);
              } else {
                assert(child_result.begin()->first > *first);
@@ -271,7 +271,7 @@ OMapInnerNode::list(
             }
             if (child_result.size() && last && iter == liter - 1) {
              auto biter = --(child_result.end());
-             if (config.inclusive) {
+             if (config.last_inclusive) {
                assert(biter->first <= *last);
              } else {
                assert(biter->first < *last);
@@ -614,23 +614,25 @@ OMapLeafNode::list(
 {
   LOG_PREFIX(OMapLeafNode::list);
   DEBUGT(
-    "first {} last {}  max_result_size {} inclusive {}, this: {}",
+    "first {} last {}  max_result_size {} first_inclusive {} \
+    last_inclusive {}, this: {}",
     oc.t,
     first ? first->c_str() : "",
     last ? last->c_str() : "",
     config.max_result_size,
-    config.inclusive,
+    config.first_inclusive,
+    config.last_inclusive,
     *this
   );
   auto ret = list_bare_ret(false, {});
   auto &[complete, result] = ret;
   auto iter = first ?
-    (config.inclusive ?
+    (config.first_inclusive ?
      string_lower_bound(*first) :
      string_upper_bound(*first)) :
     iter_begin();
   auto liter = last ?
-    (config.inclusive ?
+    (config.last_inclusive ?
      string_upper_bound(*last) :
      string_lower_bound(*last)) :
     iter_end();

diff --git a/src/crimson/os/seastore/seastore.cc b/src/crimson/os/seastore/seastore.cc
index dad8701db25b5e698690838d12ec428fa3c01908..6f67f6c68ec2cede2a5ab1a23b933da022b000b5 100644 (file)
--- a/src/crimson/os/seastore/seastore.cc
+++ b/src/crimson/os/seastore/seastore.cc
@@ -735,7 +735,7 @@ SeaStore::get_attrs_ertr::future<SeaStore::attrs_t> SeaStore::get_attrs(
     [=, this](auto &t, auto& onode) {
       auto& layout = onode.get_layout();
       return _omap_list(onode, layout.xattr_root, t, std::nullopt,
-        OMapManager::omap_list_config_t::with_inclusive(false)
+        OMapManager::omap_list_config_t().with_inclusive(false, false)
       ).si_then([&layout](auto p) {
         auto& attrs = std::get<1>(p);
         ceph::bufferlist bl;
@@ -938,7 +938,7 @@ SeaStore::omap_get_values_ret_t SeaStore::omap_get_values(
     [this, start, ch=std::move(ch)](auto& oid) {
     return omap_list(
       ch, oid, start,
-      OMapManager::omap_list_config_t::with_inclusive(false));
+      OMapManager::omap_list_config_t().with_inclusive(false, false));
   });
 }
 
@@ -1479,9 +1479,9 @@ SeaStore::tm_ret SeaStore::_omap_rmkeyrange(
        auto &omap_root,
        auto &first,
        auto &last) {
-      auto config = OMapManager::omap_list_config_t::with_inclusive(true);
-      config.max_result_size = 
-       std::numeric_limits<decltype(config.max_result_size)>::max();
+      auto config = OMapManager::omap_list_config_t()
+       .with_inclusive(true, false)
+       .without_max();
       return omap_manager.omap_rm_key_range(
        omap_root,
        *ctx.transaction,

diff --git a/src/test/crimson/seastore/test_omap_manager.cc b/src/test/crimson/seastore/test_omap_manager.cc
index 9e3ee1043de5e61db89a571cf3f3237250e4f742..04b6f66151d0e70567d8cdd586722d591510a02e 100644 (file)
--- a/src/test/crimson/seastore/test_omap_manager.cc
+++ b/src/test/crimson/seastore/test_omap_manager.cc
@@ -143,8 +143,9 @@ struct omap_manager_test_t :
     const std::string &first,
     const std::string &last) {
     logger().debug("rm keys in range {} ~ {}", first, last);
-    auto config = OMapManager::omap_list_config_t::with_max(3000);
-    config.inclusive = true;
+    auto config = OMapManager::omap_list_config_t()
+      .with_max(3000)
+      .with_inclusive(true, false);
 
     with_trans_intr(
       t,
@@ -189,9 +190,9 @@ struct omap_manager_test_t :
       logger().debug("list on start ~ end");
     }
 
-    auto config = OMapManager::omap_list_config_t::with_max(max);
-    config.max_result_size = max;
-    config.inclusive = inclusive;
+    auto config = OMapManager::omap_list_config_t()
+      .with_max(max)
+      .with_inclusive(inclusive, false);
 
     auto [complete, results] = with_trans_intr(
       t,
@@ -201,14 +202,14 @@ struct omap_manager_test_t :
 
     test_omap_t::iterator it, lit;
     if (first) {
-      it = config.inclusive ?
+      it = config.first_inclusive ?
        test_omap_mappings.lower_bound(*first) :
        test_omap_mappings.upper_bound(*first);
     } else {
       it = test_omap_mappings.begin();
     }
     if (last) {
-      lit = config.inclusive ?
+      lit = config.last_inclusive ?
        test_omap_mappings.upper_bound(*last) :
        test_omap_mappings.lower_bound(*last);
     } else {