Skip to content

Commit

Permalink
Remove explicit SearchResponse references from `org.elasticsearch.sea…
Browse files Browse the repository at this point in the history 
…rch.basic` (#102074)
  • Loading branch information
@iverase
iverase authored Nov 13, 2023
1 parent f017fab commit b3c3cc1
Show file tree
Hide file tree
Showing 7 changed files with 168 additions and 137 deletions.
46 changes: 24 additions & 22 deletions ...er/src/internalClusterTest/java/org/elasticsearch/search/basic/SearchRedStateIndexIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import org.elasticsearch.action.NoShardAvailableActionException;
import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsResponse;
import org.elasticsearch.action.search.SearchPhaseExecutionException;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.search.ShardSearchFailure;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.health.ClusterHealthStatus;
Expand All @@ -27,6 +26,7 @@
import java.util.List;

import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertResponse;
import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.emptyArray;
Expand All @@ -43,15 +43,16 @@ public void testAllowPartialsWithRedState() throws Exception {
final int numShards = cluster().numDataNodes() + 2;
buildRedIndex(numShards);

SearchResponse searchResponse = prepareSearch().setSize(0).setAllowPartialSearchResults(true).get();
assertThat(RestStatus.OK, equalTo(searchResponse.status()));
assertThat("Expect some shards failed", searchResponse.getFailedShards(), allOf(greaterThan(0), lessThanOrEqualTo(numShards)));
assertThat("Expect no shards skipped", searchResponse.getSkippedShards(), equalTo(0));
assertThat("Expect subset of shards successful", searchResponse.getSuccessfulShards(), lessThan(numShards));
assertThat("Expected total shards", searchResponse.getTotalShards(), equalTo(numShards));
for (ShardSearchFailure failure : searchResponse.getShardFailures()) {
assertThat(failure.getCause(), instanceOf(NoShardAvailableActionException.class));
}
assertResponse(prepareSearch().setSize(0).setAllowPartialSearchResults(true), response -> {
assertThat(RestStatus.OK, equalTo(response.status()));
assertThat("Expect some shards failed", response.getFailedShards(), allOf(greaterThan(0), lessThanOrEqualTo(numShards)));
assertThat("Expect no shards skipped", response.getSkippedShards(), equalTo(0));
assertThat("Expect subset of shards successful", response.getSuccessfulShards(), lessThan(numShards));
assertThat("Expected total shards", response.getTotalShards(), equalTo(numShards));
for (ShardSearchFailure failure : response.getShardFailures()) {
assertThat(failure.getCause(), instanceOf(NoShardAvailableActionException.class));
}
});
}

public void testClusterAllowPartialsWithRedState() throws Exception {
Expand All @@ -60,18 +61,19 @@ public void testClusterAllowPartialsWithRedState() throws Exception {

setClusterDefaultAllowPartialResults(true);

SearchResponse searchResponse = prepareSearch().setSize(0).get();
assertThat(RestStatus.OK, equalTo(searchResponse.status()));
assertThat("Expect some shards failed", searchResponse.getFailedShards(), allOf(greaterThan(0), lessThanOrEqualTo(numShards)));
assertThat("Expect no shards skipped", searchResponse.getSkippedShards(), equalTo(0));
assertThat("Expect subset of shards successful", searchResponse.getSuccessfulShards(), lessThan(numShards));
assertThat("Expected total shards", searchResponse.getTotalShards(), equalTo(numShards));
for (ShardSearchFailure failure : searchResponse.getShardFailures()) {
assertThat(failure.getCause(), instanceOf(NoShardAvailableActionException.class));
assertThat(failure.getCause().getStackTrace(), emptyArray());
// We don't write out the entire, repetitive stacktrace in the reason
assertThat(failure.reason(), equalTo("org.elasticsearch.action.NoShardAvailableActionException" + System.lineSeparator()));
}
assertResponse(prepareSearch().setSize(0), response -> {
assertThat(RestStatus.OK, equalTo(response.status()));
assertThat("Expect some shards failed", response.getFailedShards(), allOf(greaterThan(0), lessThanOrEqualTo(numShards)));
assertThat("Expect no shards skipped", response.getSkippedShards(), equalTo(0));
assertThat("Expect subset of shards successful", response.getSuccessfulShards(), lessThan(numShards));
assertThat("Expected total shards", response.getTotalShards(), equalTo(numShards));
for (ShardSearchFailure failure : response.getShardFailures()) {
assertThat(failure.getCause(), instanceOf(NoShardAvailableActionException.class));
assertThat(failure.getCause().getStackTrace(), emptyArray());
// We don't write out the entire, repetitive stacktrace in the reason
assertThat(failure.reason(), equalTo("org.elasticsearch.action.NoShardAvailableActionException" + System.lineSeparator()));
}
});
}

public void testDisallowPartialsWithRedState() throws Exception {
Expand Down
59 changes: 32 additions & 27 deletions ...c/internalClusterTest/java/org/elasticsearch/search/basic/SearchWhileCreatingIndexIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@
package org.elasticsearch.search.basic;

import org.elasticsearch.action.admin.indices.refresh.RefreshResponse;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.client.internal.Client;
import org.elasticsearch.cluster.health.ClusterHealthStatus;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.test.ESIntegTestCase;

import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertResponse;
import static org.hamcrest.Matchers.greaterThanOrEqualTo;

/**
Expand Down Expand Up @@ -66,32 +66,37 @@ private void searchWhileCreatingIndex(boolean createIndex, int numberOfReplicas)
// first, verify that search normal search works
assertHitCount(prepareSearch("test").setQuery(QueryBuilders.termQuery("field", "test")), 1);
Client client = client();
SearchResponse searchResponse = client.prepareSearch("test")
.setPreference(preference + Integer.toString(counter++))
.setQuery(QueryBuilders.termQuery("field", "test"))
.get();
if (searchResponse.getHits().getTotalHits().value != 1) {
refresh();
SearchResponse searchResponseAfterRefresh = client.prepareSearch("test")
.setPreference(preference)
.setQuery(QueryBuilders.termQuery("field", "test"))
.get();
logger.info(
"hits count mismatch on any shard search failed, post explicit refresh hits are {}",
searchResponseAfterRefresh.getHits().getTotalHits().value
);
ensureGreen();
SearchResponse searchResponseAfterGreen = client.prepareSearch("test")
.setPreference(preference)
.setQuery(QueryBuilders.termQuery("field", "test"))
.get();
logger.info(
"hits count mismatch on any shard search failed, post explicit wait for green hits are {}",
searchResponseAfterGreen.getHits().getTotalHits().value
);
assertHitCount(searchResponse, 1);
}
assertHitCount(searchResponse, 1);
assertResponse(
client.prepareSearch("test")
.setPreference(preference + Integer.toString(counter++))
.setQuery(QueryBuilders.termQuery("field", "test")),
searchResponse -> {
if (searchResponse.getHits().getTotalHits().value != 1) {
refresh();
assertResponse(
client.prepareSearch("test").setPreference(preference).setQuery(QueryBuilders.termQuery("field", "test")),
searchResponseAfterRefresh -> {
logger.info(
"hits count mismatch on any shard search failed, post explicit refresh hits are {}",
searchResponseAfterRefresh.getHits().getTotalHits().value
);
ensureGreen();
assertResponse(
client.prepareSearch("test")
.setPreference(preference)
.setQuery(QueryBuilders.termQuery("field", "test")),
searchResponseAfterGreen -> logger.info(
"hits count mismatch on any shard search failed, post explicit wait for green hits are {}",
searchResponseAfterGreen.getHits().getTotalHits().value
)
);
}
);
assertHitCount(searchResponse, 1);
}
assertHitCount(searchResponse, 1);
}
);
status = clusterAdmin().prepareHealth("test").get().getStatus();
internalCluster().ensureAtLeastNumDataNodes(numberOfReplicas + 1);
}
Expand Down
53 changes: 27 additions & 26 deletions .../src/internalClusterTest/java/org/elasticsearch/search/basic/SearchWhileRelocatingIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse;
import org.elasticsearch.action.index.IndexRequestBuilder;
import org.elasticsearch.action.search.SearchPhaseExecutionException;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.common.Priority;
import org.elasticsearch.search.SearchHits;
import org.elasticsearch.test.ESIntegTestCase;
Expand All @@ -25,6 +24,7 @@

import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoTimeout;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertResponse;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.formatShardStatus;
import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;
import static org.hamcrest.Matchers.equalTo;
Expand Down Expand Up @@ -74,33 +74,34 @@ private void testSearchAndRelocateConcurrently(final int numberOfReplicas) throw
public void run() {
try {
while (stop.get() == false) {
SearchResponse sr = prepareSearch().setSize(numDocs).get();
if (sr.getHits().getTotalHits().value != numDocs) {
// if we did not search all shards but had no serious failures that is potentially fine
// if only the hit-count is wrong. this can happen if the cluster-state is behind when the
// request comes in. It's a small window but a known limitation.
if (sr.getTotalShards() != sr.getSuccessfulShards()
&& Stream.of(sr.getShardFailures())
.allMatch(ssf -> ssf.getCause() instanceof NoShardAvailableActionException)) {
nonCriticalExceptions.add(
"Count is "
+ sr.getHits().getTotalHits().value
+ " but "
+ numDocs
+ " was expected. "
+ formatShardStatus(sr)
);
} else {
assertHitCount(sr, numDocs);
assertResponse(prepareSearch().setSize(numDocs), response -> {
if (response.getHits().getTotalHits().value != numDocs) {
// if we did not search all shards but had no serious failures that is potentially fine
// if only the hit-count is wrong. this can happen if the cluster-state is behind when the
// request comes in. It's a small window but a known limitation.
if (response.getTotalShards() != response.getSuccessfulShards()
&& Stream.of(response.getShardFailures())
.allMatch(ssf -> ssf.getCause() instanceof NoShardAvailableActionException)) {
nonCriticalExceptions.add(
"Count is "
+ response.getHits().getTotalHits().value
+ " but "
+ numDocs
+ " was expected. "
+ formatShardStatus(response)
);
} else {
assertHitCount(response, numDocs);
}
}
}

final SearchHits sh = sr.getHits();
assertThat(
"Expected hits to be the same size the actual hits array",
sh.getTotalHits().value,
equalTo((long) (sh.getHits().length))
);
final SearchHits sh = response.getHits();
assertThat(
"Expected hits to be the same size the actual hits array",
sh.getTotalHits().value,
equalTo((long) (sh.getHits().length))
);
});
// this is the more critical but that we hit the actual hit array has a different size than the
// actual number of hits.
}
Expand Down
40 changes: 24 additions & 16 deletions ...internalClusterTest/java/org/elasticsearch/search/basic/SearchWithRandomExceptionsIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import org.elasticsearch.action.DocWriteResponse;
import org.elasticsearch.action.admin.indices.refresh.RefreshResponse;
import org.elasticsearch.action.search.SearchPhaseExecutionException;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Setting.Property;
Expand All @@ -41,6 +40,7 @@
import java.util.concurrent.ExecutionException;

import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertResponse;

public class SearchWithRandomExceptionsIT extends ESIntegTestCase {

Expand Down Expand Up @@ -125,28 +125,36 @@ public void testRandomExceptions() throws IOException, InterruptedException, Exe

NumShards test = getNumShards("test");
final int numSearches = scaledRandomIntBetween(100, 200);
final int finalNumCreated = numCreated;
// we don't check anything here really just making sure we don't leave any open files or a broken index behind.
for (int i = 0; i < numSearches; i++) {
try {
int docToQuery = between(0, numDocs - 1);
int expectedResults = added[docToQuery] ? 1 : 0;
logger.info("Searching for [test:{}]", English.intToEnglish(docToQuery));
SearchResponse searchResponse = prepareSearch().setQuery(QueryBuilders.matchQuery("test", English.intToEnglish(docToQuery)))
.setSize(expectedResults)
.get();
logger.info("Successful shards: [{}] numShards: [{}]", searchResponse.getSuccessfulShards(), test.numPrimaries);
if (searchResponse.getSuccessfulShards() == test.numPrimaries && refreshFailed == false) {
assertResultsAndLogOnFailure(expectedResults, searchResponse);
}
assertResponse(
prepareSearch().setQuery(QueryBuilders.matchQuery("test", English.intToEnglish(docToQuery))).setSize(expectedResults),
response -> {
logger.info("Successful shards: [{}] numShards: [{}]", response.getSuccessfulShards(), test.numPrimaries);
if (response.getSuccessfulShards() == test.numPrimaries && refreshFailed == false) {
assertResultsAndLogOnFailure(expectedResults, response);
}
}
);
// check match all
searchResponse = prepareSearch().setQuery(QueryBuilders.matchAllQuery())
.setSize(numCreated)
.addSort("_id", SortOrder.ASC)
.get();
logger.info("Match all Successful shards: [{}] numShards: [{}]", searchResponse.getSuccessfulShards(), test.numPrimaries);
if (searchResponse.getSuccessfulShards() == test.numPrimaries && refreshFailed == false) {
assertResultsAndLogOnFailure(numCreated, searchResponse);
}
assertResponse(
prepareSearch().setQuery(QueryBuilders.matchAllQuery()).setSize(numCreated).addSort("_id", SortOrder.ASC),
response -> {
logger.info(
"Match all Successful shards: [{}] numShards: [{}]",
response.getSuccessfulShards(),
test.numPrimaries
);
if (response.getSuccessfulShards() == test.numPrimaries && refreshFailed == false) {
assertResultsAndLogOnFailure(finalNumCreated, response);
}
}
);

} catch (SearchPhaseExecutionException ex) {
logger.info("expected SearchPhaseException: [{}]", ex.getMessage());
Expand Down
Loading

0 comments on commit b3c3cc1

Please sign in to comment.