From 49f42278377b67cc73509e43f091ede1dfe67f1b Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Sun, 5 Jul 2020 10:49:51 +0200 Subject: [PATCH] Check acknowledged responses in FsSearchableSnapshotsIT (#59021) Despite all my attempts I did not manage to reproduce issues like the ones described in #58961. My guess is that the _mount request got retried at some point but I wasn't able to validate this assumption. Still, the FsSearchableSnapshotsIT can be pretty disk heavy if a small random chunk size and a large number of documents is picked up in the tests. The parent class also does not verify the acknowledged status of some requests. This commit lowers down the chunk size and number of docs in tests (this is extensively tests in unit tests) and also adds assertions on acknowledged responses. Relates #58961 --- .../rest/FsSearchableSnapshotsIT.java | 11 +------ ...stractSearchableSnapshotsRestTestCase.java | 30 ++++++++++++------- 2 files changed, 20 insertions(+), 21 deletions(-) diff --git a/x-pack/plugin/searchable-snapshots/qa/rest/src/test/java/org/elasticsearch/xpack/searchablesnapshots/rest/FsSearchableSnapshotsIT.java b/x-pack/plugin/searchable-snapshots/qa/rest/src/test/java/org/elasticsearch/xpack/searchablesnapshots/rest/FsSearchableSnapshotsIT.java index a3efec0c8f2..e65db96a400 100644 --- a/x-pack/plugin/searchable-snapshots/qa/rest/src/test/java/org/elasticsearch/xpack/searchablesnapshots/rest/FsSearchableSnapshotsIT.java +++ b/x-pack/plugin/searchable-snapshots/qa/rest/src/test/java/org/elasticsearch/xpack/searchablesnapshots/rest/FsSearchableSnapshotsIT.java @@ -6,7 +6,6 @@ package org.elasticsearch.xpack.searchablesnapshots.rest; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.unit.ByteSizeUnit; import org.elasticsearch.repositories.fs.FsRepository; import org.elasticsearch.xpack.searchablesnapshots.AbstractSearchableSnapshotsRestTestCase; @@ -19,14 +18,6 @@ public class FsSearchableSnapshotsIT extends AbstractSearchableSnapshotsRestTest @Override protected Settings repositorySettings() { - final Settings.Builder settings = Settings.builder(); - settings.put("location", System.getProperty("tests.path.repo")); - if (randomBoolean()) { - settings.put("compress", randomBoolean()); - } - if (randomBoolean()) { - settings.put("chunk_size", randomIntBetween(100, 1000), ByteSizeUnit.BYTES); - } - return settings.build(); + return Settings.builder().put("location", System.getProperty("tests.path.repo")).build(); } } diff --git a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/AbstractSearchableSnapshotsRestTestCase.java b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/AbstractSearchableSnapshotsRestTestCase.java index c7dd28a1e6b..00ac39c0773 100644 --- a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/AbstractSearchableSnapshotsRestTestCase.java +++ b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/AbstractSearchableSnapshotsRestTestCase.java @@ -68,7 +68,7 @@ public abstract class AbstractSearchableSnapshotsRestTestCase extends ESRestTest ); ensureGreen(indexName); - final int numDocs = randomIntBetween(1, 10_000); + final int numDocs = randomIntBetween(1, 500); logger.info("indexing [{}] documents", numDocs); final StringBuilder bulkBody = new StringBuilder(); @@ -255,11 +255,7 @@ public abstract class AbstractSearchableSnapshotsRestTestCase extends ESRestTest request.setJsonEntity(Strings.toString(new PutRepositoryRequest(repository).type(type).verify(verify).settings(settings))); final Response response = client().performRequest(request); - assertThat( - "Failed to create repository [" + repository + "] of type [" + type + "]: " + response, - response.getStatusLine().getStatusCode(), - equalTo(RestStatus.OK.getStatus()) - ); + assertAcked("Failed to create repository [" + repository + "] of type [" + type + "]: " + response, response); } protected static void createSnapshot(String repository, String snapshot, boolean waitForCompletion) throws IOException { @@ -278,11 +274,7 @@ public abstract class AbstractSearchableSnapshotsRestTestCase extends ESRestTest final Request request = new Request(HttpDelete.METHOD_NAME, "_snapshot/" + repository + '/' + snapshot); try { final Response response = client().performRequest(request); - assertThat( - "Failed to delete snapshot [" + snapshot + "] in repository [" + repository + "]: " + response, - response.getStatusLine().getStatusCode(), - equalTo(RestStatus.OK.getStatus()) - ); + assertAcked("Failed to delete snapshot [" + snapshot + "] in repository [" + repository + "]: " + response, response); } catch (IOException e) { if (ignoreMissing && e instanceof ResponseException) { Response response = ((ResponseException) e).getResponse(); @@ -324,6 +316,22 @@ public abstract class AbstractSearchableSnapshotsRestTestCase extends ESRestTest ); } + protected static void deleteIndex(String index) throws IOException { + final Response response = client().performRequest(new Request("DELETE", "/" + index)); + assertAcked("Fail to delete index [" + index + ']', response); + } + + private static void assertAcked(String message, Response response) throws IOException { + final int responseStatusCode = response.getStatusLine().getStatusCode(); + assertThat( + message + ": expecting response code [200] but got [" + responseStatusCode + ']', + responseStatusCode, + equalTo(RestStatus.OK.getStatus()) + ); + final Map responseAsMap = responseAsMap(response); + assertThat(message + ": response is not acknowledged", extractValue(responseAsMap, "acknowledged"), equalTo(Boolean.TRUE)); + } + protected static void forceMerge(String index, boolean onlyExpungeDeletes, boolean flush) throws IOException { final Request request = new Request(HttpPost.METHOD_NAME, '/' + index + "/_forcemerge"); request.addParameter("only_expunge_deletes", Boolean.toString(onlyExpungeDeletes));