Skip to content

Commit 629c442

Browse files
Make SearchPhaseController.reducedQueryPhase a little cheaper (elastic#97848)
Saw this one in recent profiling sessions -> remove redundant multiple iterations of the results collection and cleanup slightly inefficient assertion usage.
1 parent a969b65 commit 629c442

File tree

1 file changed

+18
-10
lines changed

1 file changed

+18
-10
lines changed

server/src/main/java/org/elasticsearch/action/search/SearchPhaseController.java

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -541,15 +541,25 @@ static ReducedQueryPhase reducedQueryPhase(
541541
);
542542
}
543543
int total = queryResults.size();
544-
queryResults = queryResults.stream().filter(res -> res.queryResult().isNull() == false).toList();
545-
String errorMsg = "must have at least one non-empty search result, got 0 out of " + total;
546-
assert queryResults.isEmpty() == false : errorMsg;
547-
if (queryResults.isEmpty()) {
548-
throw new IllegalStateException(errorMsg);
544+
final Collection<SearchPhaseResult> nonNullResults = new ArrayList<>();
545+
boolean hasSuggest = false;
546+
boolean hasProfileResults = false;
547+
for (SearchPhaseResult queryResult : queryResults) {
548+
var res = queryResult.queryResult();
549+
if (res.isNull()) {
550+
continue;
551+
}
552+
hasSuggest |= res.suggest() != null;
553+
hasProfileResults |= res.hasProfileResults();
554+
nonNullResults.add(queryResult);
549555
}
556+
queryResults = nonNullResults;
550557
validateMergeSortValueFormats(queryResults);
551-
final boolean hasSuggest = queryResults.stream().anyMatch(res -> res.queryResult().suggest() != null);
552-
final boolean hasProfileResults = queryResults.stream().anyMatch(res -> res.queryResult().hasProfileResults());
558+
if (queryResults.isEmpty()) {
559+
var ex = new IllegalStateException("must have at least one non-empty search result, got 0 out of " + total);
560+
assert false : ex;
561+
throw ex;
562+
}
553563

554564
// count the total (we use the query result provider here, since we might not get any hits (we scrolled past them))
555565
final Map<String, List<Suggestion<?>>> groupedSuggestions = hasSuggest ? new HashMap<>() : Collections.emptyMap();
@@ -578,9 +588,7 @@ static ReducedQueryPhase reducedQueryPhase(
578588
}
579589
}
580590
}
581-
if (bufferedTopDocs.isEmpty() == false) {
582-
assert result.hasConsumedTopDocs() : "firstResult has no aggs but we got non null buffered aggs?";
583-
}
591+
assert bufferedTopDocs.isEmpty() || result.hasConsumedTopDocs() : "firstResult has no aggs but we got non null buffered aggs?";
584592
if (hasProfileResults) {
585593
String key = result.getSearchShardTarget().toString();
586594
profileShardResults.put(key, result.consumeProfileResult());

0 commit comments

Comments
 (0)