Skip to content

Commit

Permalink
Fix search response leaks in datastreams and mustache modules (elasti…
Browse files Browse the repository at this point in the history 
…c#103532)

Fixes the remaining test leaks in datastream and mustache. Mustache will
require a separate PR to make the search template response classes
properly ref-counted but we can prepare for that here already.

for elastic#102030
  • Loading branch information
@original-brownbear @navarone-feekery
original-brownbear authored and navarone-feekery committed Dec 21, 2023
1 parent 5519302 commit 04130ad
Show file tree
Hide file tree
Showing 9 changed files with 189 additions and 143 deletions.
63 changes: 41 additions & 22 deletions ...ms/src/internalClusterTest/java/org/elasticsearch/datastreams/DataStreamsSnapshotsIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
import java.util.stream.Collectors;

import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertResponse;
import static org.hamcrest.Matchers.anEmptyMap;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsInAnyOrder;
Expand Down Expand Up @@ -161,9 +162,11 @@ public void testSnapshotAndRestore() throws Exception {
assertEquals(1, restoreSnapshotResponse.getRestoreInfo().successfulShards());

assertEquals(DOCUMENT_SOURCE, client.prepareGet(dsBackingIndexName, id).get().getSourceAsMap());
SearchHit[] hits = client.prepareSearch("ds").get().getHits().getHits();
assertEquals(1, hits.length);
assertEquals(DOCUMENT_SOURCE, hits[0].getSourceAsMap());
assertResponse(client.prepareSearch("ds"), response -> {
SearchHit[] hits = response.getHits().getHits();
assertEquals(1, hits.length);
assertEquals(DOCUMENT_SOURCE, hits[0].getSourceAsMap());
});

GetDataStreamAction.Response ds = client.execute(
GetDataStreamAction.INSTANCE,
Expand Down Expand Up @@ -219,9 +222,11 @@ public void testSnapshotAndRestoreAllDataStreamsInPlace() throws Exception {
assertEquals(1, restoreSnapshotResponse.getRestoreInfo().successfulShards());

assertEquals(DOCUMENT_SOURCE, client.prepareGet(dsBackingIndexName, id).get().getSourceAsMap());
SearchHit[] hits = client.prepareSearch("ds").get().getHits().getHits();
assertEquals(1, hits.length);
assertEquals(DOCUMENT_SOURCE, hits[0].getSourceAsMap());
assertResponse(client.prepareSearch("ds"), response -> {
SearchHit[] hits = response.getHits().getHits();
assertEquals(1, hits.length);
assertEquals(DOCUMENT_SOURCE, hits[0].getSourceAsMap());
});

GetDataStreamAction.Request getDataSteamRequest = new GetDataStreamAction.Request(new String[] { "*" });
GetDataStreamAction.Response ds = client.execute(GetDataStreamAction.INSTANCE, getDataSteamRequest).get();
Expand Down Expand Up @@ -271,9 +276,12 @@ public void testSnapshotAndRestoreInPlace() {
assertEquals(1, restoreSnapshotResponse.getRestoreInfo().successfulShards());

assertEquals(DOCUMENT_SOURCE, client.prepareGet(dsBackingIndexName, id).get().getSourceAsMap());
SearchHit[] hits = client.prepareSearch("ds").get().getHits().getHits();
assertEquals(1, hits.length);
assertEquals(DOCUMENT_SOURCE, hits[0].getSourceAsMap());

assertResponse(client.prepareSearch("ds"), response -> {
SearchHit[] hits = response.getHits().getHits();
assertEquals(1, hits.length);
assertEquals(DOCUMENT_SOURCE, hits[0].getSourceAsMap());
});

GetDataStreamAction.Request getDataSteamRequest = new GetDataStreamAction.Request(new String[] { "ds" });
GetDataStreamAction.Response ds = client.execute(GetDataStreamAction.INSTANCE, getDataSteamRequest).actionGet();
Expand Down Expand Up @@ -347,9 +355,11 @@ public void testSnapshotAndRestoreAllIncludeSpecificDataStream() throws Exceptio
assertEquals(1, restoreSnapshotResponse.getRestoreInfo().successfulShards());

assertEquals(DOCUMENT_SOURCE, client.prepareGet(backingIndexName, idToGet).get().getSourceAsMap());
SearchHit[] hits = client.prepareSearch(backingIndexName).get().getHits().getHits();
assertEquals(1, hits.length);
assertEquals(DOCUMENT_SOURCE, hits[0].getSourceAsMap());
assertResponse(client.prepareSearch(backingIndexName), response -> {
SearchHit[] hits = response.getHits().getHits();
assertEquals(1, hits.length);
assertEquals(DOCUMENT_SOURCE, hits[0].getSourceAsMap());
});

GetDataStreamAction.Response ds = client.execute(
GetDataStreamAction.INSTANCE,
Expand Down Expand Up @@ -396,9 +406,11 @@ public void testSnapshotAndRestoreReplaceAll() throws Exception {
assertEquals(2, restoreSnapshotResponse.getRestoreInfo().successfulShards());

assertEquals(DOCUMENT_SOURCE, client.prepareGet(dsBackingIndexName, id).get().getSourceAsMap());
SearchHit[] hits = client.prepareSearch("ds").get().getHits().getHits();
assertEquals(1, hits.length);
assertEquals(DOCUMENT_SOURCE, hits[0].getSourceAsMap());
assertResponse(client.prepareSearch("ds"), response -> {
SearchHit[] hits = response.getHits().getHits();
assertEquals(1, hits.length);
assertEquals(DOCUMENT_SOURCE, hits[0].getSourceAsMap());
});

GetDataStreamAction.Response ds = client.execute(
GetDataStreamAction.INSTANCE,
Expand Down Expand Up @@ -449,9 +461,11 @@ public void testSnapshotAndRestoreAll() throws Exception {
assertEquals(2, restoreSnapshotResponse.getRestoreInfo().successfulShards());

assertEquals(DOCUMENT_SOURCE, client.prepareGet(dsBackingIndexName, id).get().getSourceAsMap());
SearchHit[] hits = client.prepareSearch("ds").get().getHits().getHits();
assertEquals(1, hits.length);
assertEquals(DOCUMENT_SOURCE, hits[0].getSourceAsMap());
assertResponse(client.prepareSearch("ds"), response -> {
SearchHit[] hits = response.getHits().getHits();
assertEquals(1, hits.length);
assertEquals(DOCUMENT_SOURCE, hits[0].getSourceAsMap());
});

GetDataStreamAction.Response ds = client.execute(
GetDataStreamAction.INSTANCE,
Expand Down Expand Up @@ -505,9 +519,11 @@ public void testSnapshotAndRestoreIncludeAliasesFalse() throws Exception {
assertEquals(2, restoreSnapshotResponse.getRestoreInfo().successfulShards());

assertEquals(DOCUMENT_SOURCE, client.prepareGet(dsBackingIndexName, id).get().getSourceAsMap());
SearchHit[] hits = client.prepareSearch("ds").get().getHits().getHits();
assertEquals(1, hits.length);
assertEquals(DOCUMENT_SOURCE, hits[0].getSourceAsMap());
assertResponse(client.prepareSearch("ds"), response -> {
SearchHit[] hits = response.getHits().getHits();
assertEquals(1, hits.length);
assertEquals(DOCUMENT_SOURCE, hits[0].getSourceAsMap());
});

GetDataStreamAction.Response ds = client.execute(
GetDataStreamAction.INSTANCE,
Expand Down Expand Up @@ -557,7 +573,10 @@ public void testRename() throws Exception {
assertEquals(1, ds.getDataStreams().size());
assertEquals(1, ds.getDataStreams().get(0).getDataStream().getIndices().size());
assertEquals(ds2BackingIndexName, ds.getDataStreams().get(0).getDataStream().getIndices().get(0).getName());
assertEquals(DOCUMENT_SOURCE, client.prepareSearch("ds2").get().getHits().getHits()[0].getSourceAsMap());
assertResponse(
client.prepareSearch("ds2"),
response -> assertEquals(DOCUMENT_SOURCE, response.getHits().getHits()[0].getSourceAsMap())
);
assertEquals(DOCUMENT_SOURCE, client.prepareGet(ds2BackingIndexName, id).get().getSourceAsMap());

GetAliasesResponse getAliasesResponse = client.admin().indices().getAliases(new GetAliasesRequest("my-alias")).actionGet();
Expand Down
43 changes: 24 additions & 19 deletions ...ta-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/TSDBIndexingIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@

import static org.elasticsearch.test.MapMatcher.assertMap;
import static org.elasticsearch.test.MapMatcher.matchesMap;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertResponse;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
Expand Down Expand Up @@ -400,11 +401,12 @@ public void testSkippingShards() throws Exception {
var searchRequest = new SearchRequest("pattern-*");
searchRequest.setPreFilterShardSize(1);
searchRequest.source(matchingRange);
var searchResponse = client().search(searchRequest).actionGet();
ElasticsearchAssertions.assertHitCount(searchResponse, 2);
assertThat(searchResponse.getTotalShards(), equalTo(2));
assertThat(searchResponse.getSkippedShards(), equalTo(0));
assertThat(searchResponse.getSuccessfulShards(), equalTo(2));
assertResponse(client().search(searchRequest), searchResponse -> {
ElasticsearchAssertions.assertHitCount(searchResponse, 2);
assertThat(searchResponse.getTotalShards(), equalTo(2));
assertThat(searchResponse.getSkippedShards(), equalTo(0));
assertThat(searchResponse.getSuccessfulShards(), equalTo(2));
});
}
{
var nonMatchingRange = new SearchSourceBuilder().query(
Expand All @@ -414,11 +416,12 @@ public void testSkippingShards() throws Exception {
var searchRequest = new SearchRequest("pattern-*");
searchRequest.setPreFilterShardSize(1);
searchRequest.source(nonMatchingRange);
var searchResponse = client().search(searchRequest).actionGet();
ElasticsearchAssertions.assertNoSearchHits(searchResponse);
assertThat(searchResponse.getTotalShards(), equalTo(2));
assertThat(searchResponse.getSkippedShards(), equalTo(1));
assertThat(searchResponse.getSuccessfulShards(), equalTo(2));
assertResponse(client().search(searchRequest), searchResponse -> {
ElasticsearchAssertions.assertNoSearchHits(searchResponse);
assertThat(searchResponse.getTotalShards(), equalTo(2));
assertThat(searchResponse.getSkippedShards(), equalTo(1));
assertThat(searchResponse.getSuccessfulShards(), equalTo(2));
});
}
}

Expand Down Expand Up @@ -536,17 +539,19 @@ public void testTrimId() throws Exception {
);

// Check the search api can synthesize _id
final String idxName = indexName;
var searchRequest = new SearchRequest(dataStreamName);
searchRequest.source().trackTotalHits(true);
var searchResponse = client().search(searchRequest).actionGet();
assertThat(searchResponse.getHits().getTotalHits().value, equalTo((long) numBulkRequests * numDocsPerBulk));
String id = searchResponse.getHits().getHits()[0].getId();
assertThat(id, notNullValue());

// Check that the _id is gettable:
var getResponse = client().get(new GetRequest(indexName).id(id)).actionGet();
assertThat(getResponse.isExists(), is(true));
assertThat(getResponse.getId(), equalTo(id));
assertResponse(client().search(searchRequest), searchResponse -> {
assertThat(searchResponse.getHits().getTotalHits().value, equalTo((long) numBulkRequests * numDocsPerBulk));
String id = searchResponse.getHits().getHits()[0].getId();
assertThat(id, notNullValue());

// Check that the _id is gettable:
var getResponse = client().get(new GetRequest(idxName).id(id)).actionGet();
assertThat(getResponse.isExists(), is(true));
assertThat(getResponse.getId(), equalTo(id));
});
}

static String formatInstant(Instant instant) {
Expand Down
109 changes: 57 additions & 52 deletions ...src/internalClusterTest/java/org/elasticsearch/script/mustache/MultiSearchTemplateIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.util.Map;

import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertResponse;
import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;
import static org.hamcrest.Matchers.arrayWithSize;
import static org.hamcrest.Matchers.containsString;
Expand Down Expand Up @@ -141,42 +142,43 @@ public void testBasic() throws Exception {
search5.setScriptParams(params5);
multiRequest.add(search5);

MultiSearchTemplateResponse response = client().execute(MustachePlugin.MULTI_SEARCH_TEMPLATE_ACTION, multiRequest).get();
assertThat(response.getResponses(), arrayWithSize(5));
assertThat(response.getTook().millis(), greaterThan(0L));

MultiSearchTemplateResponse.Item response1 = response.getResponses()[0];
assertThat(response1.isFailure(), is(false));
SearchTemplateResponse searchTemplateResponse1 = response1.getResponse();
assertThat(searchTemplateResponse1.hasResponse(), is(true));
assertHitCount(searchTemplateResponse1.getResponse(), (numDocs / 2) + (numDocs % 2));
assertThat(searchTemplateResponse1.getSource().utf8ToString(), equalTo("""
{"query":{"match":{"odd":"true"}}}"""));

MultiSearchTemplateResponse.Item response2 = response.getResponses()[1];
assertThat(response2.isFailure(), is(false));
SearchTemplateResponse searchTemplateResponse2 = response2.getResponse();
assertThat(searchTemplateResponse2.hasResponse(), is(false));
assertThat(searchTemplateResponse2.getSource().utf8ToString(), equalTo("""
{"query":{"match_phrase_prefix":{"message":"quick brown f"}}}"""));

MultiSearchTemplateResponse.Item response3 = response.getResponses()[2];
assertThat(response3.isFailure(), is(false));
SearchTemplateResponse searchTemplateResponse3 = response3.getResponse();
assertThat(searchTemplateResponse3.hasResponse(), is(true));
assertHitCount(searchTemplateResponse3.getResponse(), (numDocs / 2));
assertThat(searchTemplateResponse3.getSource().utf8ToString(), equalTo("""
{"query":{"term":{"odd":"false"}}}"""));

MultiSearchTemplateResponse.Item response4 = response.getResponses()[3];
assertThat(response4.isFailure(), is(true));
assertThat(response4.getFailure(), instanceOf(IndexNotFoundException.class));
assertThat(response4.getFailure().getMessage(), equalTo("no such index [unknown]"));

MultiSearchTemplateResponse.Item response5 = response.getResponses()[4];
assertThat(response5.isFailure(), is(true));
assertNull(response5.getResponse());
assertThat(response5.getFailure(), instanceOf(XContentParseException.class));
assertResponse(client().execute(MustachePlugin.MULTI_SEARCH_TEMPLATE_ACTION, multiRequest), response -> {
assertThat(response.getResponses(), arrayWithSize(5));
assertThat(response.getTook().millis(), greaterThan(0L));

MultiSearchTemplateResponse.Item response1 = response.getResponses()[0];
assertThat(response1.isFailure(), is(false));
SearchTemplateResponse searchTemplateResponse1 = response1.getResponse();
assertThat(searchTemplateResponse1.hasResponse(), is(true));
assertHitCount(searchTemplateResponse1.getResponse(), (numDocs / 2) + (numDocs % 2));
assertThat(searchTemplateResponse1.getSource().utf8ToString(), equalTo("""
{"query":{"match":{"odd":"true"}}}"""));

MultiSearchTemplateResponse.Item response2 = response.getResponses()[1];
assertThat(response2.isFailure(), is(false));
SearchTemplateResponse searchTemplateResponse2 = response2.getResponse();
assertThat(searchTemplateResponse2.hasResponse(), is(false));
assertThat(searchTemplateResponse2.getSource().utf8ToString(), equalTo("""
{"query":{"match_phrase_prefix":{"message":"quick brown f"}}}"""));

MultiSearchTemplateResponse.Item response3 = response.getResponses()[2];
assertThat(response3.isFailure(), is(false));
SearchTemplateResponse searchTemplateResponse3 = response3.getResponse();
assertThat(searchTemplateResponse3.hasResponse(), is(true));
assertHitCount(searchTemplateResponse3.getResponse(), (numDocs / 2));
assertThat(searchTemplateResponse3.getSource().utf8ToString(), equalTo("""
{"query":{"term":{"odd":"false"}}}"""));

MultiSearchTemplateResponse.Item response4 = response.getResponses()[3];
assertThat(response4.isFailure(), is(true));
assertThat(response4.getFailure(), instanceOf(IndexNotFoundException.class));
assertThat(response4.getFailure().getMessage(), equalTo("no such index [unknown]"));

MultiSearchTemplateResponse.Item response5 = response.getResponses()[4];
assertThat(response5.isFailure(), is(true));
assertNull(response5.getResponse());
assertThat(response5.getFailure(), instanceOf(XContentParseException.class));
});
}

/**
Expand All @@ -193,21 +195,24 @@ public void testCCSCheckCompatibility() throws Exception {
searchTemplateRequest.setRequest(new SearchRequest());
MultiSearchTemplateRequest request = new MultiSearchTemplateRequest();
request.add(searchTemplateRequest);
MultiSearchTemplateResponse multiSearchTemplateResponse = client().execute(MustachePlugin.MULTI_SEARCH_TEMPLATE_ACTION, request)
.get();
Item response = multiSearchTemplateResponse.getResponses()[0];
assertTrue(response.isFailure());
Exception ex = response.getFailure();
assertThat(ex.getMessage(), containsString("[class org.elasticsearch.action.search.SearchRequest] is not compatible with version"));
assertThat(ex.getMessage(), containsString("'search.check_ccs_compatibility' setting is enabled."));

String expectedCause = Strings.format(
"[fail_before_current_version] was released first in version %s, failed compatibility "
+ "check trying to send it to node with version %s",
FailBeforeCurrentVersionQueryBuilder.FUTURE_VERSION,
TransportVersions.MINIMUM_CCS_VERSION
);
String actualCause = ex.getCause().getMessage();
assertEquals(expectedCause, actualCause);
assertResponse(client().execute(MustachePlugin.MULTI_SEARCH_TEMPLATE_ACTION, request), multiSearchTemplateResponse -> {
Item response = multiSearchTemplateResponse.getResponses()[0];
assertTrue(response.isFailure());
Exception ex = response.getFailure();
assertThat(
ex.getMessage(),
containsString("[class org.elasticsearch.action.search.SearchRequest] is not compatible with version")
);
assertThat(ex.getMessage(), containsString("'search.check_ccs_compatibility' setting is enabled."));

String expectedCause = Strings.format(
"[fail_before_current_version] was released first in version %s, failed compatibility "
+ "check trying to send it to node with version %s",
FailBeforeCurrentVersionQueryBuilder.FUTURE_VERSION,
TransportVersions.MINIMUM_CCS_VERSION
);
String actualCause = ex.getCause().getMessage();
assertEquals(expectedCause, actualCause);
});
}
}
Loading

0 comments on commit 04130ad

Please sign in to comment.