Skip to content

Commit d75f666

Browse files
Reduce latency in fetch phase for large shard counts (#120419)
We can reduce the latency of the fetch response by a non-trivial amount by moving the context-freeing for irrelevant shards to the end of the forked action. For large shard counts (and/or with security in the mix) the old comment is not entirely correct and waking up the selector many times over + doing some authz work might consume macroscopic time. Moving the freeing to the end of the task causes the fetches to be sent out potentially much quicker, reduces contention on the counter and reduces the impact of potential head-of-line blocking issues on the connections that might see context freeing needlessly queue after actual fetches.
1 parent 0a5d2e7 commit d75f666

File tree

1 file changed

+14
-13
lines changed

1 file changed

+14
-13
lines changed

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

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -158,30 +158,31 @@ private void innerRunFetch(ScoreDoc[] scoreDocs, int numShards, SearchPhaseContr
158158
);
159159
for (int i = 0; i < docIdsToLoad.length; i++) {
160160
List<Integer> entry = docIdsToLoad[i];
161-
RankDocShardInfo rankDocs = rankDocsPerShard == null || rankDocsPerShard.get(i).isEmpty()
162-
? null
163-
: new RankDocShardInfo(rankDocsPerShard.get(i));
164-
SearchPhaseResult shardPhaseResult = searchPhaseShardResults.get(i);
165161
if (entry == null) { // no results for this shard ID
166-
if (shardPhaseResult != null) {
167-
// if we got some hits from this shard we have to release the context there
168-
// we do this as we go since it will free up resources and passing on the request on the
169-
// transport layer is cheap.
170-
releaseIrrelevantSearchContext(shardPhaseResult, context);
171-
progressListener.notifyFetchResult(i);
172-
}
162+
// if we got some hits from this shard we have to release the context
163+
// we do this below after sending out the fetch requests relevant to the search to give priority to those requests
164+
// that contribute to the final search response
173165
// in any case we count down this result since we don't talk to this shard anymore
174166
counter.countDown();
175167
} else {
176168
executeFetch(
177-
shardPhaseResult,
169+
searchPhaseShardResults.get(i),
178170
counter,
179171
entry,
180-
rankDocs,
172+
rankDocsPerShard == null || rankDocsPerShard.get(i).isEmpty() ? null : new RankDocShardInfo(rankDocsPerShard.get(i)),
181173
(lastEmittedDocPerShard != null) ? lastEmittedDocPerShard[i] : null
182174
);
183175
}
184176
}
177+
for (int i = 0; i < docIdsToLoad.length; i++) {
178+
if (docIdsToLoad[i] == null) {
179+
SearchPhaseResult shardPhaseResult = searchPhaseShardResults.get(i);
180+
if (shardPhaseResult != null) {
181+
releaseIrrelevantSearchContext(shardPhaseResult, context);
182+
progressListener.notifyFetchResult(i);
183+
}
184+
}
185+
}
185186
}
186187

187188
private List<Map<Integer, RankDoc>> splitRankDocsPerShard(ScoreDoc[] scoreDocs, int numShards) {

0 commit comments

Comments
 (0)