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

Cap the maximum memory of the grenad sorters #4388

Merged
merged 2 commits into from
Feb 8, 2024
Merged

Conversation

Kerollmops
Copy link
Member

@Kerollmops Kerollmops commented Feb 6, 2024

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.

@Kerollmops Kerollmops added the performance Related to the performance in term of search/indexation speed or RAM/CPU/Disk consumption label Feb 6, 2024
@Kerollmops Kerollmops added this to the v1.7.0 milestone Feb 6, 2024
@Kerollmops
Copy link
Member Author

/benchmark indexing

@curquiza
Copy link
Member

curquiza commented Feb 6, 2024

@Kerollmops does this PR fix #4152?

@meili-bot
Copy link
Contributor

Here are your indexing benchmarks diff 👊

group                                                                     indexing_clamp-grenad-memory_47926514    indexing_main_47926514
-----                                                                     -------------------------------------    ----------------------
indexing/-geo-delete-facetedNumber-facetedGeo-searchable-                 1.13       7.4±0.49s        ? ?/sec      1.00       6.6±0.40s        ? ?/sec
indexing/-movies-delete-facetedString-facetedNumber-searchable-           1.15  1388.5±167.67ms        ? ?/sec     1.00  1202.9±90.58ms        ? ?/sec
indexing/-movies-delete-facetedString-facetedNumber-searchable-nested-    1.05       7.3±0.28s        ? ?/sec      1.00       6.9±0.19s        ? ?/sec
indexing/-songs-delete-facetedString-facetedNumber-searchable-            1.01      10.0±0.40s        ? ?/sec      1.00       9.9±0.15s        ? ?/sec
indexing/-wiki-delete-searchable-                                         1.91      46.1±4.40s        ? ?/sec      1.00      24.2±0.57s        ? ?/sec
indexing/Indexing geo_point                                               1.04      49.8±1.09s        ? ?/sec      1.00      48.1±0.50s        ? ?/sec
indexing/Indexing movies in three batches                                 1.05       3.8±0.27s        ? ?/sec      1.00       3.6±0.13s        ? ?/sec
indexing/Indexing movies with default settings                            1.08       3.8±0.24s        ? ?/sec      1.00       3.5±0.09s        ? ?/sec
indexing/Indexing nested movies with default settings                     1.14       5.3±0.47s        ? ?/sec      1.00       4.7±0.14s        ? ?/sec
indexing/Indexing nested movies without any facets                        1.07       4.4±0.31s        ? ?/sec      1.00       4.1±0.11s        ? ?/sec
indexing/Indexing songs in three batches with default settings            1.00      27.1±0.17s        ? ?/sec      1.00      27.1±0.46s        ? ?/sec
indexing/Indexing songs with default settings                             1.01      33.0±0.95s        ? ?/sec      1.00      32.6±0.99s        ? ?/sec
indexing/Indexing songs without any facets                                1.00      30.1±1.01s        ? ?/sec      1.01      30.2±0.94s        ? ?/sec
indexing/Indexing songs without faceted numbers                           1.00      31.9±0.80s        ? ?/sec      1.01      32.2±1.54s        ? ?/sec
indexing/Indexing wiki                                                    1.01     266.9±2.96s        ? ?/sec      1.00     264.5±2.98s        ? ?/sec
indexing/Indexing wiki in three batches                                   1.07     223.4±4.06s        ? ?/sec      1.00     207.9±1.34s        ? ?/sec
indexing/Reindexing geo_point                                             1.02      12.6±0.24s        ? ?/sec      1.00      12.3±0.10s        ? ?/sec
indexing/Reindexing movies with default settings                          1.01   224.3±35.96ms        ? ?/sec      1.00   223.1±32.43ms        ? ?/sec
indexing/Reindexing songs with default settings                           1.00       3.5±0.05s        ? ?/sec      1.01       3.5±0.03s        ? ?/sec
indexing/Reindexing wiki                                                  1.02     381.2±7.73s        ? ?/sec      1.00     374.0±3.89s        ? ?/sec

irevoire
irevoire previously approved these changes Feb 7, 2024
Copy link
Member

@irevoire irevoire left a 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

@Kerollmops
Copy link
Member Author

/benchmark indexing

@irevoire
Copy link
Member

irevoire commented Feb 7, 2024

IMO, considering the RAM we give to our big client, I would not be shocked if we went as up as 1GiB 👀

@Kerollmops Kerollmops changed the title Clamp the maximum memory of the grenad sorters Cap the maximum memory of the grenad sorters Feb 7, 2024
@meili-bot
Copy link
Contributor

