crimson/os/seastore/linked_tree_node: correct balance pivots calculation

for both fixed-kv tree and omap tree

Signed-off-by: Xuehan Xu <xuxuehan@qianxin.com>
(cherry picked from commit 50e9dede589dfc5863e0d212ae011da1afb49997)

diff --git a/src/crimson/common/fixed_kv_node_layout.h b/src/crimson/common/fixed_kv_node_layout.h
index db62a2df32dd6231528b90d23053edf2eab7a58f..cdba35520d613f8c2e67534d732627ec070b907f 100644 (file)
--- a/src/crimson/common/fixed_kv_node_layout.h
+++ b/src/crimson/common/fixed_kv_node_layout.h
@@ -558,6 +558,19 @@ public:
     set_meta(Meta::merge_from(left.get_meta(), right.get_meta()));
   }
 
+  static uint32_t get_balance_pivot_idx(
+    const FixedKVNodeLayout &left,
+    const FixedKVNodeLayout &right,
+    bool prefer_left)
+  {
+    auto total = left.get_size() + right.get_size();
+    auto pivot_idx = total / 2;
+    if (total % 2 && prefer_left) {
+      pivot_idx++;
+    }
+    return pivot_idx;
+  }
+
   /**
    * balance_into_new_nodes
    *
@@ -574,10 +587,7 @@ public:
     FixedKVNodeLayout &replacement_right)
   {
     auto total = left.get_size() + right.get_size();
-    auto pivot_idx = (left.get_size() + right.get_size()) / 2;
-    if (total % 2 && prefer_left) {
-      pivot_idx++;
-    }
+    auto pivot_idx = get_balance_pivot_idx(left, right, prefer_left);
     auto replacement_pivot = pivot_idx >= left.get_size() ?
       right.iter_idx(pivot_idx - left.get_size())->get_key() :
       left.iter_idx(pivot_idx)->get_key();

diff --git a/src/crimson/os/seastore/linked_tree_node.h b/src/crimson/os/seastore/linked_tree_node.h
index 9ab7b750b18c2edf96c269859c007b54fb3eaeca..d420ac2ffe8b404db671bd4c4ac21c66b22fa6be 100644 (file)
--- a/src/crimson/os/seastore/linked_tree_node.h
+++ b/src/crimson/os/seastore/linked_tree_node.h
@@ -638,11 +638,7 @@ protected:
   {
     size_t l_size = left.get_size();
     size_t r_size = right.get_size();
-    size_t total = l_size + r_size;
-    size_t pivot_idx = (l_size + r_size) / 2;
-    if (total % 2 && prefer_left) {
-      pivot_idx++;
-    }
+    size_t pivot_idx = T::get_balance_pivot_idx(left, right, prefer_left);
 
     replacement_left.maybe_expand_children(pivot_idx);
     replacement_right.maybe_expand_children(r_size + l_size - pivot_idx);

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 266bde312fd03f604ee57e5a3a61569ac7d2399f..057752eb6aa0eccc54c4b8da2ef273dd6f66c830 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
@@ -490,10 +490,10 @@ OMapInnerNode::merge_entry(
       ).si_then([liter=liter, riter=riter, l=l, r=r, oc, this](auto tuple) {
        LOG_PREFIX(OMapInnerNode::merge_entry);
         auto [replacement_l, replacement_r, replacement_pivot] = tuple;
-       DEBUGT("to update parent: {} {} {}",
-         oc.t, *this, *replacement_l, *replacement_r);
        replacement_l->init_range(l->get_begin(), replacement_pivot);
        replacement_r->init_range(replacement_pivot, r->get_end());
+       DEBUGT("to update parent: {} {} {}",
+         oc.t, *this, *replacement_l, *replacement_r);
        if (get_meta().depth > 2) { // l and r are inner nodes
          auto &left = *l->template cast<OMapInnerNode>();
          auto &right = *r->template cast<OMapInnerNode>();

diff --git a/src/crimson/os/seastore/omap_manager/btree/string_kv_node_layout.h b/src/crimson/os/seastore/omap_manager/btree/string_kv_node_layout.h
index d0991de5023e214777f8b2a7528dfe613d78588e..10f070479dd5fde99c19f7943bef9a87b1b27d87 100644 (file)
--- a/src/crimson/os/seastore/omap_manager/btree/string_kv_node_layout.h
+++ b/src/crimson/os/seastore/omap_manager/btree/string_kv_node_layout.h
@@ -738,21 +738,16 @@ public:
     set_meta(omap_node_meta_t::merge_from(left.get_meta(), right.get_meta()));
   }
 
-  /**
-   * balance_into_new_nodes
-   *
-   * Takes the contents of left and right and copies them into
-   * replacement_left and replacement_right such that
-   * the size of replacement_left just >= 1/2 of (left + right)
-   */
-  static std::string balance_into_new_nodes(
+  static uint32_t get_balance_pivot_idx(
     const StringKVInnerNodeLayout &left,
     const StringKVInnerNodeLayout &right,
-    StringKVInnerNodeLayout &replacement_left,
-    StringKVInnerNodeLayout &replacement_right)
+    // this is just for the consistency with linked tree nodes
+    bool prefer_left = false)
   {
-    uint32_t left_size = omap_inner_key_t(left.get_node_key_ptr()[left.get_size()-1]).key_off;
-    uint32_t right_size = omap_inner_key_t(right.get_node_key_ptr()[right.get_size()-1]).key_off;
+    uint32_t left_size = omap_inner_key_t(
+      left.get_node_key_ptr()[left.get_size()-1]).key_off;
+    uint32_t right_size = omap_inner_key_t(
+      right.get_node_key_ptr()[right.get_size()-1]).key_off;
     uint32_t total = left_size + right_size;
     uint32_t pivot_size = total / 2;
     uint32_t pivot_idx = 0;
@@ -778,6 +773,27 @@ public:
         }
       }
     }
+    return pivot_idx;
+  }
+
+  /**
+   * balance_into_new_nodes
+   *
+   * Takes the contents of left and right and copies them into
+   * replacement_left and replacement_right such that
+   * the size of replacement_left just >= 1/2 of (left + right)
+   */
+  static std::string balance_into_new_nodes(
+    const StringKVInnerNodeLayout &left,
+    const StringKVInnerNodeLayout &right,
+    StringKVInnerNodeLayout &replacement_left,
+    StringKVInnerNodeLayout &replacement_right)
+  {
+    uint32_t left_size = omap_inner_key_t(left.get_node_key_ptr()[left.get_size()-1]).key_off;
+    uint32_t right_size = omap_inner_key_t(right.get_node_key_ptr()[right.get_size()-1]).key_off;
+    uint32_t total = left_size + right_size;
+    uint32_t pivot_size = total / 2;
+    uint32_t pivot_idx = get_balance_pivot_idx(left, right);
 
     auto replacement_pivot = pivot_idx >= left.get_size() ?
       right.iter_idx(pivot_idx - left.get_size())->get_key() :