memgraph / mage

MAGE - Memgraph Advanced Graph Extensions :crystal_ball:
Apache License 2.0
240 stars 23 forks source link

Handle analytical mode in community_detection during parallel changes #400

Closed Darych closed 7 months ago

Darych commented 8 months ago

Description

NB: PR taken over by @antepusic; connected with https://github.com/memgraph/memgraph/pull/1395

Since there are no ACID guarantees in analytical storage mode, the graph may get modified by parallel transactions while the user’s query is running. Query procedures and functions that return graph elements (nodes and relationships) should be robust to the deletion of graph elements, and this PR aims to fix that.

This PR contains changes to query modules that ensure the intended behavior:

Merge commit message:

Add proper handling of deleted return values for query procedures and functions ran in analytical mode

Pull request type

Related issues

Resolves https://github.com/memgraph/memgraph/issues/1036 (together with https://github.com/memgraph/memgraph/pull/1395)

######################################

Reviewer checklist (the reviewer checks this part)

Module/Algorithm

######################################

as51340 commented 7 months ago

And you have build errors, please take care about that before requesting review again. For next time tip: it is better to request review after build errors are fixed since it gives reviewer more confidence that hey this is good and meaningful fix without negative impact on the rest of the system

Darych commented 7 months ago

My initial idea was to make body of GetInnerNodeIdOpt a default implementation for GetInnerNodeId, so we will have just one implementation. I mentioned about this HERE Currently. we have 2 implementations and users should think which one they need to use in their modules. @antoniofilipovic what do you think?

sonarcloud[bot] commented 7 months ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.7% 0.7% Duplication

antepusic commented 7 months ago

@vpavicic As this PR is the MAGE side of https://github.com/memgraph/memgraph/pull/1395/, they are covered by the same docs PR (https://github.com/memgraph/documentation/pull/361) and should both be linked in the same release note.