Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix link version upgrade and enable it #205

Merged
merged 1 commit into from
Oct 23, 2023
+43 −14
Merged

Fix link version upgrade and enable it #205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion conanfile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

class HomestoreConan(ConanFile):
name = "homestore"
version = "4.5.5"
version = "4.5.6"

homepage = "https://github.com/eBay/Homestore"
description = "HomeStore Storage Engine"
Expand Down
2 changes: 1 addition & 1 deletion src/include/homestore/btree/detail/btree_mutate_impl.ipp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ btree_status_t Btree< K, V >::split_node(const BtreeNodePtr& parent_node, const
}
BT_NODE_LOG(TRACE, parent_node, "Available space for split entry={}", parent_node->available_size(m_bt_cfg));

// child_node1->inc_link_version();
child_node1->inc_link_version();

// Update the existing parent node entry to point to second child ptr.
parent_node->update(parent_ind, child_node2->link_info());
Expand Down
6 changes: 5 additions & 1 deletion src/include/homestore/btree/detail/btree_node.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ class BtreeNode : public sisl::ObjLifeCounter< BtreeNode > {
return true;
}

virtual BtreeLinkInfo get_edge_value() const { return BtreeLinkInfo{edge_id(), link_version()}; }
virtual BtreeLinkInfo get_edge_value() const { return BtreeLinkInfo{edge_id(), edge_link_version()}; }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

edge link version needed to chat instead of the node link version


virtual void set_edge_value(const BtreeValue& v) {
const auto b = v.serialize();
Expand Down Expand Up @@ -609,6 +609,10 @@ class BtreeNode : public sisl::ObjLifeCounter< BtreeNode > {

bnodeid_t edge_id() const { return get_persistent_header_const()->edge_info.m_bnodeid; }
void set_edge_id(bnodeid_t edge) { get_persistent_header()->edge_info.m_bnodeid = edge; }
uint64_t edge_link_version() const { return get_persistent_header_const()->edge_info.m_link_version; }
void set_edge_link_version(uint64_t link_version) {
get_persistent_header()->edge_info.m_link_version = link_version;
}

BtreeLinkInfo::bnode_link_info edge_info() const { return get_persistent_header_const()->edge_info; }
void set_edge_info(const BtreeLinkInfo::bnode_link_info& info) { get_persistent_header()->edge_info = info; }
Expand Down
6 changes: 2 additions & 4 deletions src/include/homestore/btree/detail/btree_node_mgr.ipp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,11 @@ btree_status_t Btree< K, V >::get_child_and_lock_node(const BtreeNodePtr& node,
locktype_t int_lock_type, locktype_t leaf_lock_type,
void* context) const {
if (index == node->total_entries()) {
const auto& edge_id{node->edge_id()};
child_info.set_bnode_id(edge_id);
// If bsearch points to last index, it means the search has not found entry unless it is an edge value.
if (!child_info.has_valid_bnode_id()) {
if (!node->has_valid_edge()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: call has_valid_edge() instead of has_valid_bnode_id

BT_NODE_LOG_ASSERT(false, node, "Child index {} does not have valid bnode_id", index);
return btree_status_t::not_found;
}
child_info = node->get_edge_value();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to non-edge child (i.e., get_nth_value in the else statement), both child id and link version need to be populated.

} else {
BT_NODE_LOG_ASSERT_LT(index, node->total_entries(), node);
node->get_nth_value(index, &child_info, false /* copy */);
Expand Down
25 changes: 24 additions & 1 deletion src/include/homestore/btree/detail/btree_remove_impl.ipp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,13 +528,14 @@ btree_status_t Btree< K, V >::merge_nodes(const BtreeNodePtr& parent_node, const
// Finally update the leftmost node with latest key
leftmost_node->set_next_bnode(next_node_id);
if (leftmost_node->total_entries()) {
// leftmost_node->inc_link_version();
leftmost_node->inc_link_version();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enable the the increase link version of left most node in merge

parent_node->update(start_idx, leftmost_node->get_last_key< K >(), leftmost_node->link_info());
}

if (parent_node->total_entries() && !parent_node->has_valid_edge()) {
if (parent_node->compare_nth_key(plast_key, parent_node->total_entries() - 1)) {
auto last_node = new_nodes.size() > 0 ? new_nodes[new_nodes.size() - 1] : leftmost_node;
last_node->inc_link_version();
parent_node->update(parent_node->total_entries() - 1, plast_key, last_node->link_info());
}
}
Expand Down Expand Up @@ -567,6 +568,28 @@ btree_status_t Btree< K, V >::merge_nodes(const BtreeNodePtr& parent_node, const
#ifndef NDEBUG
// BT_DBG_ASSERT(!parent_node_step1.empty() && !parent_node_step2.empty() && !parent_node_step3.empty(),
// "Empty string");
// check if the link version of parent for each key info match the link version of its child
BtreeLinkInfo child_info;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validate the link version of updated parent keys and that of its children match

if (ret == btree_status_t::success) {
for (uint32_t idx = 0; idx < new_nodes.size(); idx++) {
parent_node->get_nth_value(start_idx + 1 + idx, &child_info, false /* copy */);
BT_NODE_DBG_ASSERT_EQ(child_info.link_version(), new_nodes[idx]->link_version(), parent_node,
"mismatch of link version of new nodes in successful merge");
}
parent_node->get_nth_value(start_idx, &child_info, false /* copy */);
BT_NODE_DBG_ASSERT_EQ(child_info.link_version(), leftmost_node->link_version(), parent_node,
"parent_node, mismatch of link version of leftmost node in successful merge");
} else {
for (uint32_t idx = 0; idx < old_nodes.size(); idx++) {
parent_node->get_nth_value(start_idx + 1 + idx, &child_info, false /* copy */);
BT_NODE_DBG_ASSERT_EQ(child_info.link_version(), old_nodes[idx]->link_version(), parent_node,
"mismatch of link version of old nodes in unsuccessful merge");
}
parent_node->get_nth_value(start_idx, &child_info, false /* copy */);
BT_NODE_DBG_ASSERT_EQ(child_info.link_version(), leftmost_node->link_version(), parent_node,
"parent_node, mismatch of link version of leftmost node in unsuccessful merge");
}

if (leftmost_node->total_entries() && (start_idx < parent_node->total_entries())) {
BT_NODE_DBG_ASSERT_LE(
leftmost_node->get_last_key< K >().compare(parent_node->get_nth_key< K >(start_idx, false)), 0,
Expand Down
8 changes: 5 additions & 3 deletions src/include/homestore/btree/detail/simple_node.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,9 +264,9 @@ class SimpleNode : public BtreeNode {
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->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);
Expand All @@ -279,7 +279,9 @@ class SimpleNode : public BtreeNode {
fmt::format_to(std::back_inserter(str), " [");
for (uint32_t i{0}; i < this->total_entries(); ++i) {
uint32_t cur_key = get_nth_key< K >(i, false).key();
fmt::format_to(std::back_inserter(str), "{}{}", cur_key, i == this->total_entries() - 1 ? "" : ", ");
BtreeLinkInfo child_info;
get_nth_value(i, &child_info, false /* copy */);
fmt::format_to(std::back_inserter(str), "{}.{} {}", cur_key, child_info.link_version(), i == this->total_entries() - 1 ? "" : ", ");
}
fmt::format_to(std::back_inserter(str), "]");
return str;
Expand Down
8 changes: 5 additions & 3 deletions src/include/homestore/btree/detail/varlen_node.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,9 +520,9 @@ class VariableNode : public BtreeNode {
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->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);
Expand All @@ -535,7 +535,9 @@ class VariableNode : public BtreeNode {
fmt::format_to(std::back_inserter(str), " [");
for (uint32_t i{0}; i < this->total_entries(); ++i) {
uint32_t cur_key = get_nth_key< K >(i, false).key();
fmt::format_to(std::back_inserter(str), "{}{}", cur_key, i == this->total_entries() - 1 ? "" : ", ");
BtreeLinkInfo child_info;
get_nth_value(i, &child_info, false /* copy */);
fmt::format_to(std::back_inserter(str), "{}.{} {}", cur_key, child_info.link_version(), i == this->total_entries() - 1 ? "" : ", ");
}
fmt::format_to(std::back_inserter(str), "]");
return str;
Expand Down