diff --git a/src/storage/v2/disk/storage.cpp b/src/storage/v2/disk/storage.cpp index 6879b8711..f7808084d 100644 --- a/src/storage/v2/disk/storage.cpp +++ b/src/storage/v2/disk/storage.cpp @@ -86,15 +86,9 @@ bool VertexExistsInCache(const utils::SkipList::Accessor &accessor, Gid } bool VertexHasLabel(const Vertex &vertex, LabelId label, Transaction *transaction, View view) { - bool deleted = false; - bool has_label = false; - Delta *delta = nullptr; - { - std::lock_guard guard(vertex.lock); - deleted = vertex.deleted; - has_label = std::find(vertex.labels.begin(), vertex.labels.end(), label) != vertex.labels.end(); - delta = vertex.delta; - } + bool deleted = vertex.deleted; + bool has_label = std::find(vertex.labels.begin(), vertex.labels.end(), label) != vertex.labels.end(); + Delta *delta = vertex.delta; ApplyDeltasForRead(transaction, delta, view, [&deleted, &has_label, label](const Delta &delta) { switch (delta.action) { case Delta::Action::REMOVE_LABEL: { @@ -129,15 +123,9 @@ bool VertexHasLabel(const Vertex &vertex, LabelId label, Transaction *transactio } PropertyValue GetVertexProperty(const Vertex &vertex, PropertyId property, Transaction *transaction, View view) { - bool deleted = false; - PropertyValue value; - Delta *delta = nullptr; - { - std::lock_guard guard(vertex.lock); - deleted = vertex.deleted; - value = vertex.properties.GetProperty(property); - delta = vertex.delta; - } + bool deleted = vertex.deleted; + PropertyValue value = vertex.properties.GetProperty(property); + Delta *delta = vertex.delta; ApplyDeltasForRead(transaction, delta, view, [&deleted, &value, property](const Delta &delta) { switch (delta.action) { case Delta::Action::SET_PROPERTY: { @@ -857,15 +845,8 @@ std::optional DiskStorage::DiskAccessor::FindVertex(storage::Gid } Result> DiskStorage::DiskAccessor::DeleteVertex(VertexAccessor *vertex) { - MG_ASSERT(vertex->transaction_ == &transaction_, - "VertexAccessor must be from the same transaction as the storage " - "accessor when deleting a vertex!"); auto *vertex_ptr = vertex->vertex_; - std::lock_guard guard(vertex_ptr->lock); - - if (!PrepareForWrite(&transaction_, vertex_ptr)) return Error::SERIALIZATION_ERROR; - if (vertex_ptr->deleted) { return std::optional{}; } @@ -884,24 +865,12 @@ Result> DiskStorage::DiskAccessor::DeleteVertex(Ve Result>>> DiskStorage::DiskAccessor::DetachDeleteVertex(VertexAccessor *vertex) { using ReturnType = std::pair>; - MG_ASSERT(vertex->transaction_ == &transaction_, - "VertexAccessor must be from the same transaction as the storage " - "accessor when deleting a vertex!"); auto *vertex_ptr = vertex->vertex_; - std::vector> in_edges; - std::vector> out_edges; + if (vertex_ptr->deleted) return std::optional{}; - { - std::lock_guard guard(vertex_ptr->lock); - - if (!PrepareForWrite(&transaction_, vertex_ptr)) return Error::SERIALIZATION_ERROR; - - if (vertex_ptr->deleted) return std::optional{}; - - in_edges = vertex_ptr->in_edges; - out_edges = vertex_ptr->out_edges; - } + std::vector> in_edges{vertex_ptr->in_edges}; + std::vector> out_edges{vertex_ptr->out_edges}; std::vector deleted_edges; for (const auto &item : in_edges) { @@ -933,14 +902,6 @@ DiskStorage::DiskAccessor::DetachDeleteVertex(VertexAccessor *vertex) { } } - std::lock_guard guard(vertex_ptr->lock); - - // We need to check again for serialization errors because we unlocked the - // vertex. Some other transaction could have modified the vertex in the - // meantime if we didn't have any edges to delete. - - if (!PrepareForWrite(&transaction_, vertex_ptr)) return Error::SERIALIZATION_ERROR; - MG_ASSERT(!vertex_ptr->deleted, "Invalid database state!"); CreateAndLinkDelta(&transaction_, vertex_ptr, Delta::RecreateObjectTag()); @@ -1009,37 +970,10 @@ Result DiskStorage::DiskAccessor::CreateEdge(const VertexAccessor const std::string_view properties, const std::string &old_disk_key) { OOMExceptionEnabler oom_exception; - MG_ASSERT(from->transaction_ == to->transaction_, - "VertexAccessors must be from the same transaction when creating " - "an edge!"); - MG_ASSERT(from->transaction_ == &transaction_, - "VertexAccessors must be from the same transaction in when " - "creating an edge!"); - auto *from_vertex = from->vertex_; auto *to_vertex = to->vertex_; - // Obtain the locks by `gid` order to avoid lock cycles. - std::unique_lock guard_from(from_vertex->lock, std::defer_lock); - std::unique_lock guard_to(to_vertex->lock, std::defer_lock); - if (from_vertex->gid < to_vertex->gid) { - guard_from.lock(); - guard_to.lock(); - } else if (from_vertex->gid > to_vertex->gid) { - guard_to.lock(); - guard_from.lock(); - } else { - // The vertices are the same vertex, only lock one. - guard_from.lock(); - } - - if (!PrepareForWrite(&transaction_, from_vertex)) return Error::SERIALIZATION_ERROR; - if (from_vertex->deleted) return Error::DELETED_OBJECT; - - if (to_vertex != from_vertex) { - if (!PrepareForWrite(&transaction_, to_vertex)) return Error::SERIALIZATION_ERROR; - if (to_vertex->deleted) return Error::DELETED_OBJECT; - } + if (from_vertex->deleted || to_vertex->deleted) return Error::DELETED_OBJECT; auto *disk_storage = static_cast(storage_); disk_storage->edge_id_.store(std::max(disk_storage->edge_id_.load(std::memory_order_acquire), gid.AsUint() + 1), @@ -1073,37 +1007,10 @@ Result DiskStorage::DiskAccessor::CreateEdge(const VertexAccessor Result DiskStorage::DiskAccessor::CreateEdge(VertexAccessor *from, VertexAccessor *to, EdgeTypeId edge_type) { - MG_ASSERT(from->transaction_ == &transaction_, - "VertexAccessor must be from the same transaction as the storage " - "accessor when deleting a vertex!"); - MG_ASSERT(to->transaction_ == &transaction_, - "VertexAccessor must be from the same transaction as the storage " - "accessor when deleting a vertex!"); - auto *from_vertex = from->vertex_; auto *to_vertex = to->vertex_; - // Obtain the locks by `gid` order to avoid lock cycles. - std::unique_lock guard_from(from_vertex->lock, std::defer_lock); - std::unique_lock guard_to(to_vertex->lock, std::defer_lock); - if (from_vertex->gid < to_vertex->gid) { - guard_from.lock(); - guard_to.lock(); - } else if (from_vertex->gid > to_vertex->gid) { - guard_to.lock(); - guard_from.lock(); - } else { - // The vertices are the same vertex, only lock one. - guard_from.lock(); - } - - if (!PrepareForWrite(&transaction_, from_vertex)) return Error::SERIALIZATION_ERROR; - if (from_vertex->deleted) return Error::DELETED_OBJECT; - - if (to_vertex != from_vertex) { - if (!PrepareForWrite(&transaction_, to_vertex)) return Error::SERIALIZATION_ERROR; - if (to_vertex->deleted) return Error::DELETED_OBJECT; - } + if (from_vertex->deleted || to_vertex->deleted) return Error::DELETED_OBJECT; auto *disk_storage = static_cast(storage_); auto gid = storage::Gid::FromUint(disk_storage->edge_id_.fetch_add(1, std::memory_order_acq_rel)); @@ -1141,38 +1048,16 @@ Result> DiskStorage::DiskAccessor::DeleteEdge(EdgeAc const auto edge_ref = edge->edge_; const auto edge_type = edge->edge_type_; - std::unique_lock guard; if (config_.properties_on_edges) { - const auto *edge_ptr = edge_ref.ptr; - guard = std::unique_lock(edge_ptr->lock); - - if (!PrepareForWrite(&transaction_, edge_ptr)) return Error::SERIALIZATION_ERROR; - - if (edge_ptr->deleted) return std::optional{}; + if (edge_ref.ptr->deleted) return std::optional{}; } auto *from_vertex = edge->from_vertex_; auto *to_vertex = edge->to_vertex_; - // Obtain the locks by `gid` order to avoid lock cycles. - std::unique_lock guard_from(from_vertex->lock, std::defer_lock); - std::unique_lock guard_to(to_vertex->lock, std::defer_lock); - if (from_vertex->gid < to_vertex->gid) { - guard_from.lock(); - guard_to.lock(); - } else if (from_vertex->gid > to_vertex->gid) { - guard_to.lock(); - guard_from.lock(); - } else { - // The vertices are the same vertex, only lock one. - guard_from.lock(); - } - - if (!PrepareForWrite(&transaction_, from_vertex)) return Error::SERIALIZATION_ERROR; MG_ASSERT(!from_vertex->deleted, "Invalid database state!"); if (to_vertex != from_vertex) { - if (!PrepareForWrite(&transaction_, to_vertex)) return Error::SERIALIZATION_ERROR; MG_ASSERT(!to_vertex->deleted, "Invalid database state!"); }