Here are your indexing benchmarks diff 👊

group                                                                     indexing_clamp-grenad-memory_84235a63    indexing_main_47926514
-----                                                                     -------------------------------------    ----------------------
indexing/-geo-delete-facetedNumber-facetedGeo-searchable-                 1.12       7.4±0.72s        ? ?/sec      1.00       6.6±0.40s        ? ?/sec
indexing/-movies-delete-facetedString-facetedNumber-searchable-           1.12  1347.8±153.72ms        ? ?/sec     1.00  1202.9±90.58ms        ? ?/sec
indexing/-movies-delete-facetedString-facetedNumber-searchable-nested-    1.08       7.5±0.47s        ? ?/sec      1.00       6.9±0.19s        ? ?/sec
indexing/-songs-delete-facetedString-facetedNumber-searchable-            1.10      10.8±0.62s        ? ?/sec      1.00       9.9±0.15s        ? ?/sec
indexing/-wiki-delete-searchable-                                         2.28      55.2±7.51s        ? ?/sec      1.00      24.2±0.57s        ? ?/sec
indexing/Indexing geo_point                                               1.07      51.6±1.64s        ? ?/sec      1.00      48.1±0.50s        ? ?/sec
indexing/Indexing movies in three batches                                 1.06       3.8±0.18s        ? ?/sec      1.00       3.6±0.13s        ? ?/sec
indexing/Indexing movies with default settings                            1.11       3.9±0.27s        ? ?/sec      1.00       3.5±0.09s        ? ?/sec
indexing/Indexing nested movies with default settings                     1.13       5.3±0.49s        ? ?/sec      1.00       4.7±0.14s        ? ?/sec
indexing/Indexing nested movies without any facets                        1.13       4.6±0.46s        ? ?/sec      1.00       4.1±0.11s        ? ?/sec
indexing/Indexing songs in three batches with default settings            1.05      28.6±1.05s        ? ?/sec      1.00      27.1±0.46s        ? ?/sec
indexing/Indexing songs with default settings                             1.04      33.9±0.92s        ? ?/sec      1.00      32.6±0.99s        ? ?/sec
indexing/Indexing songs without any facets                                1.02      30.8±1.21s        ? ?/sec      1.00      30.2±0.94s        ? ?/sec
indexing/Indexing songs without faceted numbers                           1.05      33.8±2.02s        ? ?/sec      1.00      32.2±1.54s        ? ?/sec
indexing/Indexing wiki                                                    1.14    300.8±18.72s        ? ?/sec      1.00     264.5±2.98s        ? ?/sec
indexing/Indexing wiki in three batches                                   1.10     228.5±4.81s        ? ?/sec      1.00     207.9±1.34s        ? ?/sec
indexing/Reindexing geo_point                                             1.04      12.9±0.16s        ? ?/sec      1.00      12.3±0.10s        ? ?/sec
indexing/Reindexing movies with default settings                          1.02   227.8±37.00ms        ? ?/sec      1.00   223.1±32.43ms        ? ?/sec
indexing/Reindexing songs with default settings                           1.02       3.6±0.06s        ? ?/sec      1.00       3.5±0.03s        ? ?/sec
indexing/Reindexing wiki                                                  1.07    400.9±12.14s        ? ?/sec      1.00     374.0±3.89s        ? ?/sec

@Kerollmops
Copy link
Member Author

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

meili-bors bot added a commit that referenced this pull request Feb 8, 2024
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>
Copy link
Contributor

meili-bors bot commented Feb 8, 2024

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"}

Copy link
Member

@irevoire irevoire left a 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

meili-bors bot added a commit that referenced this pull request Feb 8, 2024
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>
Copy link
Contributor

meili-bors bot commented Feb 8, 2024

Build failed:

@curquiza
Copy link
Member

curquiza commented Feb 8, 2024

bors merge

Copy link
Contributor

meili-bors bot commented Feb 8, 2024

Build succeeded:

@meili-bors meili-bors bot merged commit 72ebac1 into main Feb 8, 2024
10 checks passed
@meili-bors meili-bors bot deleted the clamp-grenad-memory branch February 8, 2024 14:16
@meili-bot meili-bot added the v1.7.0 PRs/issues solved in v1.7.0 released on 2024-03-11 label Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Related to the performance in term of search/indexation speed or RAM/CPU/Disk consumption v1.7.0 PRs/issues solved in v1.7.0 released on 2024-03-11
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cap the size of the grenad Sorter to a defined value
4 participants