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 a3efec0c8f2f3..e65db96a400d9 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 @@ protected String repositoryType() { @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 c7dd28a1e6bdb..00ac39c0773fe 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 @@ private void runSearchableSnapshotsTest(SearchableSnapshotsTestCaseBody testCase ); 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 @@ protected static void registerRepository(String repository, String type, boolean 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 @@ protected static void deleteSnapshot(String repository, String snapshot, boolean 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 @@ protected static void mountSnapshot( ); } + 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));