FIX remove (#239)

conanfile.py

@@ -5,7 +5,7 @@
 
 class HomestoreConan(ConanFile):
     name = "homestore"
-    version = "4.9.1"
+    version = "4.9.2"
 
     homepage = "https://github.com/eBay/Homestore"
     description = "HomeStore Storage Engine"

src/include/homestore/btree/detail/btree_remove_impl.ipp

@@ -335,14 +335,23 @@ btree_status_t Btree< K, V >::merge_nodes(const BtreeNodePtr& parent_node, const
 
         auto& old_ith_node = old_nodes[src_cursor.ith_node];
         auto const nentries = new_node->copy_by_size(m_bt_cfg, *old_ith_node, src_cursor.nth_entry, available_size);
+        total_size -= new_node->occupied_size();
         if (old_ith_node->total_entries() == (src_cursor.nth_entry + nentries)) {
             // Copied entire node
             ++src_cursor.ith_node;
             src_cursor.nth_entry = 0;
             available_size = balanced_size - new_node->occupied_size();
         } else {
-            src_cursor.nth_entry += nentries;
-            available_size = 0;
+            //  If it is the last node supposed to be, check if the remaining entries can be copied and not creating a
+            //  new nodes. This will make the last new node a little skewed from balanced size due to large key/values but
+            //  avoid making extra new node.
+            if (new_nodes.size() == num_nodes - 1 && total_size < new_node->available_size()) {
+                available_size = new_node->available_size();
+                src_cursor.nth_entry += nentries;
+            } else {
+                src_cursor.nth_entry += nentries;
+                available_size = 0;
+            }
         }
     }
 
@@ -362,6 +371,7 @@ btree_status_t Btree< K, V >::merge_nodes(const BtreeNodePtr& parent_node, const
     }
 
     if (!K::is_fixed_size()) {
+#if 0
         // Lets see if we have enough room in parent node to accommodate changes. This is needed only if the key is not
         // fixed length. For fixed length node merge will always result in less or equal size
         auto excess_releasing_nodes =
@@ -375,14 +385,13 @@ btree_status_t Btree< K, V >::merge_nodes(const BtreeNodePtr& parent_node, const
             goto out;
         }
 
-#if 0
+#endif
         // we first calculate the least amount of space being released after removing excess children. the key size
         // cannot be taken account; so we know for sure that value (i.e., linkinfo) and also its record will be freed.
         // If the end_idx is the parent's edge, the space is not released eventually.
         auto excess_releasing_nodes =
             old_nodes.size() - new_nodes.size() - (parent_node->total_entries() == end_idx) ? 1 : 0;
-        auto minimum_releasing_excess_size =
-            excess_releasing_nodes * (BtreeLinkInfo::get_fixed_size() + parent_node->get_record_size());
+        auto minimum_releasing_excess_size = excess_releasing_nodes * (BtreeLinkInfo::get_fixed_size());
 
         // aside from releasing size due to excess node, K::get_max_size is needed for each updating element
         // at worst case (linkinfo and record remain the same for old and new nodes). The number of updating elements
@@ -396,7 +405,6 @@ btree_status_t Btree< K, V >::merge_nodes(const BtreeNodePtr& parent_node, const
             ret = btree_status_t::merge_not_required;
             goto out;
         }
-#endif
     }
 
     // Now it is time to commit things and at this point no going back, since in-place write nodes are modified

src/include/homestore/btree/detail/simple_node.hpp

@@ -229,11 +229,9 @@ class SimpleNode : public VariantNode< K, V > {
     std::string to_string_keys(bool print_friendly = false) const override {
 #if 0
         std::string delimiter = print_friendly ? "\n" : "\t";
-        auto str = fmt::format("{}{}.{} nEntries={} {} ",
         auto str = fmt::format("{}{}.{} nEntries={} {} ",
                                print_friendly ? "------------------------------------------------------------\n" : "",
                                this->node_id(), this->link_version(), this->total_entries(), (this->is_leaf() ? "LEAF" : "INTERIOR"));
-                               this->node_id(), this->link_version(), this->total_entries(), (this->is_leaf() ? "LEAF" : "INTERIOR"));
         if (!this->is_leaf() && (this->has_valid_edge())) {
             fmt::format_to(std::back_inserter(str), "edge_id={}.{}", this->edge_info().m_bnodeid,
                            this->edge_info().m_link_version);

src/include/homestore/btree/detail/varlen_node.hpp

@@ -71,6 +71,10 @@ class VariableNode : public VariantNode< K, V > {
 
     virtual ~VariableNode() = default;
 
+    uint32_t occupied_size() const override {
+        return (get_var_node_header_const()->m_init_available_space - sizeof(var_node_header) - available_size());
+    }
+
     /* Insert the key and value in provided index
      * Assumption: Node lock is already taken */
     btree_status_t insert(uint32_t ind, const BtreeKey& key, const BtreeValue& val) override {
@@ -517,11 +521,9 @@ class VariableNode : public VariantNode< K, V > {
     std::string to_string_keys(bool print_friendly = false) const override {
 #if 0
         std::string delimiter = print_friendly ? "\n" : "\t";
-        auto str = fmt::format("{}{}.{} nEntries={} {} ",
         auto str = fmt::format("{}{}.{} nEntries={} {} ",
                                print_friendly ? "------------------------------------------------------------\n" : "",
                                this->node_id(), this->link_version(), this->total_entries(), (this->is_leaf() ? "LEAF" : "INTERIOR"));
-                               this->node_id(), this->link_version(), this->total_entries(), (this->is_leaf() ? "LEAF" : "INTERIOR"));
         if (!this->is_leaf() && (this->has_valid_edge())) {
             fmt::format_to(std::back_inserter(str), "edge_id={}.{}", this->edge_info().m_bnodeid,
                            this->edge_info().m_link_version);

src/lib/common/resource_mgr.cpp

@@ -36,7 +36,7 @@ void ResourceMgr::dec_dirty_buf_size(const uint32_t size) {
     HS_REL_ASSERT_GT(size, 0);
     const int64_t dirty_buf_cnt = m_hs_dirty_buf_cnt.fetch_sub(size, std::memory_order_relaxed);
     COUNTER_DECREMENT(m_metrics, dirty_buf_cnt, size);
-    HS_REL_ASSERT_GE(dirty_buf_cnt, size);
+    HS_REL_ASSERT_GE(dirty_buf_cnt, 0);
 }
 
 void ResourceMgr::register_dirty_buf_exceed_cb(exceed_limit_cb_t cb) { m_dirty_buf_exceed_cb = std::move(cb); }

src/tests/btree_helpers/btree_test_helper.hpp

@@ -294,9 +294,7 @@ struct BtreeTestHelper : public testing::Test {
 
     void multi_op_execute(const std::vector< std::pair< std::string, int > >& op_list) {
         preload(SISL_OPTIONS["preload_size"].as< uint32_t >());
-        print_keys();
         run_in_parallel(op_list);
-        print_keys();
     }
 
     void print(const std::string& file = "") const { m_bt->print_tree(file); }