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

Implements the search cutoff #4466

Merged
merged 16 commits into from Mar 20, 2024
Merged

Implements the search cutoff #4466

merged 16 commits into from Mar 20, 2024

Conversation

irevoire
Copy link
Member

@irevoire irevoire commented Mar 7, 2024

Pull Request

Related issue

Fixes #4488

What does this PR do?

  • Adds a cutoff to the bucket sort after 150ms has been spent
  • Adds a new setting to customize the default value of 150ms
  • When the time is exceeded, we exit early with what we had the time to sort
  • If the cutoff has been reached, the search details are updated with a new Skip ranking details for the ranking rules that were skipped
  • Adds analytics to measure the total number of degraded search requests
  • Adds the number of degraded search requests to the Prometheus metrics and Grafana dashboard
  • The cutoff must not skip the filters; otherwise, we would leak documents to people who don’t have the right to see them

@irevoire irevoire added the performance Related to the performance in term of search/indexation speed or RAM/CPU/Disk consumption label Mar 7, 2024
@irevoire irevoire added this to the v1.8.0 milestone Mar 7, 2024
@irevoire irevoire added the search relevancy Related to the relevancy of the search results label Mar 19, 2024
@irevoire irevoire requested a review from dureuill March 19, 2024 09:24
@irevoire irevoire force-pushed the search-cutoff branch 2 times, most recently from 8195141 to d06b124 Compare March 19, 2024 09:32
@irevoire irevoire marked this pull request as ready for review March 19, 2024 10:15
Copy link
Contributor

@dureuill dureuill left a comment

Choose a reason for hiding this comment

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

Thank you for implementing this!

We need to change the score details

meilisearch/src/search.rs Outdated Show resolved Hide resolved
milli/src/score_details.rs Outdated Show resolved Hide resolved
milli/src/score_details.rs Outdated Show resolved Hide resolved
milli/src/search/mod.rs Outdated Show resolved Hide resolved
@@ -256,6 +261,13 @@ impl ScoreDetails {
details_map.insert(vector, details);
order += 1;
}
ScoreDetails::Skipped => {
details_map.insert(
Copy link
Contributor

Choose a reason for hiding this comment

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

This will need updating in the scoreDetails spec

milli/src/search/new/bucket_sort.rs Show resolved Hide resolved
milli/src/search/new/tests/cutoff.rs Show resolved Hide resolved
irevoire and others added 7 commits March 19, 2024 14:49
Co-authored-by: Louis Dureuil <louis@meilisearch.com>
handles the niche case 🐩 in the hybrid search where:
1. a sort ranking rule is the first rule.
2. the keyword search is skipped at the first rule.
3. the semantic search is not skipped at the first rule.

Previously, we would have the skipped search winning, whereas we want the non skipped one winning.
@irevoire irevoire requested a review from dureuill March 19, 2024 17:31
Copy link
Contributor

@dureuill dureuill left a comment

Choose a reason for hiding this comment

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

Looking good, and with good tests ☀️

bors merge

Copy link
Contributor

meili-bors bot commented Mar 19, 2024

Merge conflict.

@irevoire
Copy link
Member Author

bors merge

Copy link
Contributor

meili-bors bot commented Mar 20, 2024

@meili-bors meili-bors bot merged commit fc1c3f4 into main Mar 20, 2024
2 checks passed
@meili-bors meili-bors bot deleted the search-cutoff branch March 20, 2024 13:51
@meili-bot meili-bot added the v1.8.0 PRs/issues solved in v1.8.0 released on 2024-05-06 label May 6, 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 search relevancy Related to the relevancy of the search results v1.8.0 PRs/issues solved in v1.8.0 released on 2024-05-06
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement a search cutoff
3 participants