-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Cap the maximum memory of the grenad sorters #4388
Conversation
/benchmark indexing |
@Kerollmops does this PR fix #4152? |
Here are your indexing benchmarks diff 👊
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codewise it looks ok for me if we’re happy with the benchmarks
/benchmark indexing |
IMO, considering the RAM we give to our big client, I would not be shocked if we went as up as 1GiB 👀 |
Here are your indexing benchmarks diff 👊
|
Seems ok to me, as #4350 will probably change many things. This PR is here to reduce the memory crashes not to speed up the engine. We can work on improving the performances in a lot of other domains. bors merge |
4388: Cap the maximum memory of the grenad sorters r=Kerollmops a=Kerollmops This PR clamps the memory usage of the grenad sorters to a reasonable maximum. Grenad sorters are opened on multiple threads at a time. This can result in higher memory usage than expected, even though it shouldn't consume more than the memory available. Fixes #4152. Co-authored-by: Clément Renault <clement@meilisearch.com>
This PR was included in a batch that successfully built, but then failed to merge into main. It will not be retried. Additional information: {"message":"At least 1 approving review is required by reviewers with write access.","documentation_url":"https://docs.github.com/articles/about-protected-branches"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep definitely!
bors merge
4388: Cap the maximum memory of the grenad sorters r=irevoire a=Kerollmops This PR clamps the memory usage of the grenad sorters to a reasonable maximum. Grenad sorters are opened on multiple threads at a time. This can result in higher memory usage than expected, even though it shouldn't consume more than the memory available. Fixes #4152. Co-authored-by: Clément Renault <clement@meilisearch.com>
Build failed: |
bors merge |
Build succeeded:
|
This PR clamps the memory usage of the grenad sorters to a reasonable maximum. Grenad sorters are opened on multiple threads at a time. This can result in higher memory usage than expected, even though it shouldn't consume more than the memory available.
Fixes #4152.