Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve removal of obsolete indexes #2043

Merged
merged 22 commits into from
May 22, 2024
Merged

Improve removal of obsolete indexes #2043

merged 22 commits into from
May 22, 2024

Conversation

DavIvek
Copy link
Contributor

@DavIvek DavIvek commented May 15, 2024 •
edited

Description

While discarding deltas and removing vertices from indexes, idea is to not go trough all indexes but just trough the ones we are deleting.

[master < Task] PR

  • Provide the full content or a guide for the final git message
    • [FINAL GIT MESSAGE]

CI Testing Labels

Please select the appropriate CI test labels (CI -build=build-name -test=test-suite)

Documentation checklist

  • Add the documentation label tag
  • Add the bug / feature label tag
  • Add the milestone for which this feature is intended
    • If not known, set for a later milestone
  • Write a release note, including added/changed clauses
    • Faster vertex deletion in cases with numerous index entries
  • Link the documentation PR here
    • [Documentation PR link]
  • Tag someone from docs team in the comments

cc @kgolubic

closes #1955

@DavIvek DavIvek added Docs - changelog only Docs - changelog only customer customer labels May 15, 2024
@DavIvek DavIvek self-assigned this May 15, 2024
@DavIvek DavIvek marked this pull request as draft May 15, 2024 13:12
@DavIvek DavIvek added CI -build=community -test=core Run community build and core tests on push CI -build=coverage -test=core Run coverage build and core tests on push CI -build=debug -test=core Run debug build and core tests on push CI -build=release -test=core Run release build and core tests on push CI -build=release -test=e2e Run release build and e2e tests on push CI -build=release -test=benchmark Run release build and benchmark on push labels May 16, 2024
@DavIvek
Copy link
Contributor Author

DavIvek commented May 16, 2024 •
edited

Flamegraph I got after this changes + the query is executing much faster now (exact numbers depends on the dataset size)
Screenshot from 2024-05-16 13-20-58

@DavIvek DavIvek marked this pull request as ready for review May 16, 2024 14:39
@DavIvek
Copy link
Contributor Author

DavIvek commented May 16, 2024

An idea to consider, does it make sense to do something similar in GarbageCollect function?

@Ignition
Copy link
Contributor

Ignition commented May 20, 2024 •
edited

Problem I think:

CREATE (n) SET n:L1 REMOVE n:L1 SET n:L2 DETACH DELETE n;

Can you add an e2e test that does this followed by a scan that will use the L1 index?
Maybe need to force gc on the vertices skip_list to actually delete the removed vertex.

@DavIvek DavIvek marked this pull request as draft May 21, 2024 08:35
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented May 21, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@DavIvek DavIvek marked this pull request as ready for review May 21, 2024 18:18
@DavIvek DavIvek requested a review from Ignition May 21, 2024 18:18
@DavIvek DavIvek changed the title Fix fast discard of deltas Improve removing obsolete indexes May 22, 2024
@DavIvek DavIvek added this to the mg-v2.18.0 milestone May 22, 2024
@DavIvek DavIvek changed the title Improve removing obsolete indexes Improve removal of obsolete indexes May 22, 2024
@DavIvek DavIvek added this pull request to the merge queue May 22, 2024
Merged via the queue into master with commit b9a209e May 22, 2024
11 checks passed
@DavIvek DavIvek deleted the fix-fast-discard-of-deltas branch May 22, 2024 10:52
@kgolubic
Copy link
Contributor

@DavIvek is this OK for RN:

Improved the removal process of obsolete indexes. When discarding deltas and removing vertices from indexes, Memgraph now targets only the relevant indexes instead of processing all indexes, resulting in faster vertex deletion in cases with numerous index entries.
#2043

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ignition Ignition Ignition approved these changes

@Josipmrden Josipmrden Awaiting requested review from Josipmrden

Assignees

@DavIvek DavIvek

Labels
CI -build=community -test=core Run community build and core tests on push CI -build=coverage -test=core Run coverage build and core tests on push CI -build=debug -test=core Run debug build and core tests on push CI -build=release -test=benchmark Run release build and benchmark on push CI -build=release -test=core Run release build and core tests on push CI -build=release -test=e2e Run release build and e2e tests on push customer customer Docs - changelog only Docs - changelog only
Projects
None yet
Milestone
mg-v2.18.0
Development

Successfully merging this pull request may close these issues.

FastDiscardOfDeltas scanning all nodes and properties on delete
3 participants
@DavIvek @Ignition @kgolubic