Skip to content

Commit 7aa968e

Browse files
Reduce latency in fetch phase for large shard counts (#120419) (#120523)
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 30d72ed commit 7aa968e

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
@@ -155,30 +155,31 @@ private void innerRunFetch(ScoreDoc[] scoreDocs, int numShards, SearchPhaseContr
155155
);
156156
for (int i = 0; i < docIdsToLoad.length; i++) {
157157
List<Integer> entry = docIdsToLoad[i];
158-
RankDocShardInfo rankDocs = rankDocsPerShard == null || rankDocsPerShard.get(i).isEmpty()
159-
? null
160-
: new RankDocShardInfo(rankDocsPerShard.get(i));
161-
SearchPhaseResult shardPhaseResult = searchPhaseShardResults.get(i);
162158
if (entry == null) { // no results for this shard ID
163-
if (shardPhaseResult != null) {
164-
// if we got some hits from this shard we have to release the context there
165-
// we do this as we go since it will free up resources and passing on the request on the
166-
// transport layer is cheap.
167-
releaseIrrelevantSearchContext(shardPhaseResult, context);
168-
progressListener.notifyFetchResult(i);
169-
}
159+
// if we got some hits from this shard we have to release the context
160+
// we do this below after sending out the fetch requests relevant to the search to give priority to those requests
161+
// that contribute to the final search response
170162
// in any case we count down this result since we don't talk to this shard anymore
171163
counter.countDown();
172164
} else {
173165
executeFetch(
174-
shardPhaseResult,
166+
searchPhaseShardResults.get(i),
175167
counter,
176168
entry,
177-
rankDocs,
169+
rankDocsPerShard == null || rankDocsPerShard.get(i).isEmpty() ? null : new RankDocShardInfo(rankDocsPerShard.get(i)),
178170
(lastEmittedDocPerShard != null) ? lastEmittedDocPerShard[i] : null
179171
);
180172
}
181173
}
174+
for (int i = 0; i < docIdsToLoad.length; i++) {
175+
if (docIdsToLoad[i] == null) {
176+
SearchPhaseResult shardPhaseResult = searchPhaseShardResults.get(i);
177+
if (shardPhaseResult != null) {
178+
releaseIrrelevantSearchContext(shardPhaseResult, context);
179+
progressListener.notifyFetchResult(i);
180+
}
181+
}
182+
}
182183
}
183184

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

0 commit comments

Comments
 (0)