From 66376fea0ecfee56638bf4fcb81cfc3eb85b78ee Mon Sep 17 00:00:00 2001 From: Andreja Tonev Date: Tue, 12 Mar 2024 14:12:12 +0100 Subject: [PATCH] WIP: Added PDS to Vertex --- .../v2/constraints/existence_constraints.cpp | 4 +- src/storage/v2/disk/label_index.cpp | 15 +- src/storage/v2/disk/label_property_index.cpp | 20 +-- src/storage/v2/disk/storage.cpp | 49 ++++--- src/storage/v2/disk/unique_constraints.cpp | 13 +- src/storage/v2/durability/snapshot.cpp | 6 +- src/storage/v2/durability/wal.cpp | 4 +- src/storage/v2/edge.hpp | 40 ++++-- src/storage/v2/indices/indices_utils.hpp | 6 +- .../v2/inmemory/label_property_index.cpp | 2 +- src/storage/v2/inmemory/storage.cpp | 6 +- .../v2/inmemory/unique_constraints.cpp | 14 +- src/storage/v2/property_disk_store.hpp | 14 +- src/storage/v2/vertex.hpp | 135 +++++++++++++++++- src/storage/v2/vertex_accessor.cpp | 22 +-- tests/unit/pds.cpp | 18 +-- tests/unit/storage_v2_wal_file.cpp | 7 +- 17 files changed, 270 insertions(+), 105 deletions(-) diff --git a/src/storage/v2/constraints/existence_constraints.cpp b/src/storage/v2/constraints/existence_constraints.cpp index 956e0a208..3ace64144 100644 --- a/src/storage/v2/constraints/existence_constraints.cpp +++ b/src/storage/v2/constraints/existence_constraints.cpp @@ -1,4 +1,4 @@ -// Copyright 2023 Memgraph Ltd. +// Copyright 2024 Memgraph Ltd. // // Use of this software is governed by the Business Source License // included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source @@ -58,7 +58,7 @@ void ExistenceConstraints::LoadExistenceConstraints(const std::vector ExistenceConstraints::ValidateVertexOnConstraint( const Vertex &vertex, const LabelId &label, const PropertyId &property) { - if (!vertex.deleted && utils::Contains(vertex.labels, label) && !vertex.properties.HasProperty(property)) { + if (!vertex.deleted && utils::Contains(vertex.labels, label) && !vertex.HasProperty(property)) { return ConstraintViolation{ConstraintViolation::Type::EXISTENCE, label, std::set{property}}; } return std::nullopt; diff --git a/src/storage/v2/disk/label_index.cpp b/src/storage/v2/disk/label_index.cpp index 56986079b..d9942de0b 100644 --- a/src/storage/v2/disk/label_index.cpp +++ b/src/storage/v2/disk/label_index.cpp @@ -1,4 +1,4 @@ -// Copyright 2023 Memgraph Ltd. +// Copyright 2024 Memgraph Ltd. // // Use of this software is governed by the Business Source License // included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source @@ -90,12 +90,13 @@ bool DiskLabelIndex::SyncVertexToLabelIndexStorage(const Vertex &vertex, uint64_ if (!utils::Contains(vertex.labels, index_label)) { continue; } - if (!disk_transaction - ->Put(utils::SerializeVertexAsKeyForLabelIndex(index_label, vertex.gid), - utils::SerializeVertexAsValueForLabelIndex(index_label, vertex.labels, vertex.properties)) - .ok()) { - return false; - } + // TODO: Re-enable + // if (!disk_transaction + // ->Put(utils::SerializeVertexAsKeyForLabelIndex(index_label, vertex.gid), + // utils::SerializeVertexAsValueForLabelIndex(index_label, vertex.labels, vertex.properties)) + // .ok()) { + // return false; + // } } return CommitWithTimestamp(disk_transaction.get(), commit_timestamp); diff --git a/src/storage/v2/disk/label_property_index.cpp b/src/storage/v2/disk/label_property_index.cpp index 9a40f03d1..e4b1674c4 100644 --- a/src/storage/v2/disk/label_property_index.cpp +++ b/src/storage/v2/disk/label_property_index.cpp @@ -1,4 +1,4 @@ -// Copyright 2023 Memgraph Ltd. +// Copyright 2024 Memgraph Ltd. // // Use of this software is governed by the Business Source License // included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source @@ -20,7 +20,9 @@ namespace memgraph::storage { namespace { bool IsVertexIndexedByLabelProperty(const Vertex &vertex, LabelId label, PropertyId property) { - return utils::Contains(vertex.labels, label) && vertex.properties.HasProperty(property); + // TODO: Re-enable + // return utils::Contains(vertex.labels, label) && vertex.properties.HasProperty(property); + return false; } [[nodiscard]] bool ClearTransactionEntriesWithRemovedIndexingLabel( @@ -94,12 +96,14 @@ bool DiskLabelPropertyIndex::SyncVertexToLabelPropertyIndexStorage(const Vertex } for (const auto &[index_label, index_property] : index_) { if (IsVertexIndexedByLabelProperty(vertex, index_label, index_property)) { - if (!disk_transaction - ->Put(utils::SerializeVertexAsKeyForLabelPropertyIndex(index_label, index_property, vertex.gid), - utils::SerializeVertexAsValueForLabelPropertyIndex(index_label, vertex.labels, vertex.properties)) - .ok()) { - return false; - } + // TODO: Re-enable + // if (!disk_transaction + // ->Put(utils::SerializeVertexAsKeyForLabelPropertyIndex(index_label, index_property, vertex.gid), + // utils::SerializeVertexAsValueForLabelPropertyIndex(index_label, vertex.labels, + // vertex.properties)) + // .ok()) { + // return false; + // } } } return CommitWithTimestamp(disk_transaction.get(), commit_timestamp); diff --git a/src/storage/v2/disk/storage.cpp b/src/storage/v2/disk/storage.cpp index 4d9ea821f..85b0b58c1 100644 --- a/src/storage/v2/disk/storage.cpp +++ b/src/storage/v2/disk/storage.cpp @@ -172,7 +172,7 @@ bool VertexHasLabel(const Vertex &vertex, LabelId label, Transaction *transactio PropertyValue GetVertexProperty(const Vertex &vertex, PropertyId property, Transaction *transaction, View view) { bool deleted = vertex.deleted; - PropertyValue value = vertex.properties.GetProperty(property); + PropertyValue value = vertex.GetProperty(property); Delta *delta = vertex.delta; ApplyDeltasForRead(transaction, delta, view, [&deleted, &value, property](const Delta &delta) { switch (delta.action) { @@ -628,10 +628,11 @@ std::unordered_set DiskStorage::MergeVerticesFromMainCacheWithLabelIndexCac spdlog::trace("Loaded vertex with gid: {} from main index storage to label index", vertex.gid.ToString()); uint64_t ts = utils::GetEarliestTimestamp(vertex.delta); /// TODO: here are doing serialization and then later deserialization again -> expensive - LoadVertexToLabelIndexCache(transaction, utils::SerializeVertexAsKeyForLabelIndex(label, vertex.gid), - utils::SerializeVertexAsValueForLabelIndex(label, vertex.labels, vertex.properties), - CreateDeleteDeserializedIndexObjectDelta(index_deltas, std::nullopt, ts), - indexed_vertices->access()); + // TODO: Re-enable + // LoadVertexToLabelIndexCache(transaction, utils::SerializeVertexAsKeyForLabelIndex(label, vertex.gid), + // utils::SerializeVertexAsValueForLabelIndex(label, vertex.labels, + // vertex.properties), CreateDeleteDeserializedIndexObjectDelta(index_deltas, + // std::nullopt, ts), indexed_vertices->access()); } } return gids; @@ -678,10 +679,11 @@ std::unordered_set DiskStorage::MergeVerticesFromMainCacheWithLabelProperty gids.insert(vertex.gid); if (label_property_filter(vertex, label, property, view)) { uint64_t ts = utils::GetEarliestTimestamp(vertex.delta); - LoadVertexToLabelPropertyIndexCache( - transaction, utils::SerializeVertexAsKeyForLabelPropertyIndex(label, property, vertex.gid), - utils::SerializeVertexAsValueForLabelPropertyIndex(label, vertex.labels, vertex.properties), - CreateDeleteDeserializedIndexObjectDelta(index_deltas, std::nullopt, ts), indexed_vertices->access()); + // TODO: Re-enable + // LoadVertexToLabelPropertyIndexCache( + // transaction, utils::SerializeVertexAsKeyForLabelPropertyIndex(label, property, vertex.gid), + // utils::SerializeVertexAsValueForLabelPropertyIndex(label, vertex.labels, vertex.properties), + // CreateDeleteDeserializedIndexObjectDelta(index_deltas, std::nullopt, ts), indexed_vertices->access()); } } @@ -763,10 +765,11 @@ std::unordered_set DiskStorage::MergeVerticesFromMainCacheWithLabelProperty if (VertexHasLabel(vertex, label, transaction, view) && IsPropertyValueWithinInterval(prop_value, lower_bound, upper_bound)) { uint64_t ts = utils::GetEarliestTimestamp(vertex.delta); - LoadVertexToLabelPropertyIndexCache( - transaction, utils::SerializeVertexAsKeyForLabelPropertyIndex(label, property, vertex.gid), - utils::SerializeVertexAsValueForLabelPropertyIndex(label, vertex.labels, vertex.properties), - CreateDeleteDeserializedIndexObjectDelta(index_deltas, std::nullopt, ts), indexed_vertices->access()); + // TODO: Re-enable + // LoadVertexToLabelPropertyIndexCache( + // transaction, utils::SerializeVertexAsKeyForLabelPropertyIndex(label, property, vertex.gid), + // utils::SerializeVertexAsValueForLabelPropertyIndex(label, vertex.labels, vertex.properties), + // CreateDeleteDeserializedIndexObjectDelta(index_deltas, std::nullopt, ts), indexed_vertices->access()); } } return gids; @@ -1028,14 +1031,15 @@ bool DiskStorage::WriteVertexToVertexColumnFamily(Transaction *transaction, cons MG_ASSERT(transaction->commit_timestamp, "Writing vertex to disk but commit timestamp not set."); auto commit_ts = transaction->commit_timestamp->load(std::memory_order_relaxed); const auto ser_vertex = utils::SerializeVertex(vertex); - auto status = transaction->disk_transaction_->Put(kvstore_->vertex_chandle, ser_vertex, - utils::SerializeProperties(vertex.properties)); - if (status.ok()) { - spdlog::trace("rocksdb: Saved vertex with key {} and ts {} to vertex column family", ser_vertex, commit_ts); - return true; - } - spdlog::error("rocksdb: Failed to save vertex with key {} and ts {} to vertex column family", ser_vertex, commit_ts); - return false; + // TODO: Re-enable + // auto status = transaction->disk_transaction_->Put(kvstore_->vertex_chandle, ser_vertex, + // utils::SerializeProperties(vertex.properties)); + // if (status.ok()) { + // spdlog::trace("rocksdb: Saved vertex with key {} and ts {} to vertex column family", ser_vertex, commit_ts); + return true; + // } + // spdlog::error("rocksdb: Failed to save vertex with key {} and ts {} to vertex column family", ser_vertex, + // commit_ts); return false; } bool DiskStorage::WriteEdgeToEdgeColumnFamily(Transaction *transaction, const std::string &serialized_edge_key, @@ -1358,7 +1362,8 @@ VertexAccessor DiskStorage::CreateVertexFromDisk(Transaction *transaction, utils MG_ASSERT(inserted, "The vertex must be inserted here!"); MG_ASSERT(it != accessor.end(), "Invalid Vertex accessor!"); it->labels = std::move(label_ids); - it->properties = std::move(properties); + // TODO: Re-enable + // it->properties = std::move(properties); delta->prev.Set(&*it); return {&*it, this, transaction}; } diff --git a/src/storage/v2/disk/unique_constraints.cpp b/src/storage/v2/disk/unique_constraints.cpp index 04a0c265a..4d96dfc7a 100644 --- a/src/storage/v2/disk/unique_constraints.cpp +++ b/src/storage/v2/disk/unique_constraints.cpp @@ -28,7 +28,7 @@ namespace { bool IsVertexUnderConstraint(const Vertex &vertex, const LabelId &constraint_label, const std::set &constraint_properties) { - return utils::Contains(vertex.labels, constraint_label) && vertex.properties.HasAllProperties(constraint_properties); + return utils::Contains(vertex.labels, constraint_label) && vertex.HasAllProperties(constraint_properties); } bool IsDifferentVertexWithSameConstraintLabel(const std::string &key, const Gid gid, const LabelId constraint_label) { @@ -105,7 +105,7 @@ std::optional DiskUniqueConstraints::Validate( std::optional DiskUniqueConstraints::TestIfVertexSatisifiesUniqueConstraint( const Vertex &vertex, std::vector> &unique_storage, const LabelId &constraint_label, const std::set &constraint_properties) const { - auto property_values = vertex.properties.ExtractPropertyValues(constraint_properties); + auto property_values = vertex.ExtractPropertyValues(constraint_properties); /// TODO: better naming. Is vertex unique if (property_values.has_value() && @@ -227,10 +227,11 @@ bool DiskUniqueConstraints::SyncVertexToUniqueConstraintsStorage(const Vertex &v if (IsVertexUnderConstraint(vertex, constraint_label, constraint_properties)) { auto key = utils::SerializeVertexAsKeyForUniqueConstraint(constraint_label, constraint_properties, vertex.gid.ToString()); - auto value = utils::SerializeVertexAsValueForUniqueConstraint(constraint_label, vertex.labels, vertex.properties); - if (!disk_transaction->Put(key, value).ok()) { - return false; - } + // TODO: Re-enable + // auto value = utils::SerializeVertexAsValueForUniqueConstraint(constraint_label, vertex.labels, + // vertex.properties); if (!disk_transaction->Put(key, value).ok()) { + // return false; + // } } } /// TODO: extract and better message diff --git a/src/storage/v2/durability/snapshot.cpp b/src/storage/v2/durability/snapshot.cpp index f48fdc927..ac50360e6 100644 --- a/src/storage/v2/durability/snapshot.cpp +++ b/src/storage/v2/durability/snapshot.cpp @@ -369,7 +369,6 @@ uint64_t LoadPartialVertices(const std::filesystem::path &path, utils::SkipList< { auto props_size = snapshot.ReadUint(); if (!props_size) throw RecoveryFailure("Couldn't read size of vertex properties!"); - auto &props = it->properties; read_properties.clear(); read_properties.reserve(*props_size); for (uint64_t j = 0; j < *props_size; ++j) { @@ -379,7 +378,7 @@ uint64_t LoadPartialVertices(const std::filesystem::path &path, utils::SkipList< if (!value) throw RecoveryFailure("Couldn't read vertex property value!"); read_properties.emplace_back(get_property_from_id(*key), std::move(*value)); } - props.InitProperties(std::move(read_properties)); + it->InitProperties(std::move(read_properties)); } // Skip in edges. @@ -794,7 +793,6 @@ RecoveredSnapshot LoadSnapshotVersion14(const std::filesystem::path &path, utils { auto props_size = snapshot.ReadUint(); if (!props_size) throw RecoveryFailure("Couldn't read the size of properties!"); - auto &props = it->properties; for (uint64_t j = 0; j < *props_size; ++j) { auto key = snapshot.ReadUint(); if (!key) throw RecoveryFailure("Couldn't read the vertex property id!"); @@ -802,7 +800,7 @@ RecoveredSnapshot LoadSnapshotVersion14(const std::filesystem::path &path, utils if (!value) throw RecoveryFailure("Couldn't read the vertex property value!"); SPDLOG_TRACE("Recovered property \"{}\" with value \"{}\" for vertex {}.", name_id_mapper->IdToName(snapshot_id_map.at(*key)), *value, *gid); - props.SetProperty(get_property_from_id(*key), *value); + it->SetProperty(get_property_from_id(*key), *value); } } diff --git a/src/storage/v2/durability/wal.cpp b/src/storage/v2/durability/wal.cpp index 69ed6b10d..1a63c5297 100644 --- a/src/storage/v2/durability/wal.cpp +++ b/src/storage/v2/durability/wal.cpp @@ -597,7 +597,7 @@ void EncodeDelta(BaseEncoder *encoder, NameIdMapper *name_id_mapper, SalientConf // TODO (mferencevic): Mitigate the memory allocation introduced here // (with the `GetProperty` call). It is the only memory allocation in the // entire WAL file writing logic. - encoder->WritePropertyValue(vertex.properties.GetProperty(delta.property.key)); + encoder->WritePropertyValue(vertex.GetProperty(delta.property.key)); break; } case Delta::Action::ADD_LABEL: @@ -842,7 +842,7 @@ RecoveryInfo LoadWal(const std::filesystem::path &path, RecoveredIndicesAndConst auto property_id = PropertyId::FromUint(name_id_mapper->NameToId(delta.vertex_edge_set_property.property)); auto &property_value = delta.vertex_edge_set_property.value; - vertex->properties.SetProperty(property_id, property_value); + vertex->SetProperty(property_id, property_value); break; } diff --git a/src/storage/v2/edge.hpp b/src/storage/v2/edge.hpp index a1842e83b..96237cc24 100644 --- a/src/storage/v2/edge.hpp +++ b/src/storage/v2/edge.hpp @@ -36,9 +36,14 @@ struct Edge { ~Edge() { // TODO: Don't want to do this here - ClearProperties(); + if (!moved) ClearProperties(); } + Edge(Edge &) = delete; + Edge &operator=(Edge &) = delete; + Edge(Edge &&) = default; + Edge &operator=(Edge &&) = delete; + Gid gid; // PropertyStore properties; @@ -47,21 +52,40 @@ struct Edge { bool deleted; // uint8_t PAD; // uint16_t PAD; + class HotFixMove { + public: + HotFixMove() {} + HotFixMove(HotFixMove &&other) noexcept { + if (this != &other) { + // We want only the latest object to be marked as not-moved; while all previous should be marked as moved + moved = false; + other.moved = true; + } + } + HotFixMove(HotFixMove &) = delete; + HotFixMove &operator=(HotFixMove &) = delete; + HotFixMove &operator=(HotFixMove &&) = delete; + + operator bool() const { return moved; } + + private: + bool moved{false}; + } moved; Delta *delta; - // PSAPI Properties() { PDS::get(); } + Gid HotFixForGID() const { return Gid::FromUint(gid.AsUint() + (1 << 31)); } PropertyValue GetProperty(PropertyId property) const { if (deleted) return {}; - const auto prop = PDS::get()->Get(gid, property); + const auto prop = PDS::get()->Get(HotFixForGID(), property); if (prop) return *prop; return {}; } bool SetProperty(PropertyId property, const PropertyValue &value) { if (deleted) return {}; - return PDS::get()->Set(gid, property, value); + return PDS::get()->Set(HotFixForGID(), property, value); } template @@ -72,7 +96,7 @@ struct Edge { if (value.IsNull()) { continue; } - if (!pds->Set(gid, property, value)) { + if (!pds->Set(HotFixForGID(), property, value)) { return false; } } @@ -81,12 +105,12 @@ struct Edge { void ClearProperties() { auto *pds = PDS::get(); - pds->Clear(gid); + pds->Clear(HotFixForGID()); } std::map Properties() { if (deleted) return {}; - return PDS::get()->Get(gid); + return PDS::get()->Get(HotFixForGID()); } std::vector> UpdateProperties( @@ -117,7 +141,7 @@ struct Edge { uint64_t PropertySize(PropertyId property) const { if (deleted) return {}; - return PDS::get()->GetSize(gid, property); + return PDS::get()->GetSize(HotFixForGID(), property); } }; diff --git a/src/storage/v2/indices/indices_utils.hpp b/src/storage/v2/indices/indices_utils.hpp index 52938a1db..c5c82b32d 100644 --- a/src/storage/v2/indices/indices_utils.hpp +++ b/src/storage/v2/indices/indices_utils.hpp @@ -121,7 +121,7 @@ inline bool AnyVersionHasLabelProperty(const Vertex &vertex, LabelId label, Prop has_label = utils::Contains(vertex.labels, label); // Avoid IsPropertyEqual if already not possible if (delta == nullptr && (deleted || !has_label)) return false; - current_value_equal_to_value = vertex.properties.IsPropertyEqual(key, value); + current_value_equal_to_value = vertex.IsPropertyEqual(key, value); } if (!deleted && has_label && current_value_equal_to_value) { @@ -186,7 +186,7 @@ inline bool CurrentVersionHasLabelProperty(const Vertex &vertex, LabelId label, auto guard = std::shared_lock{vertex.lock}; deleted = vertex.deleted; has_label = utils::Contains(vertex.labels, label); - current_value_equal_to_value = vertex.properties.IsPropertyEqual(key, value); + current_value_equal_to_value = vertex.IsPropertyEqual(key, value); delta = vertex.delta; } @@ -246,7 +246,7 @@ inline void TryInsertLabelPropertyIndex(Vertex &vertex, std::pairproperties.GetProperty(label_prop.second); + auto prop_value = vertex_after_update->GetProperty(label_prop.second); if (!prop_value.IsNull()) { auto acc = storage.access(); acc.insert(Entry{std::move(prop_value), vertex_after_update, tx.start_timestamp}); diff --git a/src/storage/v2/inmemory/storage.cpp b/src/storage/v2/inmemory/storage.cpp index bfcff9467..dbd7d77c0 100644 --- a/src/storage/v2/inmemory/storage.cpp +++ b/src/storage/v2/inmemory/storage.cpp @@ -1044,7 +1044,7 @@ void InMemoryStorage::InMemoryAccessor::Abort() { const auto &properties = index_stats.property_label.l2p.find(current->label.value); if (properties != index_stats.property_label.l2p.end()) { for (const auto &property : properties->second) { - auto current_value = vertex->properties.GetProperty(property); + auto current_value = vertex->GetProperty(property); if (!current_value.IsNull()) { label_property_cleanup[current->label.value].emplace_back(std::move(current_value), vertex); } @@ -1065,13 +1065,13 @@ void InMemoryStorage::InMemoryAccessor::Abort() { // value const auto &labels = index_stats.property_label.p2l.find(current->property.key); if (labels != index_stats.property_label.p2l.end()) { - auto current_value = vertex->properties.GetProperty(current->property.key); + auto current_value = vertex->GetProperty(current->property.key); if (!current_value.IsNull()) { property_cleanup[current->property.key].emplace_back(std::move(current_value), vertex); } } // Setting the correct value - vertex->properties.SetProperty(current->property.key, *current->property.value); + vertex->SetProperty(current->property.key, *current->property.value); break; } case Delta::Action::ADD_IN_EDGE: { diff --git a/src/storage/v2/inmemory/unique_constraints.cpp b/src/storage/v2/inmemory/unique_constraints.cpp index dd47a3f68..47f074336 100644 --- a/src/storage/v2/inmemory/unique_constraints.cpp +++ b/src/storage/v2/inmemory/unique_constraints.cpp @@ -64,7 +64,7 @@ bool LastCommittedVersionHasLabelProperty(const Vertex &vertex, LabelId label, c size_t i = 0; for (const auto &property : properties) { - current_value_equal_to_value[i] = vertex.properties.IsPropertyEqual(property, value_array[i]); + current_value_equal_to_value[i] = vertex.IsPropertyEqual(property, value_array[i]); property_array.values[i] = property; i++; } @@ -155,7 +155,7 @@ bool AnyVersionHasLabelProperty(const Vertex &vertex, LabelId label, const std:: // If delta we need to fetch for later processing size_t i = 0; for (const auto &property : properties) { - current_value_equal_to_value[i] = vertex.properties.IsPropertyEqual(property, values[i]); + current_value_equal_to_value[i] = vertex.IsPropertyEqual(property, values[i]); property_array.values[i] = property; i++; } @@ -163,7 +163,7 @@ bool AnyVersionHasLabelProperty(const Vertex &vertex, LabelId label, const std:: // otherwise do a short-circuiting check (we already know !deleted && has_label) size_t i = 0; for (const auto &property : properties) { - if (!vertex.properties.IsPropertyEqual(property, values[i])) return false; + if (!vertex.IsPropertyEqual(property, values[i])) return false; i++; } return true; @@ -269,7 +269,7 @@ void InMemoryUniqueConstraints::UpdateBeforeCommit(const Vertex *vertex, const T } for (auto &[props, storage] : constraint->second) { - auto values = vertex->properties.ExtractPropertyValues(props); + auto values = vertex->ExtractPropertyValues(props); if (!values) { continue; @@ -334,7 +334,7 @@ std::optional InMemoryUniqueConstraints::DoValidate( if (vertex.deleted || !utils::Contains(vertex.labels, label)) { return std::nullopt; } - auto values = vertex.properties.ExtractPropertyValues(properties); + auto values = vertex.ExtractPropertyValues(properties); if (!values) { return std::nullopt; } @@ -359,7 +359,7 @@ void InMemoryUniqueConstraints::AbortEntries(std::span vert } for (auto &[props, storage] : constraint->second) { - auto values = vertex->properties.ExtractPropertyValues(props); + auto values = vertex->ExtractPropertyValues(props); if (!values) { continue; @@ -451,7 +451,7 @@ std::optional InMemoryUniqueConstraints::Validate(const Ver } for (const auto &[properties, storage] : constraint->second) { - auto value_array = vertex.properties.ExtractPropertyValues(properties); + auto value_array = vertex.ExtractPropertyValues(properties); if (!value_array) { continue; diff --git a/src/storage/v2/property_disk_store.hpp b/src/storage/v2/property_disk_store.hpp index 1b8244105..15c4d203d 100644 --- a/src/storage/v2/property_disk_store.hpp +++ b/src/storage/v2/property_disk_store.hpp @@ -41,39 +41,39 @@ class PDS { return ptr_; } - std::string ToKey(Gid gid, PropertyId pid) { + static std::string ToKey(Gid gid, PropertyId pid) { std::string key(sizeof(gid) + sizeof(pid), '\0'); memcpy(key.data(), &gid, sizeof(gid)); memcpy(&key[sizeof(gid)], &pid, sizeof(pid)); return key; } - std::string ToPrefix(Gid gid) { + static std::string ToPrefix(Gid gid) { std::string key(sizeof(gid), '\0'); memcpy(key.data(), &gid, sizeof(gid)); return key; } - Gid ToGid(std::string_view sv) { + static Gid ToGid(std::string_view sv) { uint64_t gid; gid = *((uint64_t *)sv.data()); return Gid::FromUint(gid); } - PropertyId ToPid(std::string_view sv) { + static PropertyId ToPid(std::string_view sv) { uint32_t pid; pid = *((uint32_t *)&sv[sizeof(Gid)]); return PropertyId::FromUint(pid); } - PropertyValue ToPV(std::string_view sv) { + static PropertyValue ToPV(std::string_view sv) { PropertyValue pv; slk::Reader reader((const uint8_t *)sv.data(), sv.size()); slk::Load(&pv, &reader); return pv; } - std::string ToStr(const PropertyValue &pv) { + static std::string ToStr(const PropertyValue &pv) { std::string val{}; slk::Builder builder([&val](const uint8_t *data, size_t size, bool /*have_more*/) { const auto old_size = val.size(); @@ -121,6 +121,8 @@ class PDS { void Clear(Gid gid) { kvstore_.DeletePrefix(ToPrefix(gid)); } + bool Has(Gid gid, PropertyId pid) { return kvstore_.Size(ToKey(gid, pid)) != 0; } + // kvstore::KVStore::iterator Itr() {} private: diff --git a/src/storage/v2/vertex.hpp b/src/storage/v2/vertex.hpp index 41021b436..c18945992 100644 --- a/src/storage/v2/vertex.hpp +++ b/src/storage/v2/vertex.hpp @@ -1,4 +1,4 @@ -// Copyright 2023 Memgraph Ltd. +// Copyright 2024 Memgraph Ltd. // // Use of this software is governed by the Business Source License // included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source @@ -12,6 +12,7 @@ #pragma once #include +#include #include #include @@ -21,6 +22,8 @@ #include "storage/v2/property_store.hpp" #include "utils/rw_spin_lock.hpp" +#include "storage/v2/property_disk_store.hpp" + namespace memgraph::storage { struct Vertex { @@ -30,10 +33,20 @@ struct Vertex { "Vertex must be created with an initial DELETE_OBJECT delta!"); } + ~Vertex() { + // TODO: Move to another place <- this will get called twice if moved... + if (!moved) ClearProperties(); + } + + Vertex(Vertex &) = delete; + Vertex &operator=(Vertex &) = delete; + Vertex(Vertex &&) noexcept = default; + Vertex &operator=(Vertex &&) = delete; + const Gid gid; std::vector labels; - PropertyStore properties; + // PropertyStore properties; std::vector> in_edges; std::vector> out_edges; @@ -43,7 +56,125 @@ struct Vertex { // uint8_t PAD; // uint16_t PAD; + class HotFixMove { + public: + HotFixMove() {} + HotFixMove(HotFixMove &&other) noexcept { + if (this != &other) { + // We want only the latest object to be marked as not-moved; while all previous should be marked as moved + moved = false; + other.moved = true; + } + } + HotFixMove(HotFixMove &) = delete; + HotFixMove &operator=(HotFixMove &) = delete; + HotFixMove &operator=(HotFixMove &&) = delete; + + operator bool() const { return moved; } + + private: + bool moved{false}; + } moved; + Delta *delta; + + PropertyValue GetProperty(PropertyId property) const { + // if (deleted) return {}; + const auto prop = PDS::get()->Get(gid, property); + if (prop) return *prop; + return {}; + } + + bool SetProperty(PropertyId property, const PropertyValue &value) { + // if (deleted) return {}; + return PDS::get()->Set(gid, property, value); + } + + bool HasProperty(PropertyId property) const { + // if (deleted) return {}; + return PDS::get()->Has(gid, property); + } + + bool HasAllProperties(const std::set &properties) const { + // if (deleted) return {}; + return std::all_of(properties.begin(), properties.end(), [this](const auto &prop) { return HasProperty(prop); }); + } + + bool IsPropertyEqual(PropertyId property, const PropertyValue &value) const { + // if (deleted) return {}; + const auto val = GetProperty(property); + return val == value; + } + + template + bool InitProperties(const TContainer &properties) { + // if (deleted) return {}; + auto *pds = PDS::get(); + for (const auto &[property, value] : properties) { + if (value.IsNull()) { + continue; + } + if (!pds->Set(gid, property, value)) { + return false; + } + } + return true; + } + + void ClearProperties() { + auto *pds = PDS::get(); + pds->Clear(gid); + } + + std::map Properties() { + // if (deleted) return {}; + return PDS::get()->Get(gid); + } + + std::vector> UpdateProperties( + std::map &properties) { + // if (deleted) return {}; + auto old_properties = Properties(); + ClearProperties(); + + std::vector> id_old_new_change; + id_old_new_change.reserve(properties.size() + old_properties.size()); + for (const auto &[prop_id, new_value] : properties) { + if (!old_properties.contains(prop_id)) { + id_old_new_change.emplace_back(prop_id, PropertyValue(), new_value); + } + } + + for (const auto &[old_key, old_value] : old_properties) { + auto [it, inserted] = properties.emplace(old_key, old_value); + if (!inserted) { + auto &new_value = it->second; + id_old_new_change.emplace_back(it->first, old_value, new_value); + } + } + + MG_ASSERT(InitProperties(properties)); + return id_old_new_change; + } + + uint64_t PropertySize(PropertyId property) const { + // if (deleted) return {}; + return PDS::get()->GetSize(gid, property); + } + + std::optional> ExtractPropertyValues(const std::set &properties) const { + // if (deleted) return {}; + std::vector value_array; + value_array.reserve(properties.size()); + for (const auto &prop : properties) { + auto value = GetProperty(prop); + if (value.IsNull()) { + return std::nullopt; + } + value_array.emplace_back(std::move(value)); + } + return value_array; + } }; static_assert(alignof(Vertex) >= 8, "The Vertex should be aligned to at least 8!"); diff --git a/src/storage/v2/vertex_accessor.cpp b/src/storage/v2/vertex_accessor.cpp index 7d78070a8..ee3856ff7 100644 --- a/src/storage/v2/vertex_accessor.cpp +++ b/src/storage/v2/vertex_accessor.cpp @@ -261,7 +261,7 @@ Result VertexAccessor::SetProperty(PropertyId property, const Pro if (vertex_->deleted) return Error::DELETED_OBJECT; - auto current_value = vertex_->properties.GetProperty(property); + auto current_value = vertex_->GetProperty(property); // We could skip setting the value if the previous one is the same to the new // one. This would save some memory as a delta would not be created as well as // avoid copying the value. The reason we are not doing that is because the @@ -272,7 +272,7 @@ Result VertexAccessor::SetProperty(PropertyId property, const Pro utils::AtomicMemoryBlock atomic_memory_block{ [transaction = transaction_, vertex = vertex_, &value, &property, ¤t_value]() { CreateAndLinkDelta(transaction, vertex, Delta::SetPropertyTag(), property, current_value); - vertex->properties.SetProperty(property, value); + vertex->SetProperty(property, value); }}; std::invoke(atomic_memory_block); @@ -303,7 +303,7 @@ Result VertexAccessor::InitProperties(const std::mapproperties.InitProperties(properties)) { + if (!vertex->InitProperties(properties)) { result = false; return; } @@ -339,11 +339,11 @@ Result>> Vertex if (vertex_->deleted) return Error::DELETED_OBJECT; - using ReturnType = decltype(vertex_->properties.UpdateProperties(properties)); + using ReturnType = decltype(vertex_->UpdateProperties(properties)); std::optional id_old_new_change; utils::AtomicMemoryBlock atomic_memory_block{ [storage = storage_, transaction = transaction_, vertex = vertex_, &properties, &id_old_new_change]() { - id_old_new_change.emplace(vertex->properties.UpdateProperties(properties)); + id_old_new_change.emplace(vertex->UpdateProperties(properties)); if (!id_old_new_change.has_value()) { return; } @@ -375,11 +375,11 @@ Result> VertexAccessor::ClearProperties() { if (vertex_->deleted) return Error::DELETED_OBJECT; - using ReturnType = decltype(vertex_->properties.Properties()); + using ReturnType = decltype(vertex_->Properties()); std::optional properties; utils::AtomicMemoryBlock atomic_memory_block{ [storage = storage_, transaction = transaction_, vertex = vertex_, &properties]() { - properties.emplace(vertex->properties.Properties()); + properties.emplace(vertex->Properties()); if (!properties.has_value()) { return; } @@ -391,7 +391,7 @@ Result> VertexAccessor::ClearProperties() { if (transaction->constraint_verification_info) { transaction->constraint_verification_info->RemovedProperty(vertex); } - vertex->properties.ClearProperties(); + vertex->ClearProperties(); }}; std::invoke(atomic_memory_block); @@ -406,7 +406,7 @@ Result VertexAccessor::GetProperty(PropertyId property, View view { auto guard = std::shared_lock{vertex_->lock}; deleted = vertex_->deleted; - value = vertex_->properties.GetProperty(property); + value = vertex_->GetProperty(property); delta = vertex_->delta; } @@ -451,7 +451,7 @@ Result VertexAccessor::GetPropertySize(PropertyId property, View view) auto guard = std::shared_lock{vertex_->lock}; Delta *delta = vertex_->delta; if (!delta) { - return vertex_->properties.PropertySize(property); + return vertex_->PropertySize(property); } } @@ -474,7 +474,7 @@ Result> VertexAccessor::Properties(View view { auto guard = std::shared_lock{vertex_->lock}; deleted = vertex_->deleted; - properties = vertex_->properties.Properties(); + properties = vertex_->Properties(); delta = vertex_->delta; } diff --git a/tests/unit/pds.cpp b/tests/unit/pds.cpp index 346800aa4..66d809a39 100644 --- a/tests/unit/pds.cpp +++ b/tests/unit/pds.cpp @@ -314,15 +314,15 @@ TEST_F(PdsTest, BasicUsage) { TEST_F(PdsTest, Get) { using namespace memgraph::storage; auto *pds = PDS::get(); - pds->Set(Gid::FromInt(0), PropertyId::FromInt(1), PropertyValue{"test1"}); - pds->Set(Gid::FromInt(0), PropertyId::FromInt(2), PropertyValue{"test2"}); - pds->Set(Gid::FromInt(0), PropertyId::FromInt(3), PropertyValue{"test3"}); - pds->Set(Gid::FromInt(1), PropertyId::FromInt(0), PropertyValue{"test0"}); - pds->Set(Gid::FromInt(1), PropertyId::FromInt(2), PropertyValue{"test02"}); + pds->Set(Gid::FromUint(0), PropertyId::FromUint(1), PropertyValue{"test1"}); + pds->Set(Gid::FromUint(0), PropertyId::FromUint(2), PropertyValue{"test2"}); + pds->Set(Gid::FromUint(0), PropertyId::FromUint(3), PropertyValue{"test3"}); + pds->Set(Gid::FromUint(1), PropertyId::FromUint(0), PropertyValue{"test0"}); + pds->Set(Gid::FromUint(1), PropertyId::FromUint(2), PropertyValue{"test02"}); - auto all_0 = pds->Get(Gid::FromInt(0)); + auto all_0 = pds->Get(Gid::FromUint(0)); ASSERT_EQ(all_0.size(), 3); - ASSERT_EQ(all_0[PropertyId::FromInt(1)], PropertyValue{"test1"}); - ASSERT_EQ(all_0[PropertyId::FromInt(2)], PropertyValue{"test2"}); - ASSERT_EQ(all_0[PropertyId::FromInt(3)], PropertyValue{"test3"}); + ASSERT_EQ(all_0[PropertyId::FromUint(1)], PropertyValue{"test1"}); + ASSERT_EQ(all_0[PropertyId::FromUint(2)], PropertyValue{"test2"}); + ASSERT_EQ(all_0[PropertyId::FromUint(3)], PropertyValue{"test3"}); } diff --git a/tests/unit/storage_v2_wal_file.cpp b/tests/unit/storage_v2_wal_file.cpp index 4094090f5..ad078424a 100644 --- a/tests/unit/storage_v2_wal_file.cpp +++ b/tests/unit/storage_v2_wal_file.cpp @@ -137,11 +137,10 @@ class DeltaGenerator final { void SetProperty(memgraph::storage::Vertex *vertex, const std::string &property, const memgraph::storage::PropertyValue &value) { auto property_id = memgraph::storage::PropertyId::FromUint(gen_->mapper_.NameToId(property)); - auto &props = vertex->properties; - auto old_value = props.GetProperty(property_id); + auto old_value = vertex->GetProperty(property_id); memgraph::storage::CreateAndLinkDelta(&transaction_, &*vertex, memgraph::storage::Delta::SetPropertyTag(), property_id, old_value); - props.SetProperty(property_id, value); + vertex->SetProperty(property_id, value); if (transaction_.storage_mode == memgraph::storage::StorageMode::IN_MEMORY_ANALYTICAL) return; { memgraph::storage::durability::WalDeltaData data; @@ -185,7 +184,7 @@ class DeltaGenerator final { ASSERT_NE(vertex, gen_->vertices_.end()); auto property_id = memgraph::storage::PropertyId::FromUint( gen_->mapper_.NameToId(data.vertex_edge_set_property.property)); - data.vertex_edge_set_property.value = vertex->properties.GetProperty(property_id); + data.vertex_edge_set_property.value = vertex->GetProperty(property_id); } gen_->data_.emplace_back(commit_timestamp, data); }