FIX wbcache for put and modify long running index (#567)

conanfile.py

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

src/include/homestore/btree/btree.hpp

@@ -123,7 +123,7 @@ class Btree {
     void dump_tree_to_file(const std::string& file = "") const;
     std::string to_custom_string(to_string_cb_t< K, V > const& cb) const;
     std::string visualize_tree_keys(const std::string& file) const;
-    uint64_t count_keys(bnodeid_t bnodeid) const;
+    uint64_t count_keys(bnodeid_t bnodeid = 0) const;
 
     nlohmann::json get_metrics_in_json(bool updated = true);
     bnodeid_t root_node_id() const;

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

@@ -201,6 +201,7 @@ void Btree< K, V >::to_dot_keys(bnodeid_t bnodeid, std::string& buf,
 
 template < typename K, typename V >
 uint64_t Btree< K, V >::count_keys(bnodeid_t bnodeid) const {
+    if (bnodeid == 0) { bnodeid = this->root_node_id(); }
     BtreeNodePtr node;
     locktype_t acq_lock = locktype_t::READ;
     if (read_and_lock_node(bnodeid, node, acq_lock, acq_lock, nullptr) != btree_status_t::success) { return 0; }

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

@@ -283,6 +283,11 @@ btree_status_t Btree< K, V >::split_node(const BtreeNodePtr& parent_node, const
     child_node1->inc_link_version();
 
     // Update the existing parent node entry to point to second child ptr.
+    // Don't change the order. First update the parent node and then insert the new key. This is important for casee
+    // where the split key is the last key in the parent node. In this case, the split key should be inserted in the
+    // parent node. If we insert the split key first, then the split key will be inserted in the parent node and the
+    // last key in the parent node will be lost. This will lead to inconsistency in the tree. In case of empty parent
+    // (i.e., new root) or updating the edge, this order made sure that edge is updated.
     parent_node->update(parent_ind, child_node2->link_info());
     parent_node->insert(parent_ind, *out_split_key, child_node1->link_info());
 

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

@@ -204,7 +204,7 @@ class SimpleNode : public VariantNode< K, V > {
     bool has_room_for_put(btree_put_type put_type, uint32_t key_size, uint32_t value_size) const override {
 #ifdef _PRERELEASE
         auto max_keys = max_keys_in_node();
-            if(max_keys) {return (this->total_entries() < max_keys);}
+        if(max_keys) {return (this->total_entries() < max_keys);}
 #endif
         return ((put_type == btree_put_type::UPSERT) || (put_type == btree_put_type::INSERT))
             ? (get_available_entries() > 0)
@@ -230,7 +230,7 @@ class SimpleNode : public VariantNode< K, V > {
         return str;
     }
     std::string to_dot_keys() const override {
-        return to_dot_keys_impl(std::is_same<K, uint64_t>{});
+        return to_dot_keys_impl(std::is_same<decltype(std::declval<K &>().key()), uint64_t>{});
     }
 
     std::string to_dot_keys_impl(std::false_type) const {

src/include/homestore/index/index_table.hpp

@@ -224,10 +224,13 @@ class IndexTable : public IndexTableBase, public Btree< K, V > {
     }
 
     btree_status_t on_root_changed(BtreeNodePtr const& new_root, void* context) override {
+        // todo: if(m_sb->root_node == new_root->node_id() && m_sb->root_link_version == new_root->link_version()){
+        // return btree_status_t::success;}
         m_sb->root_node = new_root->node_id();
         m_sb->root_link_version = new_root->link_version();
 
         if (!wb_cache().refresh_meta_buf(m_sb_buffer, r_cast< CPContext* >(context))) {
+            LOGTRACEMOD(wbcache, "CP mismatch error - discard transact for meta node");
             return btree_status_t::cp_mismatch;
         }
 
@@ -238,8 +241,8 @@ class IndexTable : public IndexTableBase, public Btree< K, V > {
 
     btree_status_t repair_links(BtreeNodePtr const& parent_node, void* cp_ctx) {
         BT_LOG(DEBUG, "Repairing links for parent node {}", parent_node->to_string());
-
-        // Get the last key in the node
+        // TODO: is it possible that repairing many nodes causes an increase to level of btree? If so, then this needs
+        // to be handled. Get the last key in the node
         auto const last_parent_key = parent_node->get_last_key< K >();
         auto const is_parent_edge_node = parent_node->has_valid_edge();
         if ((parent_node->total_entries() == 0) && !is_parent_edge_node) {
@@ -285,8 +288,8 @@ class IndexTable : public IndexTableBase, public Btree< K, V > {
             BT_LOG(INFO, "Repairing node={} child_node={} child_last_key={}", cur_parent->node_id(),
                    child_node->to_string(), child_last_key.to_string());
 
-            if (child_last_key.compare(last_parent_key) > 0) {
-                // We have reached the last key, we can stop now
+            if (child_last_key.compare(last_parent_key) > 0 && !is_parent_edge_node) {
+                // We have reached the last key, and the parent node doesn't have edge, so we can stop now
                 break;
             }
 

src/include/homestore/index_service.hpp

@@ -70,6 +70,7 @@ class IndexService {
     void stop();
 
     // Add/Remove Index Table to/from the index service
+    uint64_t num_tables();
     void add_index_table(const std::shared_ptr< IndexTableBase >& tbl);
     void remove_index_table(const std::shared_ptr< IndexTableBase >& tbl);
     std::shared_ptr< IndexTableBase > get_index_table(uuid_t uuid) const;

src/lib/index/index_cp.cpp

@@ -142,6 +142,17 @@ void IndexCPContext::to_string_dot(const std::string& filename) {
     file << "}\n";
 
     file.close();
+    LOGINFO("cp dag is stored in file {}", filename);
+}
+
+uint16_t IndexCPContext::num_dags()  {
+    // count number of buffers whose up_buffers are nullptr
+    uint16_t count = 0;
+    std::unique_lock lg{m_flush_buffer_mtx};
+    m_dirty_buf_list.foreach_entry([&count](IndexBufferPtr buf) {
+        if (buf->m_up_buffer == nullptr) { count++; }
+    });
+    return count;
 }
 
 std::string IndexCPContext::to_string_with_dags() {
@@ -219,6 +230,7 @@ std::map< BlkId, IndexBufferPtr > IndexCPContext::recover(sisl::byte_view sb) {
 
         process_txn_record(rec, buf_map);
         cur_ptr += rec->size();
+        LOGTRACEMOD(wbcache, "Recovered txn record: {}: {}", t, rec->to_string());
     }
 
     return buf_map;
@@ -314,8 +326,8 @@ std::string IndexCPContext::txn_record::to_string() const {
         if (id_count == 0) {
             fmt::format_to(std::back_inserter(str), "empty]");
         } else {
-            for (uint8_t i{0}; i < id_count; ++i, ++idx) {
-                fmt::format_to(std::back_inserter(str), "[chunk={}, blk={}],", ids[idx].second, ids[idx].first);
+            for (uint8_t i{0}; i < id_count; ++i) {
+                fmt::format_to(std::back_inserter(str), "[{}],", blk_id(idx++).to_integer());
             }
             fmt::format_to(std::back_inserter(str), "]");
         }
@@ -324,12 +336,13 @@ std::string IndexCPContext::txn_record::to_string() const {
     std::string str = fmt::format("ordinal={}, parent=[{}], in_place_child=[{}]", index_ordinal, parent_id_string(),
                                   child_id_string(), num_new_ids, num_freed_ids);
 
-    uint8_t idx = (has_inplace_parent == 0x1) ? 1 : 0 + (has_inplace_child == 0x1) ? 1 : 0;
+    uint8_t idx = ((has_inplace_parent == 0x1) ? 1 : 0) + ((has_inplace_child == 0x1) ? 1 : 0);
     fmt::format_to(std::back_inserter(str), ", new_ids=[");
     add_to_string(str, idx, num_new_ids);
 
     fmt::format_to(std::back_inserter(str), ", freed_ids=[");
     add_to_string(str, idx, num_freed_ids);
+    fmt::format_to(std::back_inserter(str), "{}", (is_parent_meta ? ", parent is meta" : ""));
     return str;
 }
 } // namespace homestore

src/lib/index/index_cp.hpp

@@ -94,12 +94,12 @@ struct IndexCPContext : public VDevCPContext {
         }
 
         std::string parent_id_string() const {
-            return (has_inplace_parent == 0x1) ? fmt::format("chunk={}, blk={}", ids[0].second, ids[0].first) : "empty";
+            return (has_inplace_parent == 0x1) ? fmt::format("{}", blk_id(0).to_integer()) : "empty";
         }
 
         std::string child_id_string() const {
             auto const idx = (has_inplace_parent == 0x1) ? 1 : 0;
-            return (has_inplace_child == 0x1) ? fmt::format("chunk={}, blk={}", ids[idx].second, ids[idx].first)
+            return (has_inplace_child == 0x1) ? fmt::format("{}", blk_id(idx).to_integer())
                                               : "empty";
         }
 
@@ -162,6 +162,7 @@ struct IndexCPContext : public VDevCPContext {
     std::optional< IndexBufferPtr > next_dirty();
     std::string to_string();
     std::string to_string_with_dags();
+    uint16_t num_dags();
     void to_string_dot(const std::string& filename);
 
 private:

src/lib/index/index_service.cpp

@@ -87,10 +87,18 @@ void IndexService::start() {
     for (const auto& [_, tbl] : m_index_map) {
         tbl->recovery_completed();
     }
+    // Force taking cp after recovery done. This makes sure that the index table is in consistent state and dirty buffer
+    // after recovery can be added to dirty list for flushing in the new cp
+    hs()->cp_mgr().trigger_cp_flush(true /* force */);
 }
 
 void IndexService::stop() { m_wb_cache.reset(); }
 
+uint64_t IndexService::num_tables() {
+    std::unique_lock lg(m_index_map_mtx);
+    return m_index_map.size();
+}
+
 void IndexService::add_index_table(const std::shared_ptr< IndexTableBase >& tbl) {
     std::unique_lock lg(m_index_map_mtx);
     m_index_map.insert(std::make_pair(tbl->uuid(), tbl));