Skip to content

Conversation

timvisee
Copy link
Member

@timvisee timvisee commented May 1, 2025

Fixes #6423 regression
Extension of #6326

While the condition change is not perfect, it does help significantly with the issue as described above.

This just prevents batching for a specific case that is known to perform bad: when having a high number of segments versus search threads.

All Submissions:

  • Contributions should target the dev branch. Did you create your branch from dev?
  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

  1. Does your submission pass tests?
  2. Have you formatted your code locally using cargo +nightly fmt --all command prior to submission?
  3. Have you checked your code using cargo clippy --all --all-features command?

@timvisee timvisee marked this pull request as ready for review May 1, 2025 10:20
@timvisee timvisee requested a review from generall May 1, 2025 10:20

This comment was marked as resolved.

@timvisee timvisee requested a review from Copilot May 1, 2025 10:32
Copilot

This comment was marked as resolved.

@agourlay
Copy link
Member

agourlay commented May 2, 2025

How did you validate this change?

@generall
Copy link
Member

generall commented May 4, 2025

Discovered another reason to not do mini-batching:

assume we have very large on-disk collection, and we want to do a full-scan over it.

It would be much faster if we go segment-by-segment, computing full batch of points right away. In this case we only need to load from disk once.

@generall generall merged commit 064fc24 into dev May 7, 2025
17 checks passed
@generall generall deleted the parallelize-large-segment-search-batches-limits branch May 7, 2025 10:57
generall pushed a commit that referenced this pull request May 22, 2025
* Don't batch query if there's more segments than CPUs, limit threads

* Don't batch query if there's more segments than search threads

* Link to pull request

* Add note that we only consider local segments and global search threads
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.

3 participants