Fix prefix merge and enable long running

.jenkins/jenkinsfile_nightly

@@ -55,10 +55,10 @@ pipeline {
                             find /home/jenkins -type f -wholename '*/test_data_service' -exec cp {} .jenkins/test_data_service \\;
                             find /home/jenkins -type f -wholename '*/test_raft_repl_dev' -exec cp {} .jenkins/test_raft_repl_dev \\;
                             find /home/jenkins -type f -wholename '*/test_solo_repl_dev' -exec cp {} .jenkins/test_solo_repl_dev \\;
-                            find /home/jenkins -type f -wholename '*/scripts/index_test.py' -exec install -Dm755 {} .jenkins/index_test.py \\;
-                            find /home/jenkins -type f -wholename '*/scripts/log_meta_test.py' -exec install -Dm755 {} .jenkins/log_meta_test.py \\;
-                            find /home/jenkins -type f -wholename '*/scripts/data_test.py' -exec install -Dm755 {} .jenkins/data_test.py \\;
-                            find /home/jenkins -type f -wholename '*/scripts/long_running.py' -exec install -Dm755 {} .jenkins/long_running.py \\;
+                            find /home/jenkins -type f -wholename '*/test_scripts/index_test.py' -exec install -Dm755 {} .jenkins/index_test.py \\;
+                            find /home/jenkins -type f -wholename '*/test_scripts/log_meta_test.py' -exec install -Dm755 {} .jenkins/log_meta_test.py \\;
+                            find /home/jenkins -type f -wholename '*/test_scripts/data_test.py' -exec install -Dm755 {} .jenkins/data_test.py \\;
+                            find /home/jenkins -type f -wholename '*/test_scripts/long_running.py' -exec install -Dm755 {} .jenkins/long_running.py \\;
                         '''
                     }
                     post {

conanfile.py

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

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

@@ -267,6 +267,9 @@ btree_status_t Btree< K, V >::merge_nodes(const BtreeNodePtr& parent_node, const
         BT_NODE_LOG_ASSERT_EQ(child->is_node_deleted(), false, child);
 
         old_nodes.push_back(child);
+        // Todo: need a more precise calculation considering compacted size for prefix nodes because when merge happens
+        // compaction will occur for both leftmost and new nodes. This calculation makes available size not be balanced
+        // for the leftmost node and new nodes.
         total_size += child->occupied_size();
     }
 
@@ -323,6 +326,13 @@ btree_status_t Btree< K, V >::merge_nodes(const BtreeNodePtr& parent_node, const
 
         if ((old_nodes[i]->total_entries() - nentries) == 0) { // Entire node goes in
             available_size -= old_nodes[i]->occupied_size();
+            // For prefix nodes, compaction will make the size smaller, so we can compact saving to available size;
+            // hence it cannot get negative.
+            if (old_nodes[i]->get_node_type() == btree_node_type::PREFIX) {
+                auto cur_node = static_cast< FixedPrefixNode< K, V >* >(old_nodes[i].get());
+                available_size += cur_node->compact_saving();
+            }
+            BT_NODE_DBG_ASSERT_EQ(available_size >= 0, true, leftmost_node, "negative available size");
             if (i >= old_nodes.size() - 1) {
                 src_cursor.ith_node = i + 1;
                 src_cursor.nth_entry = std::numeric_limits< uint32_t >::max();

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

@@ -43,7 +43,7 @@ class FixedPrefixNode : public VariantNode< K, V > {
 #pragma pack(1)
     struct prefix_node_header {
         uint16_t used_slots; // Number of slots actually used. TODO: We can deduce from set_bit_count of bitset
-        uint16_t tail_slot;  // What is the tail slot number being used
+        uint16_t tail_slot;  // The tail slot number being used. Address will point to the beginning of tail prefix
 
         std::string to_string() const { return fmt::format("slots_used={} tail_slot={} ", used_slots, tail_slot); }
 
@@ -152,6 +152,7 @@ class FixedPrefixNode : public VariantNode< K, V > {
     FixedPrefixNode(uint8_t* node_buf, bnodeid_t id, bool init, bool is_leaf, const BtreeConfig& cfg) :
             VariantNode< K, V >(node_buf, id, init, is_leaf, cfg),
             prefix_bitset_{sisl::blob{bitset_area(), reqd_bitset_size(cfg)}, init} {
+        this->set_node_type(btree_node_type::PREFIX);
         if (init) {
             auto phdr = prefix_header();
             phdr->used_slots = 0;
@@ -305,7 +306,6 @@ class FixedPrefixNode : public VariantNode< K, V > {
                 }
             }
             if (num_removed) { this->inc_gen(); }
-
 #ifndef NDEBUG
             validate_sanity();
 #endif
@@ -338,10 +338,18 @@ class FixedPrefixNode : public VariantNode< K, V > {
         }
     }
 
+    uint16_t get_nth_suffix_slot_num(uint32_t idx) const { return get_suffix_entry_c(idx)->prefix_slot; }
+
+    uint16_t get_nth_prefix_ref_count(uint32_t idx) const {
+        return get_prefix_entry_c(get_suffix_entry_c(idx)->prefix_slot)->ref_count;
+    }
+
+    uint32_t compact_saving() const { return num_prefix_holes() * prefix_entry::size(); }
+
     uint32_t available_size() const override {
         auto num_holes = num_prefix_holes();
         if (num_holes > prefix_node_header::min_holes_to_compact) {
-            return available_size_without_compaction() + (num_holes * prefix_entry::size());
+            return available_size_with_compaction();
         } else {
             return available_size_without_compaction();
         }
@@ -430,7 +438,6 @@ class FixedPrefixNode : public VariantNode< K, V > {
         // part of Step 1, except generation count
         this->inc_gen();
         dst_node.inc_gen();
-        auto new_phdr = dst_node.prefix_header();
 
         if (!this->is_leaf() && (dst_node.total_entries() != 0)) {
             // Incase this node is an edge node, move the stick to the right hand side node
@@ -527,7 +534,9 @@ class FixedPrefixNode : public VariantNode< K, V > {
         this->invalidate_edge();
         this->inc_gen();
         prefix_bitset_ = sisl::CompactBitSet{sisl::blob{bitset_area(), reqd_bitset_size(cfg)}, true};
-
+        auto phdr = prefix_header();
+        phdr->used_slots = 0;
+        phdr->tail_slot = 0;
 #ifndef NDEBUG
         validate_sanity();
 #endif
@@ -634,22 +643,25 @@ class FixedPrefixNode : public VariantNode< K, V > {
     }
 
     std::string to_string(bool print_friendly = false) const override {
-        auto str = fmt::format("{}id={} level={} nEntries={} {} next_node={} ",
-                               (print_friendly ? "------------------------------------------------------------\n" : ""),
-                               this->node_id(), this->level(), this->total_entries(),
-                               (this->is_leaf() ? "LEAF" : "INTERIOR"), this->next_bnode());
+        auto str =
+            fmt::format("{}id={} level={} nEntries={} {} next_node={}  available_size={} occupied_size={} ",
+                        (print_friendly ? "------------------------------------------------------------\n" : ""),
+                        this->node_id(), this->level(), this->total_entries(), (this->is_leaf() ? "LEAF" : "INTERIOR"),
+                        this->next_bnode(), this->available_size(), this->occupied_size());
         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);
         }
 
-        fmt::format_to(std::back_inserter(str), "{}Prefix_Hdr={}, Prefix_Bitmap=[{}]\n",
-                       (print_friendly ? "\n\t" : " "), cprefix_header()->to_string(), prefix_bitset_.to_string());
+        fmt::format_to(std::back_inserter(str), "{}Prefix_Hdr=[{}], Prefix_Bitmap = [{}] # of holes = {}\n",
+                       (print_friendly ? "\n\t" : " "), cprefix_header()->to_string(), this->compact_bitset(),
+                       this->num_prefix_holes());
 
         for (uint32_t i{0}; i < this->total_entries(); ++i) {
-            fmt::format_to(std::back_inserter(str), "{}Entry{} [Key={} Val={}]", (print_friendly ? "\n\t" : " "), i + 1,
-                           BtreeNode::get_nth_key< K >(i, false).to_string(),
-                           this->get_nth_value(i, false).to_string());
+            fmt::format_to(std::back_inserter(str), "{}Entry{} [Key={} Val={} slot#={} ref_count={}]",
+                           (print_friendly ? "\n\t" : " "), i + 1, BtreeNode::get_nth_key< K >(i, false).to_string(),
+                           this->get_nth_value(i, false).to_string(), this->get_nth_suffix_slot_num(i),
+                           this->get_nth_prefix_ref_count(i));
         }
         return str;
     }
@@ -678,7 +690,9 @@ class FixedPrefixNode : public VariantNode< K, V > {
 
         auto phdr = prefix_header();
         ++phdr->used_slots;
-        if (slot_num > phdr->tail_slot) { phdr->tail_slot = slot_num; }
+        if (slot_num + 1u > phdr->tail_slot) { phdr->tail_slot = slot_num + 1u; }
+        DEBUG_ASSERT_LE(phdr->used_slots, phdr->tail_slot, "Prefix slot number {} is not less than tail slot number {}",
+                        slot_num, phdr->tail_slot);
         return slot_num;
     }
 
@@ -693,9 +707,9 @@ class FixedPrefixNode : public VariantNode< K, V > {
         if (--pentry->ref_count == 0) {
             --phdr->used_slots;
             prefix_bitset_.reset_bit(slot_num);
-            if ((slot_num != 0) && (slot_num == phdr->tail_slot)) {
+            if (slot_num + 1u == phdr->tail_slot) {
                 uint16_t prev_slot = prefix_bitset_.get_prev_set_bit(slot_num);
-                if (prev_slot != std::numeric_limits< uint16_t >::max()) { phdr->tail_slot = prev_slot; }
+                phdr->tail_slot = prev_slot + 1u;
             }
         }
     }
@@ -711,17 +725,16 @@ class FixedPrefixNode : public VariantNode< K, V > {
         uint8_t const* suffix = r_cast< uint8_t const* >(get_suffix_entry_c(this->total_entries()));
         uint8_t const* prefix = r_cast< uint8_t const* >(get_prefix_entry_c(cprefix_header()->tail_slot));
 
-        if (suffix <= prefix) {
-            return prefix - suffix;
+        if (suffix <= prefix + prefix_entry::size()) {
+            return prefix - suffix + prefix_entry::size();
         } else {
-            DEBUG_ASSERT(false, "Node data is corrupted, suffix area is overlapping prefix area");
+            DEBUG_ASSERT(false, "Node data is corrupted, suffix area is overlapping prefix area {}",
+                         int64_t(suffix - prefix));
             return 0;
         }
     }
 
-    uint32_t available_size_with_compaction() const {
-        return available_size_without_compaction() + (num_prefix_holes() * prefix_entry::size());
-    }
+    uint32_t available_size_with_compaction() const { return available_size_without_compaction() + compact_saving(); }
 
     bool has_room(uint16_t for_nentries) const {
         return (available_size_without_compaction() >= (prefix_entry::size() + (for_nentries * suffix_entry::size())));
@@ -733,7 +746,9 @@ class FixedPrefixNode : public VariantNode< K, V > {
 
     uint32_t num_prefix_holes() const {
         auto phdr = cprefix_header();
-        return (phdr->tail_slot + 1 - phdr->used_slots);
+        DEBUG_ASSERT_LE(phdr->used_slots, phdr->tail_slot, "Prefix slot number {} is not less than tail slot number {}",
+                        phdr->used_slots, phdr->tail_slot);
+        return (phdr->tail_slot - phdr->used_slots);
     }
 
     bool is_compaction_suggested() const { return (num_prefix_holes() > prefix_node_header::min_holes_to_compact); }
@@ -776,6 +791,9 @@ class FixedPrefixNode : public VariantNode< K, V > {
         // Finally adjust the tail offset to the compacted area.
         auto phdr = prefix_header();
         phdr->tail_slot = phdr->used_slots;
+        DEBUG_ASSERT_EQ(phdr->tail_slot, prefix_bitset_.get_next_reset_bit(0u),
+                        "Tail slot is not equal to the next reset bit, not expected");
+        DEBUG_ASSERT_EQ(this->num_prefix_holes(), 0, "Shouldn't be any hole after compression, not expected");
     }
 
 #ifndef NDEBUG
@@ -814,13 +832,15 @@ class FixedPrefixNode : public VariantNode< K, V > {
     uint8_t const* csuffix_kv_area() const { return cbitset_area() + (prefix_bitset_.size() / 8); }
 
     prefix_entry* get_prefix_entry(uint16_t slot_num) {
-        return r_cast< prefix_entry* >(this->node_data_area() +
-                                       (this->node_data_size() - ((slot_num + 1) * prefix_entry::size())));
+        return r_cast< prefix_entry* >(
+            this->node_data_area() +
+            (this->node_data_size() - (static_cast< uint16_t >(slot_num + 1) * prefix_entry::size())));
     }
 
     prefix_entry const* get_prefix_entry_c(uint16_t slot_num) const {
-        return r_cast< prefix_entry const* >(this->node_data_area_const() +
-                                             (this->node_data_size() - ((slot_num + 1) * prefix_entry::size())));
+        return r_cast< prefix_entry const* >(
+            this->node_data_area_const() +
+            (this->node_data_size() - (static_cast< uint16_t >(slot_num + 1) * prefix_entry::size())));
     }
 
     suffix_entry* get_suffix_entry(uint16_t idx) {
@@ -832,5 +852,39 @@ class FixedPrefixNode : public VariantNode< K, V > {
 
     static constexpr uint32_t get_key_size() { return prefix_entry::key_size() + suffix_entry::key_size(); }
     static constexpr uint32_t get_value_size() { return prefix_entry::value_size() + suffix_entry::value_size(); }
+
+    std::string compact_bitset() const {
+        auto x = prefix_bitset_.to_string();
+        std::ostringstream result;
+        std::vector< size_t > indices;
+        for (size_t i = 0; i < x.size(); ++i) {
+            if (x[i] == '1') { indices.push_back(i); }
+        }
+
+        if (indices.empty()) { return result.str(); }
+
+        size_t start = indices[0];
+        size_t end = start;
+        result << "size = " << indices.size() << " : ";
+        for (size_t i = 1; i < indices.size(); ++i) {
+            if (indices[i] == end + 1) {
+                end = indices[i];
+            } else {
+                if (start == end) {
+                    result << start << ", ";
+                } else {
+                    result << start << "-" << end << ", ";
+                }
+                start = end = indices[i];
+            }
+        }
+        if (start == end) {
+            result << start;
+        } else {
+            result << start << "-" << end;
+        }
+
+        return result.str();
+    }
 };
 } // namespace homestore

src/tests/btree_helpers/btree_test_helper.hpp

@@ -250,7 +250,7 @@ struct BtreeTestHelper {
     }
 
     void range_remove_existing_random() {
-        static std::uniform_int_distribution< uint32_t > s_rand_range_generator{2, 5};
+        static std::uniform_int_distribution< uint32_t > s_rand_range_generator{2, 50};
 
         auto const [start_k, end_k] = m_shadow_map.pick_random_existing_keys(s_rand_range_generator(m_re));
         do_range_remove(start_k, end_k, true /* only_existing */);

src/tests/test_btree_node.cpp

@@ -344,6 +344,39 @@ TYPED_TEST(NodeTest, SequentialInsert) {
     this->validate_get_any(98, 102);
 }
 
+TYPED_TEST(NodeTest, SimpleInsert) {
+    auto oc = this->m_node1->occupied_size();
+    this->put(1, btree_put_type::INSERT);
+    this->put(2, btree_put_type::INSERT);
+    this->put(3, btree_put_type::INSERT);
+    this->remove(2);
+    this->remove(1);
+    this->remove(3);
+    auto oc2 = this->m_node1->occupied_size();
+    ASSERT_EQ(oc, oc2) << "Occupied size cannot be more than original size";
+    this->put(1, btree_put_type::INSERT);
+    this->put(2, btree_put_type::INSERT);
+    this->put(3, btree_put_type::INSERT);
+    this->remove(3);
+    this->remove(2);
+    this->remove(1);
+    ASSERT_EQ(oc, oc2) << "Occupied size must be the same as original size";
+
+    this->put(2, btree_put_type::INSERT);
+    this->put(1, btree_put_type::INSERT);
+    this->put(4, btree_put_type::INSERT);
+    this->put(3, btree_put_type::INSERT);
+    for (uint32_t i = 5; i <= 50; ++i) {
+        this->put(i, btree_put_type::INSERT);
+    }
+    LOGDEBUG("Creating a hole with size of 11 for prefix compaction usecase");
+    for (uint32_t i = 10; i <= 20; ++i) {
+        this->remove(i);
+    }
+    this->m_node1->move_out_to_right_by_entries(this->m_cfg, *this->m_node2, 20);
+    this->m_node1->copy_by_entries(this->m_cfg, *this->m_node2, 0, std::numeric_limits< uint32_t >::max());
+}
+
 TYPED_TEST(NodeTest, ReverseInsert) {
     for (uint32_t i{100}; (i > 0 && this->has_room()); --i) {
         this->put(i - 1, btree_put_type::INSERT);
@@ -451,7 +484,6 @@ TYPED_TEST(NodeTest, Move) {
     ASSERT_EQ(this->m_node2->total_entries(), 0u) << "Remove all on right has failed";
     ASSERT_EQ(this->m_node1->total_entries(), list.size()) << "Move in from right has failed";
     this->validate_get_all();
-
     this->m_node1->move_out_to_right_by_entries(this->m_cfg, *this->m_node2, list.size() / 2);
     ASSERT_EQ(this->m_node1->total_entries(), list.size() / 2) << "Move out half entries to right has failed";
     ASSERT_EQ(this->m_node2->total_entries(), list.size() - list.size() / 2)

src/tests/test_common/homestore_test_common.hpp

@@ -353,7 +353,7 @@ class HSTestHelper {
         auto fut = homestore::hs()->cp_mgr().trigger_cp_flush(true /* force */);
         auto on_complete = [&](auto success) {
             HS_REL_ASSERT_EQ(success, true, "CP Flush failed");
-            LOGINFO("CP Flush completed");
+            LOGDEBUG("CP Flush completed");
         };
 
         if (wait) {

src/tests/test_index_btree.cpp

@@ -133,7 +133,8 @@ struct BtreeTest : public BtreeTestHelper< TestType >, public ::testing::Test {
     test_common::HSTestHelper m_helper;
 };
 
-using BtreeTypes = testing::Types< FixedLenBtree, PrefixIntervalBtree, VarKeySizeBtree, VarValueSizeBtree, VarObjSizeBtree >;
+using BtreeTypes =
+    testing::Types< FixedLenBtree, PrefixIntervalBtree, VarKeySizeBtree, VarValueSizeBtree, VarObjSizeBtree >;
 
 TYPED_TEST_SUITE(BtreeTest, BtreeTypes);
 
@@ -200,7 +201,7 @@ TYPED_TEST(BtreeTest, TriggerCacheEviction) {
         s.resource_limits.cache_size_percent = 1u;
         HS_SETTINGS_FACTORY().save();
     });
-    
+
     this->restart_homestore();
 
     LOGINFO("TriggerCacheEviction test start");
@@ -532,6 +533,8 @@ struct BtreeConcurrentTest : public BtreeTestHelper< TestType >, public ::testin
                 this->m_bt->count_keys(this->m_bt->root_node_id()));
         BtreeTestHelper< TestType >::TearDown();
         m_helper.shutdown_homestore(false);
+        this->m_bt.reset();
+        log_obj_life_counter();
     }
 
 private:
@@ -562,6 +565,10 @@ int main(int argc, char* argv[]) {
         auto seed = SISL_OPTIONS["seed"].as< uint64_t >();
         LOGINFO("Using seed {} to sow the random generation", seed);
         g_re.seed(seed);
+    } else {
+        auto seed = std::chrono::system_clock::now().time_since_epoch().count();
+        LOGINFO("No seed provided. Using randomly generated seed: {}", seed);
+        g_re.seed(seed);
     }
     auto ret = RUN_ALL_TESTS();
     return ret;