From 535cca8839437fec90f591581d5e3015bbb308f7 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 10 Feb 2025 17:01:57 +0100 Subject: [PATCH] Fix small mistake in listener wrapping in SearchService `l` is the same as the `finalListener` here, that's kind of the point of the API. Fixing this here and removing the local variable the enabled the mistake in the first place. Not a bug, but a needless capture that at times makes heap dumps harder to read and wastes a couple cycles. --- .../elasticsearch/search/SearchService.java | 47 ++++++++++--------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/SearchService.java b/server/src/main/java/org/elasticsearch/search/SearchService.java index efa27b2f3448c..e800845e2f26f 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchService.java +++ b/server/src/main/java/org/elasticsearch/search/SearchService.java @@ -582,32 +582,35 @@ private void loadOrExecuteQueryPhase(final ShardSearchRequest request, final Sea } public void executeQueryPhase(ShardSearchRequest request, SearchShardTask task, ActionListener listener) { - ActionListener finalListener = maybeWrapListenerForStackTrace(listener, request.getChannelVersion(), threadPool); assert request.canReturnNullResponseIfMatchNoDocs() == false || request.numberOfShards() > 1 : "empty responses require more than one shard"; final IndexShard shard = getShard(request); - rewriteAndFetchShardRequest(shard, request, finalListener.delegateFailure((l, orig) -> { - // check if we can shortcut the query phase entirely. - if (orig.canReturnNullResponseIfMatchNoDocs()) { - assert orig.scroll() == null; - ShardSearchRequest clone = new ShardSearchRequest(orig); - CanMatchContext canMatchContext = new CanMatchContext( - clone, - indicesService::indexServiceSafe, - this::findReaderContext, - defaultKeepAlive, - maxKeepAlive - ); - CanMatchShardResponse canMatchResp = canMatch(canMatchContext, false); - if (canMatchResp.canMatch() == false) { - finalListener.onResponse(QuerySearchResult.nullInstance()); - return; + rewriteAndFetchShardRequest( + shard, + request, + maybeWrapListenerForStackTrace(listener, request.getChannelVersion(), threadPool).delegateFailure((l, orig) -> { + // check if we can shortcut the query phase entirely. + if (orig.canReturnNullResponseIfMatchNoDocs()) { + assert orig.scroll() == null; + ShardSearchRequest clone = new ShardSearchRequest(orig); + CanMatchContext canMatchContext = new CanMatchContext( + clone, + indicesService::indexServiceSafe, + this::findReaderContext, + defaultKeepAlive, + maxKeepAlive + ); + CanMatchShardResponse canMatchResp = canMatch(canMatchContext, false); + if (canMatchResp.canMatch() == false) { + l.onResponse(QuerySearchResult.nullInstance()); + return; + } } - } - // TODO: i think it makes sense to always do a canMatch here and - // return an empty response (not null response) in case canMatch is false? - ensureAfterSeqNoRefreshed(shard, orig, () -> executeQueryPhase(orig, task), l); - })); + // TODO: i think it makes sense to always do a canMatch here and + // return an empty response (not null response) in case canMatch is false? + ensureAfterSeqNoRefreshed(shard, orig, () -> executeQueryPhase(orig, task), l); + }) + ); } private void ensureAfterSeqNoRefreshed(