Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Search latency tracking - Coordinator node #8386

Merged
merged 61 commits into from
Sep 20, 2023
Merged
Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
cecdd96
CoordinatorStats
buddharajusahil Jun 30, 2023
80ab4c2
Changed version
Jun 30, 2023
e0fc5cb
Applied formatting
Jun 30, 2023
e21f2b6
Fixed NodeIndicesStatsTests bug
Jul 5, 2023
4a12c18
Update Debug_OpenSearch.xml
buddharajusahil Jun 30, 2023
6efcae5
Fixed wildcard imports
Jul 5, 2023
62e608f
Added negative test UT's to AbstractSearchAsyncAction
Jul 6, 2023
261bfa3
Formatting
Jul 18, 2023
bc0670d
Added more revisions
Jul 18, 2023
b81236d
Changed coordinator to request, refactored logic for onPhase end, sta…
Jul 24, 2023
ffa573f
Changed Coordinator to Request in TransportSearchAction
Jul 24, 2023
d015f9b
Removed test statements in TransportSearchAction
Jul 24, 2023
33b9fac
Removed configuration files from pull request
Jul 28, 2023
40660da
Addressed PR Comments related to testing
Jul 31, 2023
b4bb78c
Formatting
Jul 31, 2023
599e080
Fixed bug with instance start map
Aug 2, 2023
8fed670
Minor logic changes
Aug 4, 2023
509d923
Changed constructor logic in AbstractSearchAsyncAction
Aug 5, 2023
f8ae34c
Changed Logic, Minor changes to TransportSearch and AbstractAsync
Aug 8, 2023
298a00f
set flag value to false by default
Aug 8, 2023
b1ad9d5
fixed tracer bug
Aug 8, 2023
bdd1b5e
reverted all Tracerfactory Changes in Node.java
Aug 8, 2023
3e0de15
Removed forbidden api from ASAA Tests
Aug 8, 2023
bb52c34
Formatting
Aug 8, 2023
34b6064
Remove math.random
Aug 9, 2023
0499e44
Fixed Test cases
Aug 9, 2023
1cf98be
Changed IT's to account for flag
Aug 9, 2023
58a0fe9
More integration testing changes
Aug 9, 2023
0be1de7
rerun tests
Aug 9, 2023
a378a4e
Fixed scope bug in SearchStatsIT
Aug 9, 2023
bbf5af9
rerun tests
Aug 9, 2023
b8778b1
java docs
Aug 9, 2023
30563a5
flaky tests
Aug 9, 2023
0637e85
flaky tests
Aug 9, 2023
84ab313
Empty-Commit
Aug 10, 2023
18aeaa8
Merge branch 'main' into CoordinatorStats
Aug 10, 2023
22e2437
Merge branch 'main' into CoordinatorStats
buddharajusahil Aug 10, 2023
18ecdcb
fixed conflict
Aug 10, 2023
fbb604a
Merge branch 'main' into CoordinatorStats
Aug 29, 2023
8751cbd
Addressing comments to use CollectionUtils.isEmpty
sgup432 Aug 29, 2023
afc4a35
Merge pull request #5 from sgup432/CoordinatorStats
sgup432 Aug 29, 2023
9e7baaa
Addressing comments and adding CHANGELOG
sgup432 Sep 11, 2023
3857542
Merge branch 'main' into CoordinatorStats
sgup432 Sep 11, 2023
0e44b49
Merge pull request #6 from sgup432/CoordinatorStats
sgup432 Sep 11, 2023
e7cb97b
Fixing gradle check failure
sgup432 Sep 11, 2023
f44fb3c
Merge pull request #7 from sgup432/CoordinatorStats
sgup432 Sep 11, 2023
0d87cfb
Merge branch 'main' into CoordinatorStats
sgup432 Sep 12, 2023
110ca71
Changing interface template for SearchRequestOperationsListener
sgup432 Sep 18, 2023
b680055
Merge branch 'main' into CoordinatorStats
sgup432 Sep 18, 2023
39f7596
Merge branch 'CoordinatorStats' into CoordinatorStats
sgup432 Sep 18, 2023
4d66914
Merge pull request #8 from sgup432/CoordinatorStats
sgup432 Sep 18, 2023
706d4f5
Using generic enumMap for holding phase stats
sgup432 Sep 19, 2023
b0a2ad3
Merge pull request #9 from sgup432/CoordinatorStats
sgup432 Sep 19, 2023
b678f61
Removing unused variable from SearchRequestStats
sgup432 Sep 19, 2023
e0b2769
Merge pull request #10 from sgup432/CoordinatorStats
sgup432 Sep 19, 2023
8f5b7e5
Fixing broken UT
sgup432 Sep 20, 2023
a4ca6b0
Merge pull request #11 from sgup432/CoordinatorStats
sgup432 Sep 20, 2023
dfb85bd
Fixing javadoc issue
sgup432 Sep 20, 2023
77e6fcf
Merge pull request #12 from sgup432/CoordinatorStats
sgup432 Sep 20, 2023
6ef0cb5
Removing RequestStats file as not needed
sgup432 Sep 20, 2023
a7acab2
Merge pull request #13 from sgup432/CoordinatorStats
sgup432 Sep 20, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@
import java.util.stream.Stream;

import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.lessThanOrEqualTo;
import static org.opensearch.action.search.TransportSearchAction.SEARCH_REQUEST_STATS_ENABLED_KEY;
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
import static org.opensearch.search.aggregations.AggregationBuilders.terms;

Expand All @@ -74,6 +76,7 @@ public void testSearchWithWRRShardRouting() throws IOException {
.put(AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_FORCE_GROUP_SETTING.getKey() + "zone" + ".values", "a,b,c")
.put(AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_ATTRIBUTE_SETTING.getKey(), "zone")
.put("cluster.routing.weighted.fail_open", false)
.put(SEARCH_REQUEST_STATS_ENABLED_KEY, true)
.build();

logger.info("--> starting 6 nodes on different zones");
Expand Down Expand Up @@ -180,12 +183,24 @@ public void testSearchWithWRRShardRouting() throws IOException {
assertFalse(!hitNodes.contains(nodeId));
}
nodeStats = client().admin().cluster().prepareNodesStats().execute().actionGet();
int num = 0;
int coordNumber = 0;

for (NodeStats stat : nodeStats.getNodes()) {
SearchStats.Stats searchStats = stat.getIndices().getSearch().getTotal();
if (searchStats.getRequestStatsLongHolder().queryMetric > 0) {
assertThat(searchStats.getRequestStatsLongHolder().queryTotal, greaterThan(0L));
assertThat(searchStats.getRequestStatsLongHolder().fetchMetric, greaterThan(0L));
assertThat(searchStats.getRequestStatsLongHolder().fetchTotal, greaterThan(0L));
assertThat(searchStats.getRequestStatsLongHolder().expandSearchTotal, greaterThan(0L));
coordNumber += 1;
}
Assert.assertTrue(searchStats.getQueryCount() > 0L);
Assert.assertTrue(searchStats.getFetchCount() > 0L);
num++;
}
assertThat(coordNumber, greaterThan(0));
assertThat(num, greaterThan(0));
}

private Map<String, List<String>> setupCluster(int nodeCountPerAZ, Settings commonSettings) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import java.util.Set;
import java.util.function.Function;

import static org.opensearch.action.search.TransportSearchAction.SEARCH_REQUEST_STATS_ENABLED_KEY;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
import static org.opensearch.index.query.QueryBuilders.matchAllQuery;
Expand All @@ -72,7 +73,7 @@
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;

@OpenSearchIntegTestCase.ClusterScope(minNumDataNodes = 2)
@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, minNumDataNodes = 2)
public class SearchStatsIT extends OpenSearchIntegTestCase {

@Override
Expand Down Expand Up @@ -103,6 +104,11 @@ public void testSimpleStats() throws Exception {
assertThat(numNodes, greaterThanOrEqualTo(2));
final int shardsIdx1 = randomIntBetween(1, 10); // we make sure each node gets at least a single shard...
final int shardsIdx2 = Math.max(numNodes - shardsIdx1, randomIntBetween(1, 10));
client().admin()
.cluster()
.prepareUpdateSettings()
.setPersistentSettings(Settings.builder().put(SEARCH_REQUEST_STATS_ENABLED_KEY, true).build())
.get();
assertThat(numNodes, lessThanOrEqualTo(shardsIdx1 + shardsIdx2));
assertAcked(
prepareCreate("test1").setSettings(
Expand Down Expand Up @@ -165,20 +171,28 @@ public void testSimpleStats() throws Exception {

Set<String> nodeIdsWithIndex = nodeIdsWithIndex("test1", "test2");
int num = 0;
int numOfCoordinators = 0;

for (NodeStats stat : nodeStats.getNodes()) {
Stats total = stat.getIndices().getSearch().getTotal();
if (total.getRequestStatsLongHolder().queryMetric > 0) {
assertEquals(total.getRequestStatsLongHolder().queryTotal, iters);
assertThat(total.getRequestStatsLongHolder().fetchMetric, greaterThan(0L));
assertEquals(total.getRequestStatsLongHolder().fetchTotal, iters);
assertEquals(total.getRequestStatsLongHolder().expandSearchTotal, iters);
numOfCoordinators += 1;
}
if (nodeIdsWithIndex.contains(stat.getNode().getId())) {
assertThat(total.getQueryCount(), greaterThan(0L));
assertThat(total.getQueryTimeInMillis(), greaterThan(0L));
num++;
} else {
assertThat(total.getQueryCount(), equalTo(0L));
assertThat(total.getQueryCount(), greaterThanOrEqualTo(0L));
assertThat(total.getQueryTimeInMillis(), equalTo(0L));
}
}

assertThat(numOfCoordinators, greaterThan(0));
assertThat(num, greaterThan(0));

}

private Set<String> nodeIdsWithIndex(String... indices) {
Expand Down
30 changes: 30 additions & 0 deletions server/src/main/java/org/opensearch/action/RequestStats.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* 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;

import org.opensearch.action.search.SearchRequestStats;

/**
* Request level stats
*
* @opensearch.internal
*/
public final class RequestStats {
msfroh marked this conversation as resolved.
Show resolved Hide resolved

public SearchRequestStats searchRequestStats;

public RequestStats() {
searchRequestStats = new SearchRequestStats();
}

public SearchRequestStats getSearchRequestStats() {
return searchRequestStats;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
package org.opensearch.action.admin.indices.stats;

import org.apache.lucene.store.AlreadyClosedException;
import org.opensearch.action.RequestStats;
import org.opensearch.common.Nullable;
import org.opensearch.core.common.io.stream.StreamInput;
import org.opensearch.core.common.io.stream.StreamOutput;
Expand Down Expand Up @@ -284,6 +285,14 @@
out.writeOptionalWriteable(recoveryStats);
}

// We are adding request stats with a separate setter method since SearchStats was tightly coupled with Shard Search Stats, and all
// nodes won't share the same response in requestStats
public void addRequestStats(RequestStats requestStats) {
if (requestStats.getSearchRequestStats() != null && this.search != null) {
search.setSearchRequestStats(requestStats.getSearchRequestStats());

Check warning on line 292 in server/src/main/java/org/opensearch/action/admin/indices/stats/CommonStats.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/action/admin/indices/stats/CommonStats.java#L292

Added line #L292 was not covered by tests
msfroh marked this conversation as resolved.
Show resolved Hide resolved
}
}

public void add(CommonStats stats) {
if (docs == null) {
if (stats.getDocs() != null) {
Expand Down
Loading