Skip to content

Clean up adjustSearchType method #96334

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

Merged
merged 2 commits into from
May 25, 2023
Merged

Clean up adjustSearchType method #96334

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9d16b63
move knn search to top of method in adjust
jdconrad May 24, 2023
edd4338
response to pr comments
jdconrad May 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,21 +403,23 @@ void executeRequest(
}

static void adjustSearchType(SearchRequest searchRequest, boolean singleShard) {
// optimize search type for cases where there is only one shard group to search on
if (singleShard) {
// if we only have one group, then we always want Q_T_F, no need for DFS, and no need to do THEN since we hit one shard
searchRequest.searchType(QUERY_THEN_FETCH);
// if there's a kNN search, always use DFS_QUERY_THEN_FETCH
if (searchRequest.hasKnnSearch()) {
searchRequest.searchType(DFS_QUERY_THEN_FETCH);
return;
}

// if there's only suggest, disable request cache and always use QUERY_THEN_FETCH
if (searchRequest.isSuggestOnly()) {
searchRequest.requestCache(false);
searchRequest.searchType(QUERY_THEN_FETCH);
return;
}

Copy link
Member

Choose a reason for hiding this comment

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

should we also move up the isSuggestOnly branch and return from it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! That also makes sense. I will make the change.

// if there's a kNN search, always use DFS_QUERY_THEN_FETCH
if (searchRequest.hasKnnSearch()) {
searchRequest.searchType(DFS_QUERY_THEN_FETCH);
// optimize search type for cases where there is only one shard group to search on
if (singleShard) {
// if we only have one group, then we always want Q_T_F, no need for DFS, and no need to do THEN since we hit one shard
searchRequest.searchType(QUERY_THEN_FETCH);
}
}

Expand Down