Skip to content

Commit

Permalink
Speedup SearchContext.hasOnlySuggest check and related logic (elastic…
Browse files Browse the repository at this point in the history
…#112747)

Random find from looking at what we do on the transport thread for
search:
This check was quite heavy by building the list of sub-searches
needlessly. Also, we were needlessly allocating capturing lambdas
in the stats logic that used this call.
  • Loading branch information
original-brownbear authored Sep 12, 2024
1 parent f7a0196 commit c1f09d3
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -56,36 +56,28 @@ public SearchStats stats(String... groups) {

@Override
public void onPreQueryPhase(SearchContext searchContext) {
computeStats(searchContext, statsHolder -> {
if (searchContext.hasOnlySuggest()) {
statsHolder.suggestCurrent.inc();
} else {
statsHolder.queryCurrent.inc();
}
});
computeStats(
searchContext,
searchContext.hasOnlySuggest() ? statsHolder -> statsHolder.suggestCurrent.inc() : statsHolder -> statsHolder.queryCurrent.inc()
);
}

@Override
public void onFailedQueryPhase(SearchContext searchContext) {
computeStats(searchContext, statsHolder -> {
if (searchContext.hasOnlySuggest()) {
statsHolder.suggestCurrent.dec();
} else {
statsHolder.queryCurrent.dec();
}
});
computeStats(
searchContext,
searchContext.hasOnlySuggest() ? statsHolder -> statsHolder.suggestCurrent.dec() : statsHolder -> statsHolder.queryCurrent.dec()
);
}

@Override
public void onQueryPhase(SearchContext searchContext, long tookInNanos) {
computeStats(searchContext, statsHolder -> {
if (searchContext.hasOnlySuggest()) {
statsHolder.suggestMetric.inc(tookInNanos);
statsHolder.suggestCurrent.dec();
} else {
statsHolder.queryMetric.inc(tookInNanos);
statsHolder.queryCurrent.dec();
}
computeStats(searchContext, searchContext.hasOnlySuggest() ? statsHolder -> {
statsHolder.suggestMetric.inc(tookInNanos);
statsHolder.suggestCurrent.dec();
} : statsHolder -> {
statsHolder.queryMetric.inc(tookInNanos);
statsHolder.queryCurrent.dec();
});
}

Expand All @@ -109,8 +101,9 @@ public void onFetchPhase(SearchContext searchContext, long tookInNanos) {

private void computeStats(SearchContext searchContext, Consumer<StatsHolder> consumer) {
consumer.accept(totalStats);
if (searchContext.groupStats() != null) {
for (String group : searchContext.groupStats()) {
var groupStats = searchContext.groupStats();
if (groupStats != null) {
for (String group : groupStats) {
consumer.accept(groupStats(group));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1104,7 +1104,7 @@ public List<SearchExtBuilder> ext() {
* @return true if the source only has suggest
*/
public boolean isSuggestOnly() {
return suggestBuilder != null && query() == null && knnSearch.isEmpty() && aggregations == null;
return suggestBuilder != null && knnSearch.isEmpty() && aggregations == null && subSearchSourceBuilders.isEmpty();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,8 @@ public void addReleasable(Releasable releasable) { // TODO most Releasables ar
* @return true if the request contains only suggest
*/
public final boolean hasOnlySuggest() {
return request().source() != null && request().source().isSuggestOnly();
var source = request().source();
return source != null && source.isSuggestOnly();
}

/**
Expand Down

0 comments on commit c1f09d3

Please sign in to comment.