From 97cab6650b0be34807b294be5b8fe733486232c5 Mon Sep 17 00:00:00 2001
From: jbajic <jure.bajic@memgraph.com>
Date: Tue, 31 Jan 2023 15:21:39 +0100
Subject: [PATCH] Remove redundant index update
---
src/storage/v3/delta.hpp | 2 +-
src/storage/v3/indices.hpp | 13 ++++-------
src/storage/v3/splitter.cpp | 32 ++-------------------------
tests/unit/storage_v3_shard_split.cpp | 22 +++++++++---------
4 files changed, 18 insertions(+), 51 deletions(-)
diff --git a/src/storage/v3/delta.hpp b/src/storage/v3/delta.hpp
index 95bd5d18a..7b8916f6d 100644
--- a/src/storage/v3/delta.hpp
+++ b/src/storage/v3/delta.hpp
@@ -31,7 +31,7 @@ struct CommitInfo;
inline uint64_t GetNextDeltaUUID() noexcept {
static utils::Synchronized<uint64_t, utils::SpinLock> delta_id{0};
- return delta_id.WithLock([](auto id) { return id++; });
+ return delta_id.WithLock([](auto &id) { return id++; });
}
// This class stores one of three pointers (`Delta`, `Vertex` and `Edge`)
diff --git a/src/storage/v3/indices.hpp b/src/storage/v3/indices.hpp
index 56555c31e..99023183b 100644
--- a/src/storage/v3/indices.hpp
+++ b/src/storage/v3/indices.hpp
@@ -122,9 +122,7 @@ class LabelIndex {
void Clear() { index_.clear(); }
- std::map<IndexType, IndexContainer> SplitIndexEntries(
- const PrimaryKey &split_key,
- std::map<IndexType, std::multimap<const Vertex *, const IndexContainer::iterator>> &vertex_entry_map) {
+ std::map<IndexType, IndexContainer> SplitIndexEntries(const PrimaryKey &split_key) {
if (index_.empty()) {
return {};
}
@@ -144,7 +142,7 @@ class LabelIndex {
cloned_indices_container.insert(index.extract(entry_it));
MG_ASSERT(inserted, "Failed to extract index entry!");
- vertex_entry_map[index_type_val].insert({inserted_entry_it->vertex, inserted_entry_it});
+ // vertex_entry_map[index_type_val].insert({inserted_entry_it->vertex, inserted_entry_it});
}
entry_it = next_entry_it;
}
@@ -269,9 +267,7 @@ class LabelPropertyIndex {
void Clear() { index_.clear(); }
- std::map<IndexType, IndexContainer> SplitIndexEntries(
- const PrimaryKey &split_key,
- std::map<IndexType, std::multimap<const Vertex *, const IndexContainer::iterator>> &vertex_entry_map) {
+ std::map<IndexType, IndexContainer> SplitIndexEntries(const PrimaryKey &split_key) {
if (index_.empty()) {
return {};
}
@@ -290,8 +286,7 @@ class LabelPropertyIndex {
[[maybe_unused]] const auto &[inserted_entry_it, inserted, node] =
cloned_index_container.insert(index.extract(entry_it));
MG_ASSERT(inserted, "Failed to extract index entry!");
-
- vertex_entry_map[index_type_val].insert({inserted_entry_it->vertex, inserted_entry_it});
+ // vertex_entry_map[index_type_val].insert({inserted_entry_it->vertex, inserted_entry_it});
}
entry_it = next_entry_it;
}
diff --git a/src/storage/v3/splitter.cpp b/src/storage/v3/splitter.cpp
index 3fb865ee8..28b8c3a83 100644
--- a/src/storage/v3/splitter.cpp
+++ b/src/storage/v3/splitter.cpp
@@ -69,31 +69,8 @@ void Splitter::ScanDeltas(std::set<uint64_t> &collected_transactions_, Delta *de
VertexContainer Splitter::CollectVertices(SplitData &data, std::set<uint64_t> &collected_transactions_,
const PrimaryKey &split_key) {
- // Collection of indices is here since it heavily depends on vertices
- // Old vertex pointer new entry pointer
- std::map<LabelId, std::multimap<const Vertex *, const LabelIndex::IndexContainer::iterator>>
- label_index_vertex_entry_map;
- std::map<std::pair<LabelId, PropertyId>,
- std::multimap<const Vertex *, const LabelPropertyIndex::IndexContainer::iterator>>
- label_property_vertex_entry_map;
-
- data.label_indices = indices_.label_index.SplitIndexEntries(split_key, label_index_vertex_entry_map);
- data.label_property_indices =
- indices_.label_property_index.SplitIndexEntries(split_key, label_property_vertex_entry_map);
- // This is needed to replace old vertex pointers in index entries with new ones
- const auto update_indices = [](auto &entry_vertex_map, auto &updating_index, const auto *old_vertex_ptr,
- auto &new_vertex_it) {
- for ([[maybe_unused]] auto &[index_type, vertex_entry_mappings] : entry_vertex_map) {
- auto [it, end] = vertex_entry_mappings.equal_range(old_vertex_ptr);
- while (it != end) {
- auto entry_to_update = *it->second;
- entry_to_update.vertex = &*new_vertex_it;
- updating_index.at(index_type).erase(it->second);
- updating_index.at(index_type).insert(std::move(entry_to_update));
- ++it;
- }
- }
- };
+ data.label_indices = indices_.label_index.SplitIndexEntries(split_key);
+ data.label_property_indices = indices_.label_property_index.SplitIndexEntries(split_key);
VertexContainer splitted_data;
auto split_key_it = vertices_.find(split_key);
@@ -101,16 +78,11 @@ VertexContainer Splitter::CollectVertices(SplitData &data, std::set<uint64_t> &c
// Go through deltas and pick up transactions start_id/commit_id
ScanDeltas(collected_transactions_, split_key_it->second.delta);
- const auto *old_vertex_ptr = &*split_key_it;
auto next_it = std::next(split_key_it);
const auto &[splitted_vertex_it, inserted, node] = splitted_data.insert(vertices_.extract(split_key_it->first));
MG_ASSERT(inserted, "Failed to extract vertex!");
- // Update indices
- update_indices(label_index_vertex_entry_map, data.label_indices, old_vertex_ptr, splitted_vertex_it);
- update_indices(label_property_vertex_entry_map, data.label_property_indices, old_vertex_ptr, splitted_vertex_it);
-
split_key_it = next_it;
}
return splitted_data;
diff --git a/tests/unit/storage_v3_shard_split.cpp b/tests/unit/storage_v3_shard_split.cpp
index 3d5954002..7e7a917fe 100644
--- a/tests/unit/storage_v3_shard_split.cpp
+++ b/tests/unit/storage_v3_shard_split.cpp
@@ -147,11 +147,11 @@ TEST_F(ShardSplitTest, TestBasicSplitWithVertices) {
EXPECT_EQ(splitted_data.label_property_indices.size(), 0);
CommitInfo commit_info{.start_or_commit_timestamp = current_hlc};
- Delta delta_delete1{Delta::DeleteObjectTag{}, &commit_info, 5, 1};
- Delta delta_delete2{Delta::DeleteObjectTag{}, &commit_info, 6, 2};
- Delta delta_remove_label{Delta::RemoveLabelTag{}, secondary_label, &commit_info, 8, 4};
- Delta delta_set_property{Delta::SetPropertyTag{}, secondary_property, PropertyValue(), &commit_info, 7, 4};
- Delta delta_delete3{Delta::DeleteObjectTag{}, &commit_info, 9, 3};
+ Delta delta_delete1{Delta::DeleteObjectTag{}, &commit_info, 4, 1};
+ Delta delta_delete2{Delta::DeleteObjectTag{}, &commit_info, 5, 2};
+ Delta delta_remove_label{Delta::RemoveLabelTag{}, secondary_label, &commit_info, 7, 4};
+ Delta delta_set_property{Delta::SetPropertyTag{}, secondary_property, PropertyValue(), &commit_info, 6, 4};
+ Delta delta_delete3{Delta::DeleteObjectTag{}, &commit_info, 8, 3};
VertexContainer expected_vertices;
expected_vertices.emplace(PrimaryKey{PropertyValue{4}}, VertexData(&delta_delete1));
@@ -191,29 +191,29 @@ TEST_F(ShardSplitTest, TestBasicSplitVerticesAndEdges) {
EXPECT_EQ(splitted_data.label_property_indices.size(), 0);
CommitInfo commit_info{.start_or_commit_timestamp = current_hlc};
- Delta delta_delete1{Delta::DeleteObjectTag{}, &commit_info, 13, 1};
- Delta delta_delete2{Delta::DeleteObjectTag{}, &commit_info, 14, 1};
- Delta delta_delete3{Delta::DeleteObjectTag{}, &commit_info, 15, 1};
+ Delta delta_delete1{Delta::DeleteObjectTag{}, &commit_info, 12, 1};
+ Delta delta_delete2{Delta::DeleteObjectTag{}, &commit_info, 13, 1};
+ Delta delta_delete3{Delta::DeleteObjectTag{}, &commit_info, 14, 1};
Delta delta_add_in_edge1{Delta::RemoveInEdgeTag{},
edge_type_id,
VertexId{primary_label, {PropertyValue(1)}},
EdgeRef{Gid::FromUint(1)},
&commit_info,
- 18,
+ 17,
1};
Delta delta_add_out_edge2{Delta::RemoveOutEdgeTag{},
edge_type_id,
VertexId{primary_label, {PropertyValue(6)}},
EdgeRef{Gid::FromUint(2)},
&commit_info,
- 20,
+ 19,
1};
Delta delta_add_in_edge2{Delta::RemoveInEdgeTag{},
edge_type_id,
VertexId{primary_label, {PropertyValue(4)}},
EdgeRef{Gid::FromUint(2)},
&commit_info,
- 21,
+ 20,
1};
VertexContainer expected_vertices;
auto [vtx4, inserted4] = expected_vertices.emplace(PrimaryKey{PropertyValue{4}}, VertexData(&delta_delete1));