search

memgraph / mage

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

Community detection memory tracking #431

Closed imilinovic closed 4 months ago

imilinovic commented 7 months ago

Description

Community detection memory tracking was not working properly when multiple threads were used so it was fixed.

Add thread memory tracking to community detection.

Requires PR436 and PR1631 before merging

Note to reviewer (!!!): Review with suffix ?w=1 to ignore whitespaces changes which are automatically made by IDE (and there are a lot of them). Link for reviewing

Pull request type

Release note:

Community detection now respects memory limits and doesn't crash database.

Related issues

closes #379 closes #219 closes https://github.com/memgraph/memgraph/issues/955

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

Reviewer checklist (the reviewer checks this part)

Module/Algorithm

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

gitbuda commented 6 months ago

@imilinovic, quick question, what's the issue related? Can you link it? 👀

sonarcloud[bot] commented 4 months ago

Quality Gate Failed Quality Gate failed

Failed conditions
21.8% Duplication on New Code (required ≤ 3%)
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

idea Catch issues before they fail your Quality Gate with our IDE extension SonarLint SonarLint

imilinovic commented 4 months ago

I think the test to check that this works can't be done because of the following:

Memory limit is always being respected but the problem was that memory counting was wrong. The only way would be to compare it with the system memory (or see if the system crashes when setting the limit close to available RAM on the machine) which I don't think is possible at least not with the current MAGE testing infrastructure. @antepusic @DavIvek

I think sonarcloud can just be ignored here and merge if all other tests pass.