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.

Sign up for GitHub

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

Jump to bottom

Enable query phase parallelism by default #101230

Merged
merged 7 commits into from
Oct 26, 2023
Merged

Enable query phase parallelism by default #101230

merged 7 commits into from
Oct 26, 2023

Conversation

javanna
Copy link
Member

@javanna javanna commented Oct 23, 2023 •
edited
Loading

Inter-segment search concurrency is enabled only in the DFS phase so far, and affects knn searches. This commit enables it by default for the query phase too.

Note that terms aggregations currently disable parallelism. We are planning on removing this restriction soon.

Closes #80693

@javanna
Enable query phase parallelism by default
89879d3 
Inter-segment search concurrency is enabled only in the DFS phase so
far, and affects knn searches. This commit enables it by default for the
query phase too.
@javanna javanna added >enhancement release highlight :Search/Search Search-related issues that do not fall into other categories v8.12.0 labels Oct 23, 2023
@elasticsearchmachine elasticsearchmachine added the Team:Search Meta label for search team label Oct 23, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search (Team:Search)

@elasticsearchmachine
Copy link
Collaborator

Hi @javanna, I've created a changelog YAML for you. Note that since this PR is labelled release highlight, you need to update the changelog YAML to fill out the extended information sections.

@elasticsearchmachine
Copy link
Collaborator

Hi @javanna, I've updated the changelog YAML for you. Note that since this PR is labelled release highlight, you need to update the changelog YAML to fill out the extended information sections.

@javanna javanna requested a review from martijnvg October 24, 2023 09:03
martijnvg
martijnvg approved these changes Oct 24, 2023
View reviewed changes
Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

👍 lgtm

@javanna javanna merged commit d2f97e7 into elastic:main Oct 26, 2023
@javanna javanna deleted the enhancement/enable_query_phase_concurrency branch October 26, 2023 10:03
@LifeIsStrange
Copy link

Thanks for this @javanna ! Would be informative to see benchmarks similar to opensearch-project/OpenSearch#1500 (comment)

@przemekwitek przemekwitek mentioned this pull request Oct 30, 2023
Merged
javanna added a commit to javanna/elasticsearch that referenced this pull request Oct 30, 2023
@javanna
Disable inter-segment concurrency when sorting by field
ba201ec 
The way multiple slices execute when sorting by field is not optimized
and can lead to a slowdown. We will dig deeper and potentially re-enable
once we have addressed that.

When sorting by _doc or _shard_doc, and potentially by _timestamp, there
will be no gain with inter-segment concurrency.

Relates to elastic#101230
@javanna javanna mentioned this pull request Oct 30, 2023
Merged
javanna added a commit that referenced this pull request Oct 30, 2023
@javanna
Disable inter-segment concurrency when sorting by field (#101535)
999b2b6 
The way multiple slices execute when sorting by field is not optimized
and can lead to a slowdown. We will dig deeper and potentially re-enable
once we have addressed that.

When sorting by _doc or _shard_doc, and potentially by _timestamp, there
will be no gain with inter-segment concurrency.

Relates to #101230
@javanna
Copy link
Member Author

javanna commented Oct 30, 2023

Our nightly benchmarks results are publicly accessible. There's quite some different workloads, but many of them currently forge merge indices which makes search not parallelizable against them. An example of the effect of concurrency is here: https://elasticsearch-benchmarks.elastic.co/index.html#tracks/nyc-taxis/nightly/default/30d . You see it especially in the date histogram visualizations. Where there is no improvement it's because terms aggregation is used which currently does not support concurrency, but we are working on.

@scampi scampi mentioned this pull request Mar 4, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martijnvg martijnvg martijnvg approved these changes

Assignees
No one assigned
Labels
>enhancement release highlight :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team v8.12.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Concurrent Searching
4 participants
@javanna @elasticsearchmachine @LifeIsStrange @martijnvg