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.

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

[master] < MG-parallelize-index-creation #882

Merged
merged 44 commits into from
Apr 26, 2023

Conversation

gvolfing
Copy link
Contributor

@gvolfing gvolfing commented Apr 16, 2023

[master < MG-parallelize-index-creation] PR

  • Check, and update documentation if necessary

János Benjamin Antal and others added 25 commits April 3, 2023 14:58
In the RecoverIndicesAndConstraints function it is now possible to
execute the index recration after a crash in a multithreaded fashion.
The issue was caused by the difference when trying to recover from
snapshots and WAL files. The parallelized version of index creation is
based on RecoveryInfo::vertex_batches which is always empty in the case
of recovering from WAL files. In that case the sequential index creation
will be used.
@gvolfing gvolfing self-assigned this Apr 16, 2023
@gvolfing gvolfing marked this pull request as ready for review April 19, 2023 11:15
src/memgraph.cpp Outdated Show resolved Hide resolved
src/storage/v2/indices.cpp Outdated Show resolved Hide resolved
gvolfing and others added 2 commits April 19, 2023 15:34
Co-authored-by: János Benjamin Antal <antaljanosbenjamin@users.noreply.github.com>
@gitbuda
Copy link
Member

gitbuda commented Apr 24, 2023

@gvolfing I think this should be disabled by default (an opinion). Another thing, I think the memgraph/config/flags.yml should be updated because of the package config file 😄

@gitbuda gitbuda mentioned this pull request Apr 24, 2023
8 tasks
Copy link
Collaborator

@antoniofilipovic antoniofilipovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we add tests here to check parallel index creation?

@gvolfing
Copy link
Contributor Author

Do we add tests here to check parallel index creation?
@antoniofilipovic
As we discussed on the daily the comprehensive testing of index creation will be done in a subsequent PR.

Copy link
Collaborator

@antoniofilipovic antoniofilipovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

Base automatically changed from MG-parallelize-recovery to master April 25, 2023 14:15
@antoniofilipovic antoniofilipovic merged commit 00f8d54 into master Apr 26, 2023
@antoniofilipovic antoniofilipovic deleted the MG-parallelize-index-creation branch April 26, 2023 14:28
@antoniofilipovic
Copy link
Collaborator

@vpavicic and @gvolfing I think you have to check about release note here

@vpavicic
Copy link
Contributor

vpavicic commented May 18, 2023

@antoniofilipovic & @gvolfing I have a release note in the docs PR, but the PR is still a draft, so is there more info coming?
memgraph/docs#833

Seems like the index page should be updated as well
https://memgraph.com/docs/memgraph/reference-guide/indexing

Maybe this has nothing to do with creating indexes outside of the recovery process? Can I use this for regular index creation?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants