From 9e3177a49622f38e78df0d8c6152dafe635a0465 Mon Sep 17 00:00:00 2001 From: David Zane Date: Wed, 13 Sep 2023 14:54:27 -0700 Subject: [PATCH] Tests Signed-off-by: David Zane --- .../AbstractSearchAsyncActionTests.java | 15 +- .../search/CreatePitControllerTests.java | 1 + .../action/search/MockSearchPhaseContext.java | 1 + .../search/MultiSearchResponseTests.java | 2 + .../action/search/SearchAsyncActionTests.java | 2 +- .../action/search/SearchRequestTests.java | 4 + .../action/search/SearchResponseTests.java | 21 ++- .../search/SearchTimeProviderTests.java | 175 ++++++++++++++++++ .../search/TransportSearchActionTests.java | 13 +- .../search/GenericSearchExtBuilderTests.java | 1 + .../search/RandomSearchRequestGenerator.java | 3 + 11 files changed, 232 insertions(+), 6 deletions(-) create mode 100644 server/src/test/java/org/opensearch/action/search/SearchTimeProviderTests.java diff --git a/server/src/test/java/org/opensearch/action/search/AbstractSearchAsyncActionTests.java b/server/src/test/java/org/opensearch/action/search/AbstractSearchAsyncActionTests.java index a674678c582fa..1214187ab225e 100644 --- a/server/src/test/java/org/opensearch/action/search/AbstractSearchAsyncActionTests.java +++ b/server/src/test/java/org/opensearch/action/search/AbstractSearchAsyncActionTests.java @@ -680,7 +680,11 @@ private SearchDfsQueryThenFetchAsyncAction createSearchDfsQueryThenFetchAsyncAct ); AtomicReference exception = new AtomicReference<>(); ActionListener listener = ActionListener.wrap(response -> fail("onResponse should not be called"), exception::set); - + TransportSearchAction.SearchTimeProvider timeProvider = new TransportSearchAction.SearchTimeProvider( + 0, + System.nanoTime(), + System::nanoTime + ); return new SearchDfsQueryThenFetchAsyncAction( logger, null, @@ -694,7 +698,7 @@ private SearchDfsQueryThenFetchAsyncAction createSearchDfsQueryThenFetchAsyncAct searchRequest, listener, shardsIter, - null, + timeProvider, null, task, SearchResponse.Clusters.EMPTY, @@ -726,6 +730,11 @@ private SearchQueryThenFetchAsyncAction createSearchQueryThenFetchAsyncAction( ); AtomicReference exception = new AtomicReference<>(); ActionListener listener = ActionListener.wrap(response -> fail("onResponse should not be called"), exception::set); + TransportSearchAction.SearchTimeProvider timeProvider = new TransportSearchAction.SearchTimeProvider( + 0, + System.nanoTime(), + System::nanoTime + ); return new SearchQueryThenFetchAsyncAction( logger, null, @@ -739,7 +748,7 @@ private SearchQueryThenFetchAsyncAction createSearchQueryThenFetchAsyncAction( searchRequest, listener, shardsIter, - null, + timeProvider, null, task, SearchResponse.Clusters.EMPTY, diff --git a/server/src/test/java/org/opensearch/action/search/CreatePitControllerTests.java b/server/src/test/java/org/opensearch/action/search/CreatePitControllerTests.java index 2643aa5b6db01..12a3ffcf84ab7 100644 --- a/server/src/test/java/org/opensearch/action/search/CreatePitControllerTests.java +++ b/server/src/test/java/org/opensearch/action/search/CreatePitControllerTests.java @@ -133,6 +133,7 @@ public void setupData() { 3, 0, 100, + SearchResponse.PhaseTook.NULL, ShardSearchFailure.EMPTY_ARRAY, SearchResponse.Clusters.EMPTY, pitId diff --git a/server/src/test/java/org/opensearch/action/search/MockSearchPhaseContext.java b/server/src/test/java/org/opensearch/action/search/MockSearchPhaseContext.java index b5e1050b968ee..9c514323368a8 100644 --- a/server/src/test/java/org/opensearch/action/search/MockSearchPhaseContext.java +++ b/server/src/test/java/org/opensearch/action/search/MockSearchPhaseContext.java @@ -118,6 +118,7 @@ public void sendSearchResponse(InternalSearchResponse internalSearchResponse, At numSuccess.get(), 0, 0, + SearchResponse.PhaseTook.NULL, failures.toArray(ShardSearchFailure.EMPTY_ARRAY), SearchResponse.Clusters.EMPTY, searchContextId diff --git a/server/src/test/java/org/opensearch/action/search/MultiSearchResponseTests.java b/server/src/test/java/org/opensearch/action/search/MultiSearchResponseTests.java index e26a3cb4731da..325d316d43628 100644 --- a/server/src/test/java/org/opensearch/action/search/MultiSearchResponseTests.java +++ b/server/src/test/java/org/opensearch/action/search/MultiSearchResponseTests.java @@ -68,6 +68,7 @@ protected MultiSearchResponse createTestInstance() { successfulShards, skippedShards, tookInMillis, + SearchResponse.PhaseTook.NULL, ShardSearchFailure.EMPTY_ARRAY, clusters, null @@ -96,6 +97,7 @@ private static MultiSearchResponse createTestInstanceWithFailures() { successfulShards, skippedShards, tookInMillis, + SearchResponse.PhaseTook.NULL, ShardSearchFailure.EMPTY_ARRAY, clusters, null diff --git a/server/src/test/java/org/opensearch/action/search/SearchAsyncActionTests.java b/server/src/test/java/org/opensearch/action/search/SearchAsyncActionTests.java index 4b94b6589c6c8..d530c6b4e0c3f 100644 --- a/server/src/test/java/org/opensearch/action/search/SearchAsyncActionTests.java +++ b/server/src/test/java/org/opensearch/action/search/SearchAsyncActionTests.java @@ -713,7 +713,7 @@ public static class TestSearchResponse extends SearchResponse { final Set queried = new HashSet<>(); TestSearchResponse() { - super(InternalSearchResponse.empty(), null, 0, 0, 0, 0L, ShardSearchFailure.EMPTY_ARRAY, Clusters.EMPTY, null); + super(InternalSearchResponse.empty(), null, 0, 0, 0, 0L, PhaseTook.NULL, ShardSearchFailure.EMPTY_ARRAY, Clusters.EMPTY, null); } } diff --git a/server/src/test/java/org/opensearch/action/search/SearchRequestTests.java b/server/src/test/java/org/opensearch/action/search/SearchRequestTests.java index 25d8c5551880f..6e1d6b0b4157c 100644 --- a/server/src/test/java/org/opensearch/action/search/SearchRequestTests.java +++ b/server/src/test/java/org/opensearch/action/search/SearchRequestTests.java @@ -107,6 +107,7 @@ public void testRandomVersionSerialization() throws IOException { Version version = VersionUtils.randomVersion(random()); SearchRequest deserializedRequest = copyWriteable(searchRequest, namedWriteableRegistry, SearchRequest::new, version); assertEquals(searchRequest.isCcsMinimizeRoundtrips(), deserializedRequest.isCcsMinimizeRoundtrips()); + assertEquals(searchRequest.isPhaseTookQueryParamEnabled(), deserializedRequest.isPhaseTookQueryParamEnabled()); assertEquals(searchRequest.getLocalClusterAlias(), deserializedRequest.getLocalClusterAlias()); assertEquals(searchRequest.getAbsoluteStartMillis(), deserializedRequest.getAbsoluteStartMillis()); assertEquals(searchRequest.isFinalReduce(), deserializedRequest.isFinalReduce()); @@ -244,6 +245,9 @@ private SearchRequest mutate(SearchRequest searchRequest) { ); mutators.add(() -> mutation.source(randomValueOtherThan(searchRequest.source(), this::createSearchSourceBuilder))); mutators.add(() -> mutation.setCcsMinimizeRoundtrips(searchRequest.isCcsMinimizeRoundtrips() == false)); + mutators.add( + () -> mutation.setPhaseTookQueryParamEnabled(searchRequest.isPhaseTookQueryParamEnabled() == SearchRequest.ParamValue.FALSE) + ); mutators.add( () -> mutation.setCancelAfterTimeInterval( searchRequest.getCancelAfterTimeInterval() != null diff --git a/server/src/test/java/org/opensearch/action/search/SearchResponseTests.java b/server/src/test/java/org/opensearch/action/search/SearchResponseTests.java index 097e922147698..b4d07cacc44bf 100644 --- a/server/src/test/java/org/opensearch/action/search/SearchResponseTests.java +++ b/server/src/test/java/org/opensearch/action/search/SearchResponseTests.java @@ -152,6 +152,13 @@ public SearchResponse createTestItem( Boolean terminatedEarly = randomBoolean() ? null : randomBoolean(); int numReducePhases = randomIntBetween(1, 10); long tookInMillis = randomNonNegativeLong(); + SearchResponse.PhaseTook phaseTook = new SearchResponse.PhaseTook( + randomNonNegativeLong(), + randomNonNegativeLong(), + randomNonNegativeLong(), + randomNonNegativeLong(), + randomNonNegativeLong() + ); int totalShards = randomIntBetween(1, Integer.MAX_VALUE); int successfulShards = randomIntBetween(0, totalShards); int skippedShards = randomIntBetween(0, totalShards); @@ -182,6 +189,7 @@ public SearchResponse createTestItem( successfulShards, skippedShards, tookInMillis, + phaseTook, shardSearchFailures, randomBoolean() ? randomClusters() : SearchResponse.Clusters.EMPTY, null @@ -320,6 +328,7 @@ public void testToXContent() { 0, 0, 0, + SearchResponse.PhaseTook.NULL, ShardSearchFailure.EMPTY_ARRAY, SearchResponse.Clusters.EMPTY, null @@ -368,13 +377,23 @@ public void testToXContent() { 0, 0, 0, + new SearchResponse.PhaseTook(0, 0, 50, 25, 0), ShardSearchFailure.EMPTY_ARRAY, - new SearchResponse.Clusters(5, 3, 2) + new SearchResponse.Clusters(5, 3, 2), + null ); StringBuilder expectedString = new StringBuilder(); expectedString.append("{"); { expectedString.append("\"took\":0,"); + expectedString.append("\"phase_took\":"); + { + expectedString.append("{\"dfs_prequery\":0,"); + expectedString.append("\"can_match\":0,"); + expectedString.append("\"query\":50,"); + expectedString.append("\"fetch\":25,"); + expectedString.append("\"expand_search\":0},"); + } expectedString.append("\"timed_out\":false,"); expectedString.append("\"_shards\":"); { diff --git a/server/src/test/java/org/opensearch/action/search/SearchTimeProviderTests.java b/server/src/test/java/org/opensearch/action/search/SearchTimeProviderTests.java new file mode 100644 index 0000000000000..aa5c228a67596 --- /dev/null +++ b/server/src/test/java/org/opensearch/action/search/SearchTimeProviderTests.java @@ -0,0 +1,175 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.action.search; + +import org.opensearch.test.OpenSearchTestCase; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Phaser; + +public class SearchTimeProviderTests extends OpenSearchTestCase { + public void testSearchRequestPhaseFailure() { + TransportSearchAction.SearchTimeProvider testRequestStats = new TransportSearchAction.SearchTimeProvider(0, 0, () -> 0); + SearchPhaseContext ctx = new MockSearchPhaseContext(1); + + testRequestStats.onDFSPreQueryPhaseStart(ctx); + testRequestStats.onDFSPreQueryPhaseFailure(ctx); + assertEquals(0, testRequestStats.getDFSPreQueryTotal()); + + testRequestStats.onCanMatchPhaseStart(ctx); + testRequestStats.onCanMatchPhaseFailure(ctx); + assertEquals(0, testRequestStats.getCanMatchTotal()); + + testRequestStats.onQueryPhaseStart(ctx); + testRequestStats.onQueryPhaseFailure(ctx); + assertEquals(0, testRequestStats.getQueryTotal()); + + testRequestStats.onFetchPhaseStart(ctx); + testRequestStats.onFetchPhaseFailure(ctx); + assertEquals(0, testRequestStats.getFetchTotal()); + + testRequestStats.onExpandSearchPhaseStart(ctx); + testRequestStats.onExpandSearchPhaseFailure(ctx); + assertEquals(0, testRequestStats.getExpandSearchTotal()); + } + + public void testSearchTimeProvider() { + TransportSearchAction.SearchTimeProvider testRequestStats = new TransportSearchAction.SearchTimeProvider(0, 0, () -> 0); + + SearchPhaseContext ctx = new MockSearchPhaseContext(1); + long tookTime = randomIntBetween(1, 10); + + testRequestStats.onDFSPreQueryPhaseStart(ctx); + testRequestStats.onDFSPreQueryPhaseEnd(ctx, tookTime); + assertEquals(tookTime, testRequestStats.getDFSPreQueryTotal()); + + testRequestStats.onCanMatchPhaseStart(ctx); + testRequestStats.onCanMatchPhaseEnd(ctx, tookTime); + assertEquals(tookTime, testRequestStats.getCanMatchTotal()); + + testRequestStats.onQueryPhaseStart(ctx); + testRequestStats.onQueryPhaseEnd(ctx, tookTime); + assertEquals(tookTime, testRequestStats.getQueryTotal()); + + testRequestStats.onFetchPhaseStart(ctx); + testRequestStats.onFetchPhaseEnd(ctx, tookTime); + testRequestStats.onFetchPhaseEnd(ctx, 10); + assertEquals(tookTime + 10, testRequestStats.getFetchTotal()); + + testRequestStats.onExpandSearchPhaseStart(ctx); + testRequestStats.onExpandSearchPhaseEnd(ctx, tookTime); + testRequestStats.onExpandSearchPhaseEnd(ctx, tookTime); + assertEquals(2 * tookTime, testRequestStats.getExpandSearchTotal()); + } + + public void testSearchTimeProviderOnDFSPreQueryEndConcurrently() throws InterruptedException { + TransportSearchAction.SearchTimeProvider testRequestStats = new TransportSearchAction.SearchTimeProvider(0, 0, () -> 0); + SearchPhaseContext ctx = new MockSearchPhaseContext(1); + int numTasks = randomIntBetween(5, 50); + long tookTime = randomIntBetween(1, 10); + Thread[] threads = new Thread[numTasks]; + Phaser phaser = new Phaser(numTasks + 1); + CountDownLatch countDownLatch = new CountDownLatch(numTasks); + for (int i = 0; i < numTasks; i++) { + threads[i] = new Thread(() -> { + phaser.arriveAndAwaitAdvance(); + testRequestStats.onDFSPreQueryPhaseEnd(ctx, tookTime); + countDownLatch.countDown(); + }); + threads[i].start(); + } + phaser.arriveAndAwaitAdvance(); + countDownLatch.await(); + assertEquals(tookTime * numTasks, testRequestStats.getDFSPreQueryTotal()); + } + + public void testSearchTimeProviderOnCanMatchQueryEndConcurrently() throws InterruptedException { + TransportSearchAction.SearchTimeProvider testRequestStats = new TransportSearchAction.SearchTimeProvider(0, 0, () -> 0); + SearchPhaseContext ctx = new MockSearchPhaseContext(1); + int numTasks = randomIntBetween(5, 50); + long tookTime = randomIntBetween(1, 10); + Thread[] threads = new Thread[numTasks]; + Phaser phaser = new Phaser(numTasks + 1); + CountDownLatch countDownLatch = new CountDownLatch(numTasks); + for (int i = 0; i < numTasks; i++) { + threads[i] = new Thread(() -> { + phaser.arriveAndAwaitAdvance(); + testRequestStats.onCanMatchPhaseEnd(ctx, tookTime); + countDownLatch.countDown(); + }); + threads[i].start(); + } + phaser.arriveAndAwaitAdvance(); + countDownLatch.await(); + assertEquals(tookTime * numTasks, testRequestStats.getCanMatchTotal()); + } + + public void testSearchTimeProviderOnQueryEndConcurrently() throws InterruptedException { + TransportSearchAction.SearchTimeProvider testRequestStats = new TransportSearchAction.SearchTimeProvider(0, 0, () -> 0); + SearchPhaseContext ctx = new MockSearchPhaseContext(1); + int numTasks = randomIntBetween(5, 50); + long tookTime = randomIntBetween(1, 10); + Thread[] threads = new Thread[numTasks]; + Phaser phaser = new Phaser(numTasks + 1); + CountDownLatch countDownLatch = new CountDownLatch(numTasks); + for (int i = 0; i < numTasks; i++) { + threads[i] = new Thread(() -> { + phaser.arriveAndAwaitAdvance(); + testRequestStats.onQueryPhaseEnd(ctx, tookTime); + countDownLatch.countDown(); + }); + threads[i].start(); + } + phaser.arriveAndAwaitAdvance(); + countDownLatch.await(); + assertEquals(tookTime * numTasks, testRequestStats.getQueryTotal()); + } + + public void testSearchTimeProviderOnFetchEndConcurrently() throws InterruptedException { + TransportSearchAction.SearchTimeProvider testRequestStats = new TransportSearchAction.SearchTimeProvider(0, 0, () -> 0); + SearchPhaseContext ctx = new MockSearchPhaseContext(1); + int numTasks = randomIntBetween(5, 50); + long tookTime = randomIntBetween(1, 10); + Thread[] threads = new Thread[numTasks]; + Phaser phaser = new Phaser(numTasks + 1); + CountDownLatch countDownLatch = new CountDownLatch(numTasks); + for (int i = 0; i < numTasks; i++) { + threads[i] = new Thread(() -> { + phaser.arriveAndAwaitAdvance(); + testRequestStats.onFetchPhaseEnd(ctx, tookTime); + countDownLatch.countDown(); + }); + threads[i].start(); + } + phaser.arriveAndAwaitAdvance(); + countDownLatch.await(); + assertEquals(tookTime * numTasks, testRequestStats.getFetchTotal()); + } + + public void testSearchTimeProviderOnExpandSearchEndConcurrently() throws InterruptedException { + TransportSearchAction.SearchTimeProvider testRequestStats = new TransportSearchAction.SearchTimeProvider(0, 0, () -> 0); + SearchPhaseContext ctx = new MockSearchPhaseContext(1); + int numTasks = randomIntBetween(5, 50); + long tookTime = randomIntBetween(1, 10); + Thread[] threads = new Thread[numTasks]; + Phaser phaser = new Phaser(numTasks + 1); + CountDownLatch countDownLatch = new CountDownLatch(numTasks); + for (int i = 0; i < numTasks; i++) { + threads[i] = new Thread(() -> { + phaser.arriveAndAwaitAdvance(); + testRequestStats.onExpandSearchPhaseEnd(ctx, tookTime); + countDownLatch.countDown(); + }); + threads[i].start(); + } + phaser.arriveAndAwaitAdvance(); + countDownLatch.await(); + assertEquals(tookTime * numTasks, testRequestStats.getExpandSearchTotal()); + } +} diff --git a/server/src/test/java/org/opensearch/action/search/TransportSearchActionTests.java b/server/src/test/java/org/opensearch/action/search/TransportSearchActionTests.java index c4bf8a5d87172..91b1f5a63b033 100644 --- a/server/src/test/java/org/opensearch/action/search/TransportSearchActionTests.java +++ b/server/src/test/java/org/opensearch/action/search/TransportSearchActionTests.java @@ -445,7 +445,18 @@ private static SearchResponse emptySearchResponse() { null, 1 ); - return new SearchResponse(response, null, 1, 1, 0, 100, ShardSearchFailure.EMPTY_ARRAY, SearchResponse.Clusters.EMPTY, null); + return new SearchResponse( + response, + null, + 1, + 1, + 0, + 100, + SearchResponse.PhaseTook.NULL, + ShardSearchFailure.EMPTY_ARRAY, + SearchResponse.Clusters.EMPTY, + null + ); } public void testCCSRemoteReduceMergeFails() throws Exception { diff --git a/server/src/test/java/org/opensearch/search/GenericSearchExtBuilderTests.java b/server/src/test/java/org/opensearch/search/GenericSearchExtBuilderTests.java index 8fb1814962155..2c39c5f2177d9 100644 --- a/server/src/test/java/org/opensearch/search/GenericSearchExtBuilderTests.java +++ b/server/src/test/java/org/opensearch/search/GenericSearchExtBuilderTests.java @@ -264,6 +264,7 @@ public SearchResponse createTestItem( successfulShards, skippedShards, tookInMillis, + SearchResponse.PhaseTook.NULL, shardSearchFailures, randomBoolean() ? randomClusters() : SearchResponse.Clusters.EMPTY, null diff --git a/test/framework/src/main/java/org/opensearch/search/RandomSearchRequestGenerator.java b/test/framework/src/main/java/org/opensearch/search/RandomSearchRequestGenerator.java index b942136e1f1e2..c754a5efc4ca7 100644 --- a/test/framework/src/main/java/org/opensearch/search/RandomSearchRequestGenerator.java +++ b/test/framework/src/main/java/org/opensearch/search/RandomSearchRequestGenerator.java @@ -131,6 +131,9 @@ public static SearchRequest randomSearchRequest(Supplier ra if (randomBoolean()) { searchRequest.setCancelAfterTimeInterval(TimeValue.parseTimeValue(randomTimeValue(), null, "cancel_after_time_interval")); } + if (randomBoolean()) { + searchRequest.setPhaseTookQueryParamEnabled(randomBoolean()); + } return searchRequest; }