From 72eaef0a0e5dbb1ccff311ceb7e8cec391496ab3 Mon Sep 17 00:00:00 2001 From: Chris Parrinello Date: Thu, 9 Oct 2025 11:42:31 -0500 Subject: [PATCH 01/12] init From 5fea59582585bab90f61148dbda62211021ac121 Mon Sep 17 00:00:00 2001 From: Chris Parrinello Date: Thu, 9 Oct 2025 12:41:13 -0500 Subject: [PATCH 02/12] added failing test --- .../SearchPhaseCoordinatorAPMMetricsTests.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java b/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java index 9cd3a8d9dc68c..e7b0bb045fd9e 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java @@ -41,6 +41,7 @@ public class SearchPhaseCoordinatorAPMMetricsTests extends ESSingleNodeTestCase private static final String indexName = "test_coordinator_search_phase_metrics"; private final int num_primaries = randomIntBetween(2, 7); + private static final String CAN_MATCH_SEARCH_PHASE_METRIC = "es.search_response.took_durations.can_match.histogram"; private static final String DFS_QUERY_SEARCH_PHASE_METRIC = "es.search_response.took_durations.dfs_query.histogram"; private static final String DFS_SEARCH_PHASE_METRIC = "es.search_response.took_durations.dfs.histogram"; private static final String FETCH_SEARCH_PHASE_METRIC = "es.search_response.took_durations.fetch.histogram"; @@ -112,6 +113,20 @@ public void testPointInTime() { } } + public void testCanMatchSearch() { + assertSearchHitsWithoutFailures( + client().prepareSearch(indexName) + .setSearchType(SearchType.QUERY_THEN_FETCH) + .setPreFilterShardSize(1) + .setQuery(simpleQueryStringQuery("doc1")), + "1" + ); + + assertMeasurements( + List.of(CAN_MATCH_SEARCH_PHASE_METRIC, FETCH_SEARCH_PHASE_METRIC, QUERY_SEARCH_PHASE_METRIC) + ); + + } private void resetMeter() { getTestTelemetryPlugin().resetMeter(); } From cb58c0e1552ac674006230470ed47ef5e6e9c29b Mon Sep 17 00:00:00 2001 From: Chris Parrinello Date: Fri, 10 Oct 2025 10:55:55 -0500 Subject: [PATCH 03/12] can match coordinator metric --- .../search/CanMatchPreFilterSearchPhase.java | 18 +++++++++++++++--- .../search/TransportOpenPointInTimeAction.java | 3 ++- .../action/search/TransportSearchAction.java | 3 ++- .../search/TransportSearchShardsAction.java | 3 ++- .../action/search/SearchResponseMetrics.java | 6 ++++++ .../CanMatchPreFilterSearchPhaseTests.java | 15 ++++++++++----- .../SearchPhaseCoordinatorAPMMetricsTests.java | 6 ++---- 7 files changed, 39 insertions(+), 15 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java b/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java index b5faf88dff125..2447c8cf82e17 100644 --- a/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java +++ b/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java @@ -19,6 +19,7 @@ import org.elasticsearch.core.Nullable; import org.elasticsearch.index.query.CoordinatorRewriteContext; import org.elasticsearch.index.query.CoordinatorRewriteContextProvider; +import org.elasticsearch.rest.action.search.SearchResponseMetrics; import org.elasticsearch.search.CanMatchShardResponse; import org.elasticsearch.search.SearchService; import org.elasticsearch.search.SearchShardTarget; @@ -76,6 +77,8 @@ final class CanMatchPreFilterSearchPhase { private final MinAndMax[] minAndMaxes; private int numPossibleMatches; private final CoordinatorRewriteContextProvider coordinatorRewriteContextProvider; + private final SearchResponseMetrics searchResponseMetrics; + private long phaseStartTimeInNanos; private CanMatchPreFilterSearchPhase( Logger logger, @@ -90,7 +93,8 @@ private CanMatchPreFilterSearchPhase( SearchTask task, boolean requireAtLeastOneMatch, CoordinatorRewriteContextProvider coordinatorRewriteContextProvider, - ActionListener> listener + ActionListener> listener, + SearchResponseMetrics searchResponseMetrics ) { this.logger = logger; this.searchTransportService = searchTransportService; @@ -122,6 +126,7 @@ private CanMatchPreFilterSearchPhase( shardItIndexMap.put(naturalOrder[j], j); } this.shardItIndexMap = shardItIndexMap; + this.searchResponseMetrics = searchResponseMetrics; } public static SubscribableListener> execute( @@ -136,7 +141,8 @@ public static SubscribableListener> execute( TransportSearchAction.SearchTimeProvider timeProvider, SearchTask task, boolean requireAtLeastOneMatch, - CoordinatorRewriteContextProvider coordinatorRewriteContextProvider + CoordinatorRewriteContextProvider coordinatorRewriteContextProvider, + SearchResponseMetrics searchResponseMetrics ) { if (shardsIts.isEmpty()) { return SubscribableListener.newSucceeded(List.of()); @@ -168,7 +174,8 @@ protected void doRun() { task, requireAtLeastOneMatch, coordinatorRewriteContextProvider, - listener + listener, + searchResponseMetrics ).runCoordinatorRewritePhase(); } }); @@ -184,6 +191,7 @@ private static boolean assertSearchCoordinationThread() { private void runCoordinatorRewritePhase() { // TODO: the index filter (i.e, `_index:patten`) should be prefiltered on the coordinator assert assertSearchCoordinationThread(); + phaseStartTimeInNanos = System.nanoTime(); final List matchedShardLevelRequests = new ArrayList<>(); for (SearchShardIterator searchShardIterator : shardsIts) { final CanMatchNodeRequest canMatchNodeRequest = new CanMatchNodeRequest( @@ -223,6 +231,10 @@ private void runCoordinatorRewritePhase() { checkNoMissingShards(matchedShardLevelRequests); new Round(matchedShardLevelRequests).run(); } + // this could be null in the case where this phase is running in a remote cluster + if (searchResponseMetrics != null) { + searchResponseMetrics.recordSearchPhaseDuration("can_match", System.nanoTime() - phaseStartTimeInNanos); + } } private void consumeResult(boolean canMatch, ShardSearchRequest request) { diff --git a/server/src/main/java/org/elasticsearch/action/search/TransportOpenPointInTimeAction.java b/server/src/main/java/org/elasticsearch/action/search/TransportOpenPointInTimeAction.java index 2563b0f5f26f0..f9e779a610df4 100644 --- a/server/src/main/java/org/elasticsearch/action/search/TransportOpenPointInTimeAction.java +++ b/server/src/main/java/org/elasticsearch/action/search/TransportOpenPointInTimeAction.java @@ -184,7 +184,8 @@ public void runNewSearchPhase( timeProvider, task, false, - searchService.getCoordinatorRewriteContextProvider(timeProvider::absoluteStartMillis) + searchService.getCoordinatorRewriteContextProvider(timeProvider::absoluteStartMillis), + searchResponseMetrics ) .addListener( listener.delegateFailureAndWrap( diff --git a/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java b/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java index d5085701efb88..41bf1d9e25b41 100644 --- a/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java +++ b/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java @@ -1643,7 +1643,8 @@ public void runNewSearchPhase( timeProvider, task, requireAtLeastOneMatch, - searchService.getCoordinatorRewriteContextProvider(timeProvider::absoluteStartMillis) + searchService.getCoordinatorRewriteContextProvider(timeProvider::absoluteStartMillis), + searchResponseMetrics ) .addListener( listener.delegateFailureAndWrap( diff --git a/server/src/main/java/org/elasticsearch/action/search/TransportSearchShardsAction.java b/server/src/main/java/org/elasticsearch/action/search/TransportSearchShardsAction.java index 507893df0c0c1..b8d0a976108a5 100644 --- a/server/src/main/java/org/elasticsearch/action/search/TransportSearchShardsAction.java +++ b/server/src/main/java/org/elasticsearch/action/search/TransportSearchShardsAction.java @@ -175,7 +175,8 @@ public void searchShards(Task task, SearchShardsRequest searchShardsRequest, Act timeProvider, (SearchTask) task, false, - searchService.getCoordinatorRewriteContextProvider(timeProvider::absoluteStartMillis) + searchService.getCoordinatorRewriteContextProvider(timeProvider::absoluteStartMillis), + null // do not record a phase duration for can_match in search_shards ) .addListener( delegate.map( diff --git a/server/src/main/java/org/elasticsearch/rest/action/search/SearchResponseMetrics.java b/server/src/main/java/org/elasticsearch/rest/action/search/SearchResponseMetrics.java index 240d08874c2fb..2e1a702f8d68e 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/search/SearchResponseMetrics.java +++ b/server/src/main/java/org/elasticsearch/rest/action/search/SearchResponseMetrics.java @@ -66,6 +66,12 @@ public SearchResponseMetrics(MeterRegistry meterRegistry) { ); phaseNameToDurationHistogram = Map.of( + "can_match", + meterRegistry.registerLongHistogram( + String.format(Locale.ROOT, SEARCH_PHASE_METRIC_FORMAT, "can_match"), + "The search phase can_match duration in milliseconds at the coordinator, expressed as a histogram", + "millis" + ), "dfs", meterRegistry.registerLongHistogram( String.format(Locale.ROOT, SEARCH_PHASE_METRIC_FORMAT, "dfs"), diff --git a/server/src/test/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhaseTests.java b/server/src/test/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhaseTests.java index 1c3a6cd47a3b7..1ca81848c1456 100644 --- a/server/src/test/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhaseTests.java +++ b/server/src/test/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhaseTests.java @@ -161,7 +161,8 @@ public void sendCanMatch( timeProvider, null, true, - EMPTY_CONTEXT_PROVIDER + EMPTY_CONTEXT_PROVIDER, + null ).addListener(ActionTestUtils.assertNoFailureListener(iter -> { result.set(iter); latch.countDown(); @@ -256,7 +257,8 @@ public void sendCanMatch( timeProvider, null, true, - EMPTY_CONTEXT_PROVIDER + EMPTY_CONTEXT_PROVIDER, + null ).addListener(ActionTestUtils.assertNoFailureListener(iter -> { result.set(iter); latch.countDown(); @@ -347,7 +349,8 @@ public void sendCanMatch( timeProvider, null, true, - EMPTY_CONTEXT_PROVIDER + EMPTY_CONTEXT_PROVIDER, + null ).addListener(ActionTestUtils.assertNoFailureListener(iter -> { result.set(iter); latch.countDown(); @@ -446,7 +449,8 @@ public void sendCanMatch( timeProvider, null, shardsIter.size() > shardToSkip.size(), - EMPTY_CONTEXT_PROVIDER + EMPTY_CONTEXT_PROVIDER, + null ).addListener(ActionTestUtils.assertNoFailureListener(iter -> { result.set(iter); latch.countDown(); @@ -1418,7 +1422,8 @@ public void sendCanMatch( timeProvider, null, true, - contextProvider + contextProvider, + null ), requests ); diff --git a/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java b/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java index e7b0bb045fd9e..b88d76a7d811a 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java @@ -122,11 +122,9 @@ public void testCanMatchSearch() { "1" ); - assertMeasurements( - List.of(CAN_MATCH_SEARCH_PHASE_METRIC, FETCH_SEARCH_PHASE_METRIC, QUERY_SEARCH_PHASE_METRIC) - ); - + assertMeasurements(List.of(CAN_MATCH_SEARCH_PHASE_METRIC, FETCH_SEARCH_PHASE_METRIC, QUERY_SEARCH_PHASE_METRIC)); } + private void resetMeter() { getTestTelemetryPlugin().resetMeter(); } From 97bfd37277fd722ae2516ac16aed4506a610ac9c Mon Sep 17 00:00:00 2001 From: Chris Parrinello Date: Mon, 13 Oct 2025 15:09:51 -0500 Subject: [PATCH 04/12] better coordinator level metrics --- .../search/CanMatchPreFilterSearchPhase.java | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java b/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java index 2447c8cf82e17..dafb0c842d185 100644 --- a/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java +++ b/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java @@ -77,8 +77,6 @@ final class CanMatchPreFilterSearchPhase { private final MinAndMax[] minAndMaxes; private int numPossibleMatches; private final CoordinatorRewriteContextProvider coordinatorRewriteContextProvider; - private final SearchResponseMetrics searchResponseMetrics; - private long phaseStartTimeInNanos; private CanMatchPreFilterSearchPhase( Logger logger, @@ -126,7 +124,6 @@ private CanMatchPreFilterSearchPhase( shardItIndexMap.put(naturalOrder[j], j); } this.shardItIndexMap = shardItIndexMap; - this.searchResponseMetrics = searchResponseMetrics; } public static SubscribableListener> execute( @@ -148,6 +145,23 @@ public static SubscribableListener> execute( return SubscribableListener.newSucceeded(List.of()); } final SubscribableListener> listener = new SubscribableListener<>(); + long phaseStartTimeInNanos = System.nanoTime(); + + listener.addListener(new ActionListener<>() { + @Override + public void onResponse(List shardsIts) { + // searchResponseMetrics is null for node can-match requests + if (searchResponseMetrics != null) { + searchResponseMetrics.recordSearchPhaseDuration("can_match", System.nanoTime() - phaseStartTimeInNanos); + } + } + + @Override + public void onFailure(Exception e) { + // do not record duration of failed phases + } + }); + // Note that the search is failed when this task is rejected by the executor executor.execute(new AbstractRunnable() { @Override @@ -191,7 +205,6 @@ private static boolean assertSearchCoordinationThread() { private void runCoordinatorRewritePhase() { // TODO: the index filter (i.e, `_index:patten`) should be prefiltered on the coordinator assert assertSearchCoordinationThread(); - phaseStartTimeInNanos = System.nanoTime(); final List matchedShardLevelRequests = new ArrayList<>(); for (SearchShardIterator searchShardIterator : shardsIts) { final CanMatchNodeRequest canMatchNodeRequest = new CanMatchNodeRequest( @@ -231,10 +244,6 @@ private void runCoordinatorRewritePhase() { checkNoMissingShards(matchedShardLevelRequests); new Round(matchedShardLevelRequests).run(); } - // this could be null in the case where this phase is running in a remote cluster - if (searchResponseMetrics != null) { - searchResponseMetrics.recordSearchPhaseDuration("can_match", System.nanoTime() - phaseStartTimeInNanos); - } } private void consumeResult(boolean canMatch, ShardSearchRequest request) { From f31181ed5ba01f784187cb8c87f20a47404bafa0 Mon Sep 17 00:00:00 2001 From: Chris Parrinello Date: Mon, 20 Oct 2025 12:32:59 -0500 Subject: [PATCH 05/12] Update docs/changelog/136828.yaml --- docs/changelog/136828.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/136828.yaml diff --git a/docs/changelog/136828.yaml b/docs/changelog/136828.yaml new file mode 100644 index 0000000000000..7d4ffe3d892c2 --- /dev/null +++ b/docs/changelog/136828.yaml @@ -0,0 +1,5 @@ +pr: 136828 +summary: Can match phase coordinator duration APM metric +area: Search +type: enhancement +issues: [] From 9ef90b00e354e505677fda7705adb4a32395e357 Mon Sep 17 00:00:00 2001 From: Chris Parrinello Date: Thu, 9 Oct 2025 11:42:31 -0500 Subject: [PATCH 06/12] init From 76844ec920833bd749ec67298ed70fe7bd34f3f1 Mon Sep 17 00:00:00 2001 From: Chris Parrinello Date: Fri, 24 Oct 2025 12:39:40 -0500 Subject: [PATCH 07/12] replace can_match with a constant --- .../action/search/CanMatchPreFilterSearchPhase.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java b/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java index dafb0c842d185..8f0e4093bb4bd 100644 --- a/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java +++ b/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java @@ -59,6 +59,8 @@ */ final class CanMatchPreFilterSearchPhase { + private static final String PHASE_NAME = "can_match"; + private final Logger logger; private final SearchRequest request; private final List shardsIts; @@ -152,7 +154,7 @@ public static SubscribableListener> execute( public void onResponse(List shardsIts) { // searchResponseMetrics is null for node can-match requests if (searchResponseMetrics != null) { - searchResponseMetrics.recordSearchPhaseDuration("can_match", System.nanoTime() - phaseStartTimeInNanos); + searchResponseMetrics.recordSearchPhaseDuration(PHASE_NAME, System.nanoTime() - phaseStartTimeInNanos); } } @@ -169,7 +171,7 @@ public void onFailure(Exception e) { if (logger.isDebugEnabled()) { logger.debug(() -> format("Failed to execute [%s] while running [can_match] phase", request), e); } - listener.onFailure(new SearchPhaseExecutionException("can_match", "start", e, ShardSearchFailure.EMPTY_ARRAY)); + listener.onFailure(new SearchPhaseExecutionException(PHASE_NAME, "start", e, ShardSearchFailure.EMPTY_ARRAY)); } @Override @@ -270,7 +272,7 @@ private synchronized void consumeResult(int shardIndex, boolean canMatch, MinAnd private void checkNoMissingShards(List shards) { assert assertSearchCoordinationThread(); - SearchPhase.doCheckNoMissingShards("can_match", request, shards); + SearchPhase.doCheckNoMissingShards(PHASE_NAME, request, shards); } private Map> groupByNode(List shards) { @@ -407,7 +409,7 @@ public void onFailure(Exception e) { if (logger.isDebugEnabled()) { logger.debug(() -> format("Failed to execute [%s] while running [can_match] phase", request), e); } - listener.onFailure(new SearchPhaseExecutionException("can_match", "round", e, ShardSearchFailure.EMPTY_ARRAY)); + listener.onFailure(new SearchPhaseExecutionException(PHASE_NAME, "round", e, ShardSearchFailure.EMPTY_ARRAY)); } } From 07a8a3e0f84d7caac2f9a32484c51148fa5f0f00 Mon Sep 17 00:00:00 2001 From: Chris Parrinello Date: Tue, 28 Oct 2025 10:34:42 -0500 Subject: [PATCH 08/12] PR fixes --- .../search/CanMatchPreFilterSearchPhase.java | 5 ++- .../TransportOpenPointInTimeAction.java | 2 +- ...SearchPhaseCoordinatorAPMMetricsTests.java | 34 +++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java b/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java index 8f0e4093bb4bd..87e7519befbb8 100644 --- a/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java +++ b/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java @@ -152,7 +152,10 @@ public static SubscribableListener> execute( listener.addListener(new ActionListener<>() { @Override public void onResponse(List shardsIts) { - // searchResponseMetrics is null for node can-match requests + // we only want to record the phase duration if this call to execute is running on the coordinator node + // as part of the can-match phase or a search request. It will be null if this is the data node round trip can-match + // execution + // or an open PIT request if (searchResponseMetrics != null) { searchResponseMetrics.recordSearchPhaseDuration(PHASE_NAME, System.nanoTime() - phaseStartTimeInNanos); } diff --git a/server/src/main/java/org/elasticsearch/action/search/TransportOpenPointInTimeAction.java b/server/src/main/java/org/elasticsearch/action/search/TransportOpenPointInTimeAction.java index f9e779a610df4..6b91d6af4f5cc 100644 --- a/server/src/main/java/org/elasticsearch/action/search/TransportOpenPointInTimeAction.java +++ b/server/src/main/java/org/elasticsearch/action/search/TransportOpenPointInTimeAction.java @@ -185,7 +185,7 @@ public void runNewSearchPhase( task, false, searchService.getCoordinatorRewriteContextProvider(timeProvider::absoluteStartMillis), - searchResponseMetrics + null ) .addListener( listener.delegateFailureAndWrap( diff --git a/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java b/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java index b88d76a7d811a..2be8134e03d98 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java @@ -84,6 +84,7 @@ public void testSearchQueryThenFetch() { "1" ); assertMeasurements(List.of(QUERY_SEARCH_PHASE_METRIC, FETCH_SEARCH_PHASE_METRIC)); + assertNotMeasured(List.of(CAN_MATCH_SEARCH_PHASE_METRIC, DFS_SEARCH_PHASE_METRIC, DFS_QUERY_SEARCH_PHASE_METRIC, OPEN_PIT_SEARCH_PHASE_METRIC)); } public void testDfsSearch() { @@ -92,10 +93,12 @@ public void testDfsSearch() { "1" ); assertMeasurements(List.of(DFS_SEARCH_PHASE_METRIC, DFS_QUERY_SEARCH_PHASE_METRIC, FETCH_SEARCH_PHASE_METRIC)); + assertNotMeasured(List.of(CAN_MATCH_SEARCH_PHASE_METRIC, QUERY_SEARCH_PHASE_METRIC, OPEN_PIT_SEARCH_PHASE_METRIC)); } public void testPointInTime() { OpenPointInTimeRequest request = new OpenPointInTimeRequest(indexName).keepAlive(TimeValue.timeValueMinutes(10)); + request.indexFilter(simpleQueryStringQuery("doc1")); OpenPointInTimeResponse response = client().execute(TransportOpenPointInTimeAction.TYPE, request).actionGet(); BytesReference pointInTimeId = response.getPointInTimeId(); @@ -108,6 +111,29 @@ public void testPointInTime() { "1" ); assertMeasurements(List.of(OPEN_PIT_SEARCH_PHASE_METRIC, QUERY_SEARCH_PHASE_METRIC, FETCH_SEARCH_PHASE_METRIC)); + assertNotMeasured(List.of(CAN_MATCH_SEARCH_PHASE_METRIC, DFS_SEARCH_PHASE_METRIC, DFS_QUERY_SEARCH_PHASE_METRIC)); + } finally { + client().execute(TransportClosePointInTimeAction.TYPE, new ClosePointInTimeRequest(pointInTimeId)).actionGet(); + } + } + + public void testPointInTimeWithPreFiltering() { + OpenPointInTimeRequest request = new OpenPointInTimeRequest(indexName).keepAlive(TimeValue.timeValueMinutes(10)); + request.indexFilter(simpleQueryStringQuery("doc1")); + OpenPointInTimeResponse response = client().execute(TransportOpenPointInTimeAction.TYPE, request).actionGet(); + BytesReference pointInTimeId = response.getPointInTimeId(); + + try { + assertSearchHitsWithoutFailures( + client().prepareSearch() + .setPointInTime(new PointInTimeBuilder(pointInTimeId)) + .setSize(1) + .setPreFilterShardSize(1) + .setQuery(simpleQueryStringQuery("doc1")), + "1" + ); + assertMeasurements(List.of(OPEN_PIT_SEARCH_PHASE_METRIC, CAN_MATCH_SEARCH_PHASE_METRIC, QUERY_SEARCH_PHASE_METRIC, FETCH_SEARCH_PHASE_METRIC)); + assertNotMeasured(List.of(DFS_SEARCH_PHASE_METRIC, DFS_QUERY_SEARCH_PHASE_METRIC)); } finally { client().execute(TransportClosePointInTimeAction.TYPE, new ClosePointInTimeRequest(pointInTimeId)).actionGet(); } @@ -123,6 +149,7 @@ public void testCanMatchSearch() { ); assertMeasurements(List.of(CAN_MATCH_SEARCH_PHASE_METRIC, FETCH_SEARCH_PHASE_METRIC, QUERY_SEARCH_PHASE_METRIC)); + assertNotMeasured(List.of(DFS_SEARCH_PHASE_METRIC, DFS_QUERY_SEARCH_PHASE_METRIC, OPEN_PIT_SEARCH_PHASE_METRIC)); } private void resetMeter() { @@ -133,6 +160,13 @@ private TestTelemetryPlugin getTestTelemetryPlugin() { return getInstanceFromNode(PluginsService.class).filterPlugins(TestTelemetryPlugin.class).toList().get(0); } + private void assertNotMeasured(Collection metricNames) { + for (var metricName : metricNames) { + List measurements = getTestTelemetryPlugin().getLongHistogramMeasurement(metricName); + assertThat(measurements, hasSize(0)); + } + } + private void assertMeasurements(Collection metricNames) { for (var metricName : metricNames) { List measurements = getTestTelemetryPlugin().getLongHistogramMeasurement(metricName); From ca3fd60abe2cf0195fdacbbae270e482dc1d1fec Mon Sep 17 00:00:00 2001 From: Chris Parrinello Date: Tue, 28 Oct 2025 10:37:52 -0500 Subject: [PATCH 09/12] spotless apply --- .../search/SearchPhaseCoordinatorAPMMetricsTests.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java b/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java index 2be8134e03d98..848d6d0c76d86 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java @@ -84,7 +84,9 @@ public void testSearchQueryThenFetch() { "1" ); assertMeasurements(List.of(QUERY_SEARCH_PHASE_METRIC, FETCH_SEARCH_PHASE_METRIC)); - assertNotMeasured(List.of(CAN_MATCH_SEARCH_PHASE_METRIC, DFS_SEARCH_PHASE_METRIC, DFS_QUERY_SEARCH_PHASE_METRIC, OPEN_PIT_SEARCH_PHASE_METRIC)); + assertNotMeasured( + List.of(CAN_MATCH_SEARCH_PHASE_METRIC, DFS_SEARCH_PHASE_METRIC, DFS_QUERY_SEARCH_PHASE_METRIC, OPEN_PIT_SEARCH_PHASE_METRIC) + ); } public void testDfsSearch() { @@ -132,7 +134,9 @@ public void testPointInTimeWithPreFiltering() { .setQuery(simpleQueryStringQuery("doc1")), "1" ); - assertMeasurements(List.of(OPEN_PIT_SEARCH_PHASE_METRIC, CAN_MATCH_SEARCH_PHASE_METRIC, QUERY_SEARCH_PHASE_METRIC, FETCH_SEARCH_PHASE_METRIC)); + assertMeasurements( + List.of(OPEN_PIT_SEARCH_PHASE_METRIC, CAN_MATCH_SEARCH_PHASE_METRIC, QUERY_SEARCH_PHASE_METRIC, FETCH_SEARCH_PHASE_METRIC) + ); assertNotMeasured(List.of(DFS_SEARCH_PHASE_METRIC, DFS_QUERY_SEARCH_PHASE_METRIC)); } finally { client().execute(TransportClosePointInTimeAction.TYPE, new ClosePointInTimeRequest(pointInTimeId)).actionGet(); From 3e07ffa9d0ce2b42d6974395a856921295175c54 Mon Sep 17 00:00:00 2001 From: Chris Parrinello Date: Wed, 29 Oct 2025 13:46:16 -0500 Subject: [PATCH 10/12] measure can-match coordinator execution for all code paths --- .../search/CanMatchPreFilterSearchPhase.java | 14 +++--------- .../TransportOpenPointInTimeAction.java | 2 +- .../search/TransportSearchShardsAction.java | 8 +++++-- .../CanMatchPreFilterSearchPhaseTests.java | 12 +++++----- ...SearchPhaseCoordinatorAPMMetricsTests.java | 22 ++++++++++--------- 5 files changed, 29 insertions(+), 29 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java b/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java index 87e7519befbb8..c9285c23ef53e 100644 --- a/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java +++ b/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java @@ -93,8 +93,7 @@ private CanMatchPreFilterSearchPhase( SearchTask task, boolean requireAtLeastOneMatch, CoordinatorRewriteContextProvider coordinatorRewriteContextProvider, - ActionListener> listener, - SearchResponseMetrics searchResponseMetrics + ActionListener> listener ) { this.logger = logger; this.searchTransportService = searchTransportService; @@ -152,13 +151,7 @@ public static SubscribableListener> execute( listener.addListener(new ActionListener<>() { @Override public void onResponse(List shardsIts) { - // we only want to record the phase duration if this call to execute is running on the coordinator node - // as part of the can-match phase or a search request. It will be null if this is the data node round trip can-match - // execution - // or an open PIT request - if (searchResponseMetrics != null) { - searchResponseMetrics.recordSearchPhaseDuration(PHASE_NAME, System.nanoTime() - phaseStartTimeInNanos); - } + searchResponseMetrics.recordSearchPhaseDuration(PHASE_NAME, System.nanoTime() - phaseStartTimeInNanos); } @Override @@ -193,8 +186,7 @@ protected void doRun() { task, requireAtLeastOneMatch, coordinatorRewriteContextProvider, - listener, - searchResponseMetrics + listener ).runCoordinatorRewritePhase(); } }); diff --git a/server/src/main/java/org/elasticsearch/action/search/TransportOpenPointInTimeAction.java b/server/src/main/java/org/elasticsearch/action/search/TransportOpenPointInTimeAction.java index 6b91d6af4f5cc..f9e779a610df4 100644 --- a/server/src/main/java/org/elasticsearch/action/search/TransportOpenPointInTimeAction.java +++ b/server/src/main/java/org/elasticsearch/action/search/TransportOpenPointInTimeAction.java @@ -185,7 +185,7 @@ public void runNewSearchPhase( task, false, searchService.getCoordinatorRewriteContextProvider(timeProvider::absoluteStartMillis), - null + searchResponseMetrics ) .addListener( listener.delegateFailureAndWrap( diff --git a/server/src/main/java/org/elasticsearch/action/search/TransportSearchShardsAction.java b/server/src/main/java/org/elasticsearch/action/search/TransportSearchShardsAction.java index b8d0a976108a5..3ce1cce8a9162 100644 --- a/server/src/main/java/org/elasticsearch/action/search/TransportSearchShardsAction.java +++ b/server/src/main/java/org/elasticsearch/action/search/TransportSearchShardsAction.java @@ -25,6 +25,7 @@ import org.elasticsearch.index.Index; import org.elasticsearch.index.query.Rewriteable; import org.elasticsearch.injection.guice.Inject; +import org.elasticsearch.rest.action.search.SearchResponseMetrics; import org.elasticsearch.search.SearchService; import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.search.internal.AliasFilter; @@ -60,6 +61,7 @@ public class TransportSearchShardsAction extends HandledTransportAction { result.set(iter); latch.countDown(); @@ -258,7 +260,7 @@ public void sendCanMatch( null, true, EMPTY_CONTEXT_PROVIDER, - null + new SearchResponseMetrics(TelemetryProvider.NOOP.getMeterRegistry()) ).addListener(ActionTestUtils.assertNoFailureListener(iter -> { result.set(iter); latch.countDown(); @@ -350,7 +352,7 @@ public void sendCanMatch( null, true, EMPTY_CONTEXT_PROVIDER, - null + new SearchResponseMetrics(TelemetryProvider.NOOP.getMeterRegistry()) ).addListener(ActionTestUtils.assertNoFailureListener(iter -> { result.set(iter); latch.countDown(); @@ -450,7 +452,7 @@ public void sendCanMatch( null, shardsIter.size() > shardToSkip.size(), EMPTY_CONTEXT_PROVIDER, - null + new SearchResponseMetrics(TelemetryProvider.NOOP.getMeterRegistry()) ).addListener(ActionTestUtils.assertNoFailureListener(iter -> { result.set(iter); latch.countDown(); @@ -1423,7 +1425,7 @@ public void sendCanMatch( null, true, contextProvider, - null + new SearchResponseMetrics(TelemetryProvider.NOOP.getMeterRegistry()) ), requests ); diff --git a/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java b/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java index 848d6d0c76d86..10ac101a6ac8b 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java @@ -83,7 +83,7 @@ public void testSearchQueryThenFetch() { client().prepareSearch(indexName).setSearchType(SearchType.QUERY_THEN_FETCH).setQuery(simpleQueryStringQuery("doc1")), "1" ); - assertMeasurements(List.of(QUERY_SEARCH_PHASE_METRIC, FETCH_SEARCH_PHASE_METRIC)); + assertMeasurements(List.of(QUERY_SEARCH_PHASE_METRIC, FETCH_SEARCH_PHASE_METRIC), 1); assertNotMeasured( List.of(CAN_MATCH_SEARCH_PHASE_METRIC, DFS_SEARCH_PHASE_METRIC, DFS_QUERY_SEARCH_PHASE_METRIC, OPEN_PIT_SEARCH_PHASE_METRIC) ); @@ -94,7 +94,7 @@ public void testDfsSearch() { client().prepareSearch(indexName).setSearchType(SearchType.DFS_QUERY_THEN_FETCH).setQuery(simpleQueryStringQuery("doc1")), "1" ); - assertMeasurements(List.of(DFS_SEARCH_PHASE_METRIC, DFS_QUERY_SEARCH_PHASE_METRIC, FETCH_SEARCH_PHASE_METRIC)); + assertMeasurements(List.of(DFS_SEARCH_PHASE_METRIC, DFS_QUERY_SEARCH_PHASE_METRIC, FETCH_SEARCH_PHASE_METRIC), 1); assertNotMeasured(List.of(CAN_MATCH_SEARCH_PHASE_METRIC, QUERY_SEARCH_PHASE_METRIC, OPEN_PIT_SEARCH_PHASE_METRIC)); } @@ -112,8 +112,8 @@ public void testPointInTime() { .setQuery(simpleQueryStringQuery("doc1")), "1" ); - assertMeasurements(List.of(OPEN_PIT_SEARCH_PHASE_METRIC, QUERY_SEARCH_PHASE_METRIC, FETCH_SEARCH_PHASE_METRIC)); - assertNotMeasured(List.of(CAN_MATCH_SEARCH_PHASE_METRIC, DFS_SEARCH_PHASE_METRIC, DFS_QUERY_SEARCH_PHASE_METRIC)); + assertMeasurements(List.of(OPEN_PIT_SEARCH_PHASE_METRIC, QUERY_SEARCH_PHASE_METRIC, FETCH_SEARCH_PHASE_METRIC), 1); + assertNotMeasured(List.of(DFS_SEARCH_PHASE_METRIC, DFS_QUERY_SEARCH_PHASE_METRIC)); } finally { client().execute(TransportClosePointInTimeAction.TYPE, new ClosePointInTimeRequest(pointInTimeId)).actionGet(); } @@ -134,8 +134,10 @@ public void testPointInTimeWithPreFiltering() { .setQuery(simpleQueryStringQuery("doc1")), "1" ); + assertMeasurements(List.of(OPEN_PIT_SEARCH_PHASE_METRIC, QUERY_SEARCH_PHASE_METRIC, FETCH_SEARCH_PHASE_METRIC), 1); assertMeasurements( - List.of(OPEN_PIT_SEARCH_PHASE_METRIC, CAN_MATCH_SEARCH_PHASE_METRIC, QUERY_SEARCH_PHASE_METRIC, FETCH_SEARCH_PHASE_METRIC) + List.of(CAN_MATCH_SEARCH_PHASE_METRIC), + 2 // one during open PIT, one during can-match phase of search ); assertNotMeasured(List.of(DFS_SEARCH_PHASE_METRIC, DFS_QUERY_SEARCH_PHASE_METRIC)); } finally { @@ -152,7 +154,7 @@ public void testCanMatchSearch() { "1" ); - assertMeasurements(List.of(CAN_MATCH_SEARCH_PHASE_METRIC, FETCH_SEARCH_PHASE_METRIC, QUERY_SEARCH_PHASE_METRIC)); + assertMeasurements(List.of(CAN_MATCH_SEARCH_PHASE_METRIC, FETCH_SEARCH_PHASE_METRIC, QUERY_SEARCH_PHASE_METRIC), 1); assertNotMeasured(List.of(DFS_SEARCH_PHASE_METRIC, DFS_QUERY_SEARCH_PHASE_METRIC, OPEN_PIT_SEARCH_PHASE_METRIC)); } @@ -167,15 +169,15 @@ private TestTelemetryPlugin getTestTelemetryPlugin() { private void assertNotMeasured(Collection metricNames) { for (var metricName : metricNames) { List measurements = getTestTelemetryPlugin().getLongHistogramMeasurement(metricName); - assertThat(measurements, hasSize(0)); + assertThat(metricName, measurements, hasSize(0)); } } - private void assertMeasurements(Collection metricNames) { + private void assertMeasurements(Collection metricNames, int numberOfMeasurements) { for (var metricName : metricNames) { List measurements = getTestTelemetryPlugin().getLongHistogramMeasurement(metricName); - assertThat(measurements, hasSize(1)); - assertThat(measurements.getFirst().getLong(), greaterThanOrEqualTo(0L)); + assertThat(metricName, measurements, hasSize(numberOfMeasurements)); + assertThat(metricName, measurements.getFirst().getLong(), greaterThanOrEqualTo(0L)); } } } From cdffc5b58cb0c9c0378bd6b3cddc8f9e2aac973f Mon Sep 17 00:00:00 2001 From: Chris Parrinello Date: Wed, 29 Oct 2025 15:57:06 -0500 Subject: [PATCH 11/12] added test for search shards --- .../SearchPhaseCoordinatorAPMMetricsTests.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java b/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java index 10ac101a6ac8b..3a175429540ce 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java @@ -12,9 +12,12 @@ import org.elasticsearch.action.search.ClosePointInTimeRequest; import org.elasticsearch.action.search.OpenPointInTimeRequest; import org.elasticsearch.action.search.OpenPointInTimeResponse; +import org.elasticsearch.action.search.SearchRequest; +import org.elasticsearch.action.search.SearchShardsRequest; import org.elasticsearch.action.search.SearchType; import org.elasticsearch.action.search.TransportClosePointInTimeAction; import org.elasticsearch.action.search.TransportOpenPointInTimeAction; +import org.elasticsearch.action.search.TransportSearchShardsAction; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.settings.Settings; @@ -158,6 +161,21 @@ public void testCanMatchSearch() { assertNotMeasured(List.of(DFS_SEARCH_PHASE_METRIC, DFS_QUERY_SEARCH_PHASE_METRIC, OPEN_PIT_SEARCH_PHASE_METRIC)); } + public void testSearchShards() { + var request = new SearchShardsRequest( + new String[] { indexName }, + SearchRequest.DEFAULT_INDICES_OPTIONS, + simpleQueryStringQuery("doc1"), + null, + null, + randomBoolean(), + randomBoolean() ? null : randomAlphaOfLength(10) + ); + var resp = client().execute(TransportSearchShardsAction.TYPE, request).actionGet(); + assertThat(resp.getGroups(), hasSize(num_primaries)); + assertMeasurements(List.of(CAN_MATCH_SEARCH_PHASE_METRIC), 1); + } + private void resetMeter() { getTestTelemetryPlugin().resetMeter(); } From 85ddf1dcd4c73f4978526ea146dd0ed2b3ead4bf Mon Sep 17 00:00:00 2001 From: Chris Parrinello Date: Thu, 30 Oct 2025 10:28:44 -0500 Subject: [PATCH 12/12] more involved open PIT test --- ...SearchPhaseCoordinatorAPMMetricsTests.java | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java b/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java index 3a175429540ce..70a03419c3a5e 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/search/SearchPhaseCoordinatorAPMMetricsTests.java @@ -22,6 +22,7 @@ import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.TimeValue; +import org.elasticsearch.index.query.RangeQueryBuilder; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.plugins.PluginsService; import org.elasticsearch.search.builder.PointInTimeBuilder; @@ -42,6 +43,7 @@ public class SearchPhaseCoordinatorAPMMetricsTests extends ESSingleNodeTestCase { private static final String indexName = "test_coordinator_search_phase_metrics"; + private static final String secondIndexName = "test_coordinator_search_phase_metrics_2"; private final int num_primaries = randomIntBetween(2, 7); private static final String CAN_MATCH_SEARCH_PHASE_METRIC = "es.search_response.took_durations.can_match.histogram"; @@ -67,8 +69,22 @@ private void setUpIndex() throws Exception { ); ensureGreen(indexName); - prepareIndex(indexName).setId("1").setSource("body", "doc1").setRefreshPolicy(IMMEDIATE).get(); - prepareIndex(indexName).setId("2").setSource("body", "doc2").setRefreshPolicy(IMMEDIATE).get(); + prepareIndex(indexName).setId("1").setSource("body", "doc1", "@timestamp", "2024-11-01").setRefreshPolicy(IMMEDIATE).get(); + prepareIndex(indexName).setId("2").setSource("body", "doc2", "@timestamp", "2024-12-01").setRefreshPolicy(IMMEDIATE).get(); + prepareIndex(indexName).setId("3").setSource("body", "doc3", "@timestamp", "2025-01-01").setRefreshPolicy(IMMEDIATE).get(); + + createIndex( + secondIndexName, + Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, num_primaries) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) + .build() + ); + ensureGreen(secondIndexName); + + prepareIndex(secondIndexName).setId("4").setSource("body", "doc1", "@timestamp", "2025-11-01").setRefreshPolicy(IMMEDIATE).get(); + prepareIndex(secondIndexName).setId("5").setSource("body", "doc2", "@timestamp", "2025-12-01").setRefreshPolicy(IMMEDIATE).get(); + prepareIndex(secondIndexName).setId("6").setSource("body", "doc3", "@timestamp", "2026-01-01").setRefreshPolicy(IMMEDIATE).get(); } @After @@ -123,8 +139,8 @@ public void testPointInTime() { } public void testPointInTimeWithPreFiltering() { - OpenPointInTimeRequest request = new OpenPointInTimeRequest(indexName).keepAlive(TimeValue.timeValueMinutes(10)); - request.indexFilter(simpleQueryStringQuery("doc1")); + OpenPointInTimeRequest request = new OpenPointInTimeRequest(indexName, secondIndexName).keepAlive(TimeValue.timeValueMinutes(10)); + request.indexFilter(new RangeQueryBuilder("@timestamp").gte("2025-07-01")); OpenPointInTimeResponse response = client().execute(TransportOpenPointInTimeAction.TYPE, request).actionGet(); BytesReference pointInTimeId = response.getPointInTimeId(); @@ -134,8 +150,8 @@ public void testPointInTimeWithPreFiltering() { .setPointInTime(new PointInTimeBuilder(pointInTimeId)) .setSize(1) .setPreFilterShardSize(1) - .setQuery(simpleQueryStringQuery("doc1")), - "1" + .setQuery(simpleQueryStringQuery("doc3")), + "6" ); assertMeasurements(List.of(OPEN_PIT_SEARCH_PHASE_METRIC, QUERY_SEARCH_PHASE_METRIC, FETCH_SEARCH_PHASE_METRIC), 1); assertMeasurements(