From 307dbc6b5efe3634ad1d79e0d399147bb6498318 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Thu, 24 Jun 2021 16:58:33 +0200 Subject: [PATCH] Flatten Get Snapshots Response (#74451) This PR returns the get snapshots API to the 7.x format (and transport client behavior) and enhances it for requests that ask for multiple repositories. The changes for requests that target multiple repositories are: * Add `repository` field to `SnapshotInfo` and REST response * Add `failures` map alongside `snapshots` list instead of returning just an exception response as done for single repo requests * Pagination now works across repositories instead of being per repository for multi-repository requests closes #69108 closes #43462 --- .../org/elasticsearch/client/SnapshotIT.java | 59 +++--- .../apis/create-snapshot-api.asciidoc | 1 + .../apis/get-snapshot-api.asciidoc | 8 +- .../url/URLSnapshotRestoreIT.java | 3 - .../rest-api-spec/api/snapshot.get.json | 4 + .../test/snapshot.get/10_basic.yml | 38 ++++ .../SharedClusterSnapshotRestoreIT.java | 1 - .../snapshots/SnapshotStatusApisIT.java | 194 ++++++++++++++++-- .../create/CreateSnapshotRequestBuilder.java | 12 ++ .../snapshots/get/GetSnapshotsRequest.java | 14 +- .../client/ClusterAdminClient.java | 6 +- .../client/support/AbstractClient.java | 4 +- .../AbstractThirdPartyRepositoryTestCase.java | 3 +- .../ESBlobStoreRepositoryIntegTestCase.java | 2 +- .../AbstractSnapshotIntegTestCase.java | 47 ++++- .../xpack/ilm/CCRIndexLifecycleIT.java | 4 +- .../xpack/TimeSeriesRestDriver.java | 1 + .../actions/SearchableSnapshotActionIT.java | 8 +- .../xpack/slm/SnapshotLifecycleRestIT.java | 26 +-- .../slm/SLMSnapshotBlockingIntegTests.java | 4 +- .../xpack/ilm/IndexLifecycle.java | 2 +- .../xpack/slm/SnapshotRetentionTask.java | 79 +++---- .../slm/SnapshotRetentionServiceTests.java | 2 +- .../xpack/slm/SnapshotRetentionTaskTests.java | 20 +- .../authz/SnapshotUserRoleIntegTests.java | 3 +- 25 files changed, 392 insertions(+), 153 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/SnapshotIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/SnapshotIT.java index 29274b39165df..80563d86b8cbd 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/SnapshotIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/SnapshotIT.java @@ -33,22 +33,23 @@ import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.repositories.fs.FsRepository; import org.elasticsearch.rest.RestStatus; +import org.elasticsearch.snapshots.AbstractSnapshotIntegTestCase; import org.elasticsearch.snapshots.RestoreInfo; import org.elasticsearch.snapshots.SnapshotInfo; +import org.mockito.internal.util.collections.Sets; import java.io.IOException; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.stream.Collectors; import static org.elasticsearch.snapshots.SnapshotsService.NO_FEATURE_STATES_VALUE; import static org.elasticsearch.tasks.TaskResultsService.TASKS_FEATURE_NAME; -import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; public class SnapshotIT extends ESRestHighLevelClientTestCase { @@ -177,50 +178,54 @@ public void testCreateSnapshot() throws Exception { } public void testGetSnapshots() throws IOException { - String repository = "test_repository"; + String repository1 = "test_repository1"; + String repository2 = "test_repository2"; String snapshot1 = "test_snapshot1"; String snapshot2 = "test_snapshot2"; - AcknowledgedResponse putRepositoryResponse = createTestRepository(repository, FsRepository.TYPE, "{\"location\": \".\"}"); + AcknowledgedResponse putRepositoryResponse = + createTestRepository(repository1, FsRepository.TYPE, "{\"location\": \"loc1\"}"); assertTrue(putRepositoryResponse.isAcknowledged()); - CreateSnapshotRequest createSnapshotRequest1 = new CreateSnapshotRequest(repository, snapshot1); + AcknowledgedResponse putRepositoryResponse2 = + createTestRepository(repository2, FsRepository.TYPE, "{\"location\": \"loc2\"}"); + assertTrue(putRepositoryResponse2.isAcknowledged()); + + CreateSnapshotRequest createSnapshotRequest1 = new CreateSnapshotRequest(repository1, snapshot1); createSnapshotRequest1.waitForCompletion(true); CreateSnapshotResponse putSnapshotResponse1 = createTestSnapshot(createSnapshotRequest1); - CreateSnapshotRequest createSnapshotRequest2 = new CreateSnapshotRequest(repository, snapshot2); + CreateSnapshotRequest createSnapshotRequest2 = new CreateSnapshotRequest(repository2, snapshot2); createSnapshotRequest2.waitForCompletion(true); - Map originalMetadata = randomUserMetadata(); + Map originalMetadata = AbstractSnapshotIntegTestCase.randomUserMetadata(); createSnapshotRequest2.userMetadata(originalMetadata); CreateSnapshotResponse putSnapshotResponse2 = createTestSnapshot(createSnapshotRequest2); // check that the request went ok without parsing JSON here. When using the high level client, check acknowledgement instead. assertEquals(RestStatus.OK, putSnapshotResponse1.status()); assertEquals(RestStatus.OK, putSnapshotResponse2.status()); - GetSnapshotsRequest request; - if (randomBoolean()) { - request = new GetSnapshotsRequest(repository); - } else if (randomBoolean()) { - request = new GetSnapshotsRequest(repository, new String[] {"_all"}); + GetSnapshotsRequest request = new GetSnapshotsRequest( + randomFrom(new String[]{"_all"}, new String[]{"*"}, new String[]{repository1, repository2}), + randomFrom(new String[]{"_all"}, new String[]{"*"}, new String[]{snapshot1, snapshot2}) + ); + request.ignoreUnavailable(true); - } else { - request = new GetSnapshotsRequest(repository, new String[] {snapshot1, snapshot2}); - } GetSnapshotsResponse response = execute(request, highLevelClient().snapshot()::get, highLevelClient().snapshot()::getAsync); - assertEquals(2, response.getSnapshots().size()); - assertThat(response.getSnapshots().stream().map((s) -> s.snapshotId().getName()).collect(Collectors.toList()), - contains("test_snapshot1", "test_snapshot2")); - Optional> returnedMetadata = response.getSnapshots().stream() - .filter(s -> s.snapshotId().getName().equals("test_snapshot2")) - .findFirst() - .map(SnapshotInfo::userMetadata); - if (returnedMetadata.isPresent()) { - assertEquals(originalMetadata, returnedMetadata.get()); - } else { - assertNull("retrieved metadata is null, expected non-null metadata", originalMetadata); - } + assertThat(response.isFailed(), is(false)); + assertEquals( + Sets.newSet(repository1, repository2), + response.getSnapshots().stream().map(SnapshotInfo::repository).collect(Collectors.toSet()) + ); + + assertThat(response.getSnapshots(), hasSize(2)); + assertThat(response.getSnapshots().get(0).snapshotId().getName(), equalTo(snapshot1)); + assertThat(response.getSnapshots().get(0).repository(), equalTo(repository1)); + assertThat(response.getSnapshots().get(1).snapshotId().getName(), equalTo(snapshot2)); + assertThat(response.getSnapshots().get(1).userMetadata(), equalTo(originalMetadata)); + assertThat(response.getSnapshots().get(1).repository(), equalTo(repository2)); } + public void testSnapshotsStatus() throws IOException { String testRepository = "test"; String testSnapshot = "snapshot"; diff --git a/docs/reference/snapshot-restore/apis/create-snapshot-api.asciidoc b/docs/reference/snapshot-restore/apis/create-snapshot-api.asciidoc index 9f13c4b25549f..3543c2c5bd643 100644 --- a/docs/reference/snapshot-restore/apis/create-snapshot-api.asciidoc +++ b/docs/reference/snapshot-restore/apis/create-snapshot-api.asciidoc @@ -176,6 +176,7 @@ The API returns the following response: "snapshot": { "snapshot": "snapshot_2", "uuid": "vdRctLCxSketdKb54xw67g", + "repository": "my_repository", "version_id": , "version": , "indices": [], diff --git a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc index 2516eb7772130..4ed67e3cc14d6 100644 --- a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc +++ b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc @@ -59,7 +59,8 @@ Use the get snapshot API to return information about one or more snapshots, incl ``:: (Required, string) -Snapshot repository name used to limit the request. +Comma-separated list of snapshot repository names used to limit the request. +Wildcard (`*`) expressions are supported. + To get information about all snapshot repositories registered in the cluster, omit this parameter or use `*` or `_all`. @@ -301,6 +302,7 @@ The API returns the following response: { "snapshot": "snapshot_2", "uuid": "vdRctLCxSketdKb54xw67g", + "repository": "my_repository", "version_id": , "version": , "indices": [], @@ -310,7 +312,7 @@ The API returns the following response: "state": "SUCCESS", "start_time": "2020-07-06T21:55:18.129Z", "start_time_in_millis": 1593093628850, - "end_time": "2020-07-06T21:55:18.876Z", + "end_time": "2020-07-06T21:55:18.129Z", "end_time_in_millis": 1593094752018, "duration_in_millis": 0, "failures": [], @@ -328,7 +330,7 @@ The API returns the following response: // TESTRESPONSE[s/"version": /"version": $body.snapshots.0.version/] // TESTRESPONSE[s/"start_time": "2020-07-06T21:55:18.129Z"/"start_time": $body.snapshots.0.start_time/] // TESTRESPONSE[s/"start_time_in_millis": 1593093628850/"start_time_in_millis": $body.snapshots.0.start_time_in_millis/] -// TESTRESPONSE[s/"end_time": "2020-07-06T21:55:18.876Z"/"end_time": $body.snapshots.0.end_time/] +// TESTRESPONSE[s/"end_time": "2020-07-06T21:55:18.129Z"/"end_time": $body.snapshots.0.end_time/] // TESTRESPONSE[s/"end_time_in_millis": 1593094752018/"end_time_in_millis": $body.snapshots.0.end_time_in_millis/] // TESTRESPONSE[s/"duration_in_millis": 0/"duration_in_millis": $body.snapshots.0.duration_in_millis/] diff --git a/modules/repository-url/src/internalClusterTest/java/org/elasticsearch/repositories/url/URLSnapshotRestoreIT.java b/modules/repository-url/src/internalClusterTest/java/org/elasticsearch/repositories/url/URLSnapshotRestoreIT.java index 7f7016e007d2d..b2ed4f744d41d 100644 --- a/modules/repository-url/src/internalClusterTest/java/org/elasticsearch/repositories/url/URLSnapshotRestoreIT.java +++ b/modules/repository-url/src/internalClusterTest/java/org/elasticsearch/repositories/url/URLSnapshotRestoreIT.java @@ -28,7 +28,6 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; -import static org.hamcrest.Matchers.notNullValue; @ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST) public class URLSnapshotRestoreIT extends ESIntegTestCase { @@ -105,7 +104,6 @@ public void testUrlRepository() throws Exception { logger.info("--> list available shapshots"); GetSnapshotsResponse getSnapshotsResponse = client.admin().cluster().prepareGetSnapshots("url-repo").get(); - assertThat(getSnapshotsResponse.getSnapshots(), notNullValue()); assertThat(getSnapshotsResponse.getSnapshots().size(), equalTo(1)); logger.info("--> delete snapshot"); @@ -114,7 +112,6 @@ public void testUrlRepository() throws Exception { logger.info("--> list available shapshot again, no snapshots should be returned"); getSnapshotsResponse = client.admin().cluster().prepareGetSnapshots("url-repo").get(); - assertThat(getSnapshotsResponse.getSnapshots(), notNullValue()); assertThat(getSnapshotsResponse.getSnapshots().size(), equalTo(0)); } } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.get.json b/rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.get.json index fd338fe2511fc..01387918e5278 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.get.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.get.json @@ -42,6 +42,10 @@ "type":"boolean", "description":"Whether to include details of each index in the snapshot, if those details are available. Defaults to false." }, + "include_repository":{ + "type":"boolean", + "description":"Whether to include the repository name in the snapshot info. Defaults to true." + }, "verbose":{ "type":"boolean", "description":"Whether to show verbose snapshot info or only show the basic info found in the repository index blob" diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/snapshot.get/10_basic.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/snapshot.get/10_basic.yml index 1af795e861614..72a53b0f95cf9 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/snapshot.get/10_basic.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/snapshot.get/10_basic.yml @@ -186,6 +186,7 @@ setup: - match: { snapshots.0.state: SUCCESS } - match: { snapshots.0.metadata.taken_by: test } - match: { snapshots.0.metadata.foo.bar: baz } + - is_false: snapshots.0.index_details - do: snapshot.delete: @@ -231,3 +232,40 @@ setup: snapshot.delete: repository: test_repo_get_1 snapshot: test_snapshot_with_index_details + +--- +"Get snapshot info without repository names": + - skip: + version: " - 7.13.99" + reason: "7.14 changes get snapshots response format" + + - do: + indices.create: + index: test_index + body: + settings: + number_of_shards: 1 + number_of_replicas: 0 + + - do: + snapshot.create: + repository: test_repo_get_1 + snapshot: test_snapshot_no_repo_name + wait_for_completion: true + + - do: + snapshot.get: + repository: test_repo_get_1 + snapshot: test_snapshot_no_repo_name + include_repository: false + human: true + + - is_true: snapshots + - match: { snapshots.0.snapshot: test_snapshot_no_repo_name } + - match: { snapshots.0.state: SUCCESS } + - is_false: snapshots.0.repository + + - do: + snapshot.delete: + repository: test_repo_get_1 + snapshot: test_snapshot_no_repo_name diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java index 18ccd3e0c053e..8fe6b231126c0 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java @@ -1071,7 +1071,6 @@ public void testReadonlyRepository() throws Exception { logger.info("--> list available shapshots"); GetSnapshotsResponse getSnapshotsResponse = client.admin().cluster().prepareGetSnapshots("readonly-repo").get(); - assertThat(getSnapshotsResponse.getSnapshots(), notNullValue()); assertThat(getSnapshotsResponse.getSnapshots().size(), equalTo(1)); logger.info("--> try deleting snapshot"); diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SnapshotStatusApisIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SnapshotStatusApisIT.java index 180e11dc414a0..860d5544dd803 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SnapshotStatusApisIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SnapshotStatusApisIT.java @@ -10,7 +10,6 @@ import org.elasticsearch.Version; import org.elasticsearch.action.ActionFuture; import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse; -import org.elasticsearch.action.admin.cluster.snapshots.get.GetSnapshotsRequest; import org.elasticsearch.action.admin.cluster.snapshots.get.GetSnapshotsResponse; import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotIndexShardStage; import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotIndexShardStatus; @@ -20,6 +19,7 @@ import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotsStatusResponse; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.SnapshotsInProgress; +import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeUnit; @@ -32,8 +32,10 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; @@ -44,6 +46,7 @@ import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; @@ -65,9 +68,9 @@ public void testStatusApiConsistency() throws Exception { logger.info("--> indexing some data"); for (int i = 0; i < 100; i++) { - index("test-idx-1", "_doc", Integer.toString(i), "foo", "bar" + i); - index("test-idx-2", "_doc", Integer.toString(i), "foo", "baz" + i); - index("test-idx-3", "_doc", Integer.toString(i), "foo", "baz" + i); + indexDoc("test-idx-1", Integer.toString(i), "foo", "bar" + i); + indexDoc("test-idx-2", Integer.toString(i), "foo", "baz" + i); + indexDoc("test-idx-3", Integer.toString(i), "foo", "baz" + i); } refresh(); @@ -96,7 +99,7 @@ public void testStatusAPICallInProgressSnapshot() throws Exception { logger.info("--> indexing some data"); for (int i = 0; i < 100; i++) { - index("test-idx-1", "_doc", Integer.toString(i), "foo", "bar" + i); + indexDoc("test-idx-1", Integer.toString(i), "foo", "bar" + i); } refresh(); @@ -129,7 +132,7 @@ public void testExceptionOnMissingSnapBlob() throws IOException { expectThrows( SnapshotMissingException.class, - () -> client().admin().cluster().getSnapshots(new GetSnapshotsRequest("test-repo", new String[] { "test-snap" })).actionGet() + () -> client().admin().cluster().prepareGetSnapshots("test-repo").setSnapshots("test-snap").execute().actionGet() ); } @@ -144,7 +147,7 @@ public void testExceptionOnMissingShardLevelSnapBlob() throws IOException { logger.info("--> indexing some data"); for (int i = 0; i < 100; i++) { - index("test-idx-1", "_doc", Integer.toString(i), "foo", "bar" + i); + indexDoc("test-idx-1", Integer.toString(i), "foo", "bar" + i); } refresh(); @@ -205,9 +208,9 @@ public void testCorrectCountsForDoneShards() throws Exception { final String dataNodeTwo = dataNodes.get(1); createIndex(indexOne, singleShardOneNode(dataNodeOne)); - index(indexOne, "_doc", "some_doc_id", "foo", "bar"); + indexDoc(indexOne, "some_doc_id", "foo", "bar"); createIndex(indexTwo, singleShardOneNode(dataNodeTwo)); - index(indexTwo, "_doc", "some_doc_id", "foo", "bar"); + indexDoc(indexTwo, "some_doc_id", "foo", "bar"); final String repoName = "test-repo"; createRepository(repoName, "mock"); @@ -225,7 +228,6 @@ public void testCorrectCountsForDoneShards() throws Exception { assertBusy(() -> { final SnapshotStatus snapshotStatusOne = getSnapshotStatus(repoName, snapshotOne); - assertThat(snapshotStatusOne.getState(), is(SnapshotsInProgress.State.STARTED)); final SnapshotIndexShardStatus snapshotShardState = stateFirstShard(snapshotStatusOne, indexTwo); assertThat(snapshotShardState.getStage(), is(SnapshotIndexShardStage.DONE)); assertThat(snapshotShardState.getStats().getTotalFileCount(), greaterThan(0)); @@ -250,7 +252,7 @@ public void testCorrectCountsForDoneShards() throws Exception { assertThat(responseSnapshotOne.get().getSnapshotInfo().state(), is(SnapshotState.SUCCESS)); // indexing another document to the second index so it will do writes during the snapshot and we can block on those writes - index(indexTwo, "_doc", "some_other_doc_id", "foo", "other_bar"); + indexDoc(indexTwo, "some_other_doc_id", "foo", "other_bar"); blockDataNode(repoName, dataNodeTwo); @@ -289,7 +291,167 @@ public void testCorrectCountsForDoneShards() throws Exception { assertThat(indexSnapshotDetails.toString(), indexSnapshotDetails.getShardCount(), equalTo(1)); assertThat(indexSnapshotDetails.toString(), indexSnapshotDetails.getMaxSegmentsPerShard(), greaterThanOrEqualTo(1)); assertThat(indexSnapshotDetails.toString(), indexSnapshotDetails.getSize().getBytes(), greaterThan(0L)); + } + + public void testGetSnapshotsNoRepos() { + ensureGreen(); + GetSnapshotsResponse getSnapshotsResponse = clusterAdmin().prepareGetSnapshots(new String[] { "_all" }) + .setSnapshots(randomFrom("_all", "*")) + .get(); + + assertTrue(getSnapshotsResponse.getSnapshots().isEmpty()); + assertTrue(getSnapshotsResponse.getFailures().isEmpty()); + } + + public void testGetSnapshotsMultipleRepos() throws Exception { + final Client client = client(); + + List snapshotList = new ArrayList<>(); + List repoList = new ArrayList<>(); + Map> repo2SnapshotNames = new HashMap<>(); + + logger.info("--> create an index and index some documents"); + final String indexName = "test-idx"; + createIndexWithRandomDocs(indexName, 10); + final int numberOfShards = IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.get( + client.admin().indices().prepareGetSettings(indexName).get().getIndexToSettings().get(indexName) + ); + + for (int repoIndex = 0; repoIndex < randomIntBetween(2, 5); repoIndex++) { + final String repoName = "repo" + repoIndex; + repoList.add(repoName); + final Path repoPath = randomRepoPath(); + logger.info("--> create repository with name " + repoName); + assertAcked( + client.admin() + .cluster() + .preparePutRepository(repoName) + .setType("fs") + .setSettings(Settings.builder().put("location", repoPath).build()) + ); + List snapshotNames = new ArrayList<>(); + repo2SnapshotNames.put(repoName, snapshotNames); + + for (int snapshotIndex = 0; snapshotIndex < randomIntBetween(2, 5); snapshotIndex++) { + final String snapshotName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT); + snapshotList.add(snapshotName); + // Wait for at least 1ms to ensure that snapshots can be ordered by timestamp deterministically + for (final ThreadPool threadPool : internalCluster().getInstances(ThreadPool.class)) { + final long startMillis = threadPool.absoluteTimeInMillis(); + assertBusy(() -> assertThat(threadPool.absoluteTimeInMillis(), greaterThan(startMillis))); + } + logger.info("--> create snapshot with index {} and name {} in repository {}", snapshotIndex, snapshotName, repoName); + CreateSnapshotResponse createSnapshotResponse = client.admin() + .cluster() + .prepareCreateSnapshot(repoName, snapshotName) + .setWaitForCompletion(true) + .setIndices(indexName) + .get(); + final SnapshotInfo snapshotInfo = createSnapshotResponse.getSnapshotInfo(); + assertThat(snapshotInfo.successfulShards(), greaterThan(0)); + assertTrue(snapshotInfo.indexSnapshotDetails().containsKey(indexName)); + final SnapshotInfo.IndexSnapshotDetails indexSnapshotDetails = snapshotInfo.indexSnapshotDetails().get(indexName); + assertThat(indexSnapshotDetails.getShardCount(), equalTo(numberOfShards)); + assertThat(indexSnapshotDetails.getMaxSegmentsPerShard(), greaterThanOrEqualTo(1)); + assertThat(indexSnapshotDetails.getSize().getBytes(), greaterThan(0L)); + snapshotNames.add(snapshotName); + } + } + + logger.info("--> get and verify snapshots"); + GetSnapshotsResponse getSnapshotsResponse = client.admin() + .cluster() + .prepareGetSnapshots(randomFrom(new String[] { "_all" }, new String[] { "repo*" }, repoList.toArray(new String[0]))) + .setSnapshots(randomFrom("_all", "*")) + .get(); + + for (Map.Entry> repo2Names : repo2SnapshotNames.entrySet()) { + String repo = repo2Names.getKey(); + List snapshotNames = repo2Names.getValue(); + List snapshots = getSnapshotsResponse.getSnapshots(); + assertEquals( + snapshotNames, + snapshots.stream().filter(s -> s.repository().equals(repo)).map(s -> s.snapshotId().getName()).collect(Collectors.toList()) + ); + } + + logger.info("--> specify all snapshot names with ignoreUnavailable=false"); + GetSnapshotsResponse getSnapshotsResponse2 = client.admin() + .cluster() + .prepareGetSnapshots(randomFrom("_all", "repo*")) + .setIgnoreUnavailable(false) + .setSnapshots(snapshotList.toArray(new String[0])) + .get(); + + for (String repo : repoList) { + assertThat(getSnapshotsResponse2.getFailures().get(repo), instanceOf(SnapshotMissingException.class)); + } + + logger.info("--> specify all snapshot names with ignoreUnavailable=true"); + GetSnapshotsResponse getSnapshotsResponse3 = client.admin() + .cluster() + .prepareGetSnapshots(randomFrom("_all", "repo*")) + .setIgnoreUnavailable(true) + .setSnapshots(snapshotList.toArray(new String[0])) + .get(); + + for (Map.Entry> repo2Names : repo2SnapshotNames.entrySet()) { + String repo = repo2Names.getKey(); + List snapshotNames = repo2Names.getValue(); + List snapshots = getSnapshotsResponse3.getSnapshots(); + assertEquals( + snapshotNames, + snapshots.stream().filter(s -> s.repository().equals(repo)).map(s -> s.snapshotId().getName()).collect(Collectors.toList()) + ); + } + } + + public void testGetSnapshotsWithSnapshotInProgress() throws Exception { + createRepository("test-repo", "mock", Settings.builder().put("location", randomRepoPath()).put("block_on_data", true)); + + createIndexWithContent("test-idx-1"); + ensureGreen(); + + ActionFuture createSnapshotResponseActionFuture = startFullSnapshot("test-repo", "test-snap"); + + logger.info("--> wait for data nodes to get blocked"); + waitForBlockOnAnyDataNode("test-repo"); + awaitNumberOfSnapshotsInProgress(1); + + GetSnapshotsResponse response1 = client().admin() + .cluster() + .prepareGetSnapshots("test-repo") + .setSnapshots("test-snap") + .setIgnoreUnavailable(true) + .get(); + List snapshotInfoList = response1.getSnapshots(); + assertEquals(1, snapshotInfoList.size()); + assertEquals(SnapshotState.IN_PROGRESS, snapshotInfoList.get(0).state()); + String notExistedSnapshotName = "snapshot_not_exist"; + GetSnapshotsResponse response2 = client().admin() + .cluster() + .prepareGetSnapshots("test-repo") + .setSnapshots(notExistedSnapshotName) + .setIgnoreUnavailable(true) + .get(); + assertEquals(0, response2.getSnapshots().size()); + + expectThrows( + SnapshotMissingException.class, + () -> client().admin() + .cluster() + .prepareGetSnapshots("test-repo") + .setSnapshots(notExistedSnapshotName) + .setIgnoreUnavailable(false) + .execute() + .actionGet() + ); + + logger.info("--> unblock all data nodes"); + unblockAllDataNodes("test-repo"); + + assertSuccessful(createSnapshotResponseActionFuture); } public void testSnapshotStatusOnFailedSnapshot() throws Exception { @@ -303,11 +465,7 @@ public void testSnapshotStatusOnFailedSnapshot() throws Exception { ensureGreen(); indexRandomDocs("test-idx-good", randomIntBetween(1, 5)); - final SnapshotsStatusResponse snapshotsStatusResponse = client().admin() - .cluster() - .prepareSnapshotStatus(repoName) - .setSnapshots(snapshot) - .get(); + final SnapshotsStatusResponse snapshotsStatusResponse = clusterAdmin().prepareSnapshotStatus(repoName).setSnapshots(snapshot).get(); assertEquals(1, snapshotsStatusResponse.getSnapshots().size()); assertEquals(SnapshotsInProgress.State.FAILED, snapshotsStatusResponse.getSnapshots().get(0).getState()); } @@ -366,7 +524,7 @@ public void testGetSnapshotsRequest() throws Exception { assertEquals(1, getSnapshotsResponse.getSnapshots().size()); assertEquals("snap-on-empty-repo", getSnapshotsResponse.getSnapshots().get(0).snapshotId().getName()); unblockNode(repositoryName, initialBlockedNode); // unblock node - admin().cluster().prepareDeleteSnapshot(repositoryName, "snap-on-empty-repo").get(); + startDeleteSnapshot(repositoryName, "snap-on-empty-repo").get(); final int numSnapshots = randomIntBetween(1, 3) + 1; logger.info("--> take {} snapshot(s)", numSnapshots - 1); @@ -385,7 +543,7 @@ public void testGetSnapshotsRequest() throws Exception { logger.info("--> take another snapshot to be in-progress"); // add documents so there are data files to block on for (int i = 10; i < 20; i++) { - index(indexName, "_doc", Integer.toString(i), "foo", "bar" + i); + indexDoc(indexName, Integer.toString(i), "foo", "bar" + i); } refresh(); diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequestBuilder.java index 878f09f1e523c..fecd2c56d99f3 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequestBuilder.java @@ -13,6 +13,7 @@ import org.elasticsearch.client.ElasticsearchClient; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.core.Nullable; import java.util.Map; @@ -181,4 +182,15 @@ public CreateSnapshotRequestBuilder setFeatureStates(String... featureStates) { request.featureStates(featureStates); return this; } + + /** + * Provide a map of user metadata that should be included in the snapshot metadata. + * + * @param metadata user metadata map + * @return this builder + */ + public CreateSnapshotRequestBuilder setUserMetadata(@Nullable Map metadata) { + request.userMetadata(metadata); + return this; + } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java index 33027d4456757..88ad1734b794f 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java @@ -190,6 +190,13 @@ public String[] repositories() { return this.repositories; } + public boolean isSingleRepositoryRequest() { + return repositories.length == 1 + && repositories[0] != null + && "_all".equals(repositories[0]) == false + && Regex.isSimpleMatchPattern(repositories[0]) == false; + } + /** * Sets repository name * @@ -215,13 +222,6 @@ public String repository() { return this.repositories[0]; } - public boolean isSingleRepositoryRequest() { - return repositories.length == 1 - && repositories[0] != null - && "_all".equals(repositories[0]) == false - && Regex.isSimpleMatchPattern(repositories[0]) == false; - } - /** * Returns the names of the snapshots. * diff --git a/server/src/main/java/org/elasticsearch/client/ClusterAdminClient.java b/server/src/main/java/org/elasticsearch/client/ClusterAdminClient.java index 66e2b5b85cc73..e23537573f55a 100644 --- a/server/src/main/java/org/elasticsearch/client/ClusterAdminClient.java +++ b/server/src/main/java/org/elasticsearch/client/ClusterAdminClient.java @@ -517,14 +517,14 @@ public interface ClusterAdminClient extends ElasticsearchClient { ActionFuture getSnapshots(GetSnapshotsRequest request); /** - * Get snapshot. + * Get snapshots. */ void getSnapshots(GetSnapshotsRequest request, ActionListener listener); /** - * Get snapshot. + * Get snapshots. */ - GetSnapshotsRequestBuilder prepareGetSnapshots(String repository); + GetSnapshotsRequestBuilder prepareGetSnapshots(String... repository); /** * Delete snapshot. diff --git a/server/src/main/java/org/elasticsearch/client/support/AbstractClient.java b/server/src/main/java/org/elasticsearch/client/support/AbstractClient.java index 239e0b0dede20..18df0155a41b7 100644 --- a/server/src/main/java/org/elasticsearch/client/support/AbstractClient.java +++ b/server/src/main/java/org/elasticsearch/client/support/AbstractClient.java @@ -982,8 +982,8 @@ public void getSnapshots(GetSnapshotsRequest request, ActionListener - client().admin().cluster().prepareGetSnapshots(repoName).setSnapshots(snapshotName).get()); + client().admin().cluster().prepareGetSnapshots(repoName).setSnapshots(snapshotName).execute().actionGet()); expectThrows(SnapshotMissingException.class, () -> client().admin().cluster().prepareDeleteSnapshot(repoName, snapshotName).get()); diff --git a/test/framework/src/main/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java b/test/framework/src/main/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java index 4818945432596..200bcb2d79f92 100644 --- a/test/framework/src/main/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java @@ -72,6 +72,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -655,7 +656,21 @@ public static List createNSnapshots(Logger logger, String repoName, int for (int i = 0; i < count; i++) { final String snapshot = prefix + i; snapshotNames.add(snapshot); - client().admin().cluster().prepareCreateSnapshot(repoName, snapshot).setWaitForCompletion(true).execute(snapshotsListener); + final Map userMetadata = randomUserMetadata(); + clusterAdmin() + .prepareCreateSnapshot(repoName, snapshot) + .setWaitForCompletion(true) + .setUserMetadata(userMetadata) + .execute(snapshotsListener.delegateFailure((l, response) -> { + final SnapshotInfo snapshotInfoInResponse = response.getSnapshotInfo(); + assertEquals(userMetadata, snapshotInfoInResponse.userMetadata()); + clusterAdmin().prepareGetSnapshots(repoName) + .setSnapshots(snapshot) + .execute(l.delegateFailure((ll, getResponse) -> { + assertEquals(snapshotInfoInResponse, getResponse.getSnapshots().get(0)); + ll.onResponse(response); + })); + })); } for (CreateSnapshotResponse snapshotResponse : allSnapshotsDone.get()) { assertThat(snapshotResponse.getSnapshotInfo().state(), is(SnapshotState.SUCCESS)); @@ -709,4 +724,34 @@ public static void assertSnapshotListSorted(List snapshotInfos, @N orderAssertion.accept(snapshotInfos.get(i), snapshotInfos.get(i + 1)); } } + + /** + * Randomly either generates some random snapshot user metadata or returns {@code null}. + * + * @return random snapshot user metadata or {@code null} + */ + @Nullable + public static Map randomUserMetadata() { + if (randomBoolean()) { + return null; + } + + Map metadata = new HashMap<>(); + long fields = randomLongBetween(0, 4); + for (int i = 0; i < fields; i++) { + if (randomBoolean()) { + metadata.put(randomValueOtherThanMany(metadata::containsKey, () -> randomAlphaOfLengthBetween(2, 10)), + randomAlphaOfLengthBetween(5, 5)); + } else { + Map nested = new HashMap<>(); + long nestedFields = randomLongBetween(0, 4); + for (int j = 0; j < nestedFields; j++) { + nested.put(randomValueOtherThanMany(nested::containsKey, () -> randomAlphaOfLengthBetween(2, 10)), + randomAlphaOfLengthBetween(5, 5)); + } + metadata.put(randomValueOtherThanMany(metadata::containsKey, () -> randomAlphaOfLengthBetween(2, 10)), nested); + } + } + return metadata; + } } diff --git a/x-pack/plugin/ilm/qa/multi-cluster/src/test/java/org/elasticsearch/xpack/ilm/CCRIndexLifecycleIT.java b/x-pack/plugin/ilm/qa/multi-cluster/src/test/java/org/elasticsearch/xpack/ilm/CCRIndexLifecycleIT.java index b5fa103646065..afc40e77bfcbf 100644 --- a/x-pack/plugin/ilm/qa/multi-cluster/src/test/java/org/elasticsearch/xpack/ilm/CCRIndexLifecycleIT.java +++ b/x-pack/plugin/ilm/qa/multi-cluster/src/test/java/org/elasticsearch/xpack/ilm/CCRIndexLifecycleIT.java @@ -795,13 +795,15 @@ public static void updatePolicy(String indexName, String policy) throws IOExcept assertOK(client().performRequest(changePolicyRequest)); } + @SuppressWarnings("unchecked") private String getSnapshotState(String snapshot) throws IOException { Response response = client().performRequest(new Request("GET", "/_snapshot/repo/" + snapshot)); Map responseMap; try (InputStream is = response.getEntity().getContent()) { responseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true); } - @SuppressWarnings("unchecked") Map snapResponse = ((List>) responseMap.get("snapshots")).get(0); + + Map snapResponse = ((List>) responseMap.get("snapshots")).get(0); assertThat(snapResponse.get("snapshot"), equalTo(snapshot)); return (String) snapResponse.get("state"); } diff --git a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/TimeSeriesRestDriver.java b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/TimeSeriesRestDriver.java index 994681c8de0ea..57178f1333150 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/TimeSeriesRestDriver.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/TimeSeriesRestDriver.java @@ -325,6 +325,7 @@ public static String getSnapshotState(RestClient client, String snapshot) throws try (InputStream is = response.getEntity().getContent()) { responseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true); } + Map snapResponse = ((List>) responseMap.get("snapshots")).get(0); assertThat(snapResponse.get("snapshot"), equalTo(snapshot)); return (String) snapResponse.get("state"); diff --git a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java index c4fcd631decbd..956cf407a072a 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java @@ -198,8 +198,8 @@ public void testDeleteActionDeletesSearchableSnapshot() throws Exception { try (InputStream is = getSnapshotsResponse.getEntity().getContent()) { responseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true); } - List> snapshots = (List>) responseMap.get("snapshots"); - return snapshots.size() == 0; + Object snapshots = responseMap.get("snapshots"); + return ((List>) snapshots).size() == 0; } catch (Exception e) { logger.error(e.getMessage(), e); return false; @@ -400,7 +400,7 @@ public void testIdenticalSearchableSnapshotActionIsNoop() throws Exception { responseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true); } assertThat("expected to have only one snapshot, but got: " + responseMap, - ((List) responseMap.get("snapshots")).size(), equalTo(1)); + ((List>) responseMap.get("snapshots")).size(), equalTo(1)); Request hitCount = new Request("GET", "/" + searchableSnapMountedIndexName + "/_count"); Map count = entityAsMap(client().performRequest(hitCount)); @@ -448,7 +448,7 @@ public void testConvertingSearchableSnapshotFromFullToPartial() throws Exception responseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true); } assertThat("expected to have only one snapshot, but got: " + responseMap, - ((List) responseMap.get("snapshots")).size(), equalTo(1)); + ((List>) responseMap.get("snapshots")).size(), equalTo(1)); Request hitCount = new Request("GET", "/" + searchableSnapMountedIndexName + "/_count"); Map count = entityAsMap(client().performRequest(hitCount)); diff --git a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/slm/SnapshotLifecycleRestIT.java b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/slm/SnapshotLifecycleRestIT.java index 0e8d32a7169ac..96c1f861443d1 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/slm/SnapshotLifecycleRestIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/slm/SnapshotLifecycleRestIT.java @@ -46,10 +46,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.concurrent.TimeUnit; import java.util.function.Function; -import java.util.function.Predicate; import java.util.stream.Collectors; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; @@ -60,9 +58,11 @@ import static org.elasticsearch.xpack.core.slm.history.SnapshotHistoryStore.SLM_HISTORY_DATA_STREAM; import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.startsWith; public class SnapshotLifecycleRestIT extends ESRestTestCase { @@ -115,11 +115,6 @@ public void testFullPolicySnapshot() throws Exception { createSnapshotPolicy(policyName, "snap", "*/1 * * * * ?", repoId, indexName, true); - // A test for whether the repository's snapshots have any snapshots starting with "snap-" - Predicate> repoHasSnapshot = snapMap -> Optional.ofNullable((String) snapMap.get("snapshot")) - .map(snapName -> snapName.startsWith("snap-")) - .orElse(false); - // Check that the snapshot was actually taken assertBusy(() -> { Response response = client().performRequest(new Request("GET", "/_snapshot/" + repoId + "/_all")); @@ -128,14 +123,11 @@ public void testFullPolicySnapshot() throws Exception { snapshotResponseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true); } assertThat(snapshotResponseMap.size(), greaterThan(0)); - Map snapResponse = ((List>) snapshotResponseMap.get("snapshots")).stream() - .peek(allReposSnapshots -> logger.info("--> all repository's snapshots: {}", allReposSnapshots)) - .filter(repoHasSnapshot) - .peek(allRepos -> logger.info("--> snapshots with 'snap-' snapshot: {}", allRepos)) - .findFirst() - .orElseThrow(() -> new AssertionError("failed to find snapshot response in " + snapshotResponseMap)); - assertThat(snapResponse.get("indices"), equalTo(Collections.singletonList(indexName))); - Map metadata = (Map) snapResponse.get("metadata"); + List> snapResponse = ((List>) snapshotResponseMap.get("snapshots")); + assertThat(snapResponse, not(empty())); + assertThat(snapResponse.get(0).get("indices"), equalTo(Collections.singletonList(indexName))); + assertThat((String) snapResponse.get(0).get("snapshot"), startsWith("snap-")); + Map metadata = (Map) snapResponse.get(0).get("metadata"); assertNotNull(metadata); assertThat(metadata.get("policy"), equalTo(policyName)); }); @@ -605,8 +597,8 @@ private static Map extractMetadata(Map snapshotR @SuppressWarnings("unchecked") private static Map extractSnapshot(Map snapshotResponseMap, String snapshotPrefix) { - List> snapshots = ((List>) snapshotResponseMap.get("snapshots")); - return snapshots.stream() + List> snapResponse = ((List>) snapshotResponseMap.get("snapshots")); + return snapResponse.stream() .filter(snapshot -> ((String) snapshot.get("snapshot")).startsWith(snapshotPrefix)) .findFirst() .orElse(null); diff --git a/x-pack/plugin/ilm/src/internalClusterTest/java/org/elasticsearch/xpack/slm/SLMSnapshotBlockingIntegTests.java b/x-pack/plugin/ilm/src/internalClusterTest/java/org/elasticsearch/xpack/slm/SLMSnapshotBlockingIntegTests.java index 937caa8d4b77d..176a6ba11a518 100644 --- a/x-pack/plugin/ilm/src/internalClusterTest/java/org/elasticsearch/xpack/slm/SLMSnapshotBlockingIntegTests.java +++ b/x-pack/plugin/ilm/src/internalClusterTest/java/org/elasticsearch/xpack/slm/SLMSnapshotBlockingIntegTests.java @@ -392,8 +392,8 @@ private void testUnsuccessfulSnapshotRetention(boolean partialSuccess) throws Ex assertBusy(() -> { final SnapshotInfo snapshotInfo; try { - final GetSnapshotsResponse snapshotsStatusResponse = client().admin().cluster() - .prepareGetSnapshots(REPO).setSnapshots(successfulSnapshotName.get()).execute().actionGet(); + GetSnapshotsResponse snapshotsStatusResponse = client().admin().cluster() + .prepareGetSnapshots(REPO).setSnapshots(successfulSnapshotName.get()).execute().actionGet(); snapshotInfo = snapshotsStatusResponse.getSnapshots().get(0); } catch (SnapshotMissingException sme) { throw new AssertionError(sme); diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycle.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycle.java index 5aeefb8d5d186..dffb171e18277 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycle.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycle.java @@ -224,7 +224,7 @@ public Collection createComponents(Client client, ClusterService cluster () -> new SnapshotLifecycleTask(client, clusterService, snapshotHistoryStore.get()), clusterService, getClock())); snapshotLifecycleService.get().init(); snapshotRetentionService.set(new SnapshotRetentionService(settings, - () -> new SnapshotRetentionTask(client, clusterService, System::nanoTime, snapshotHistoryStore.get(), threadPool), + () -> new SnapshotRetentionTask(client, clusterService, System::nanoTime, snapshotHistoryStore.get()), getClock())); snapshotRetentionService.get().init(clusterService); components.addAll(Arrays.asList(snapshotLifecycleService.get(), snapshotHistoryStore.get(), snapshotRetentionService.get())); diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/slm/SnapshotRetentionTask.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/slm/SnapshotRetentionTask.java index 170318891ebc8..bc40924e3a6e2 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/slm/SnapshotRetentionTask.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/slm/SnapshotRetentionTask.java @@ -17,13 +17,12 @@ import org.elasticsearch.client.OriginSettingClient; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.Strings; import org.elasticsearch.core.Tuple; import org.elasticsearch.core.TimeValue; -import org.elasticsearch.common.util.concurrent.CountDown; import org.elasticsearch.snapshots.SnapshotId; import org.elasticsearch.snapshots.SnapshotInfo; import org.elasticsearch.snapshots.SnapshotState; -import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xpack.core.ClientHelper; import org.elasticsearch.xpack.core.scheduler.SchedulerEngine; import org.elasticsearch.xpack.core.slm.SnapshotLifecycleMetadata; @@ -35,15 +34,14 @@ import java.io.IOException; import java.time.Instant; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; import java.util.function.LongSupplier; @@ -64,7 +62,6 @@ public class SnapshotRetentionTask implements SchedulerEngine.Listener { private final Client client; private final ClusterService clusterService; private final LongSupplier nowNanoSupplier; - private final ThreadPool threadPool; private final SnapshotHistoryStore historyStore; /** @@ -73,12 +70,11 @@ public class SnapshotRetentionTask implements SchedulerEngine.Listener { private final Set runningDeletions = Collections.synchronizedSet(new HashSet<>()); public SnapshotRetentionTask(Client client, ClusterService clusterService, LongSupplier nowNanoSupplier, - SnapshotHistoryStore historyStore, ThreadPool threadPool) { + SnapshotHistoryStore historyStore) { this.client = new OriginSettingClient(client, ClientHelper.INDEX_LIFECYCLE_ORIGIN); this.clusterService = clusterService; this.nowNanoSupplier = nowNanoSupplier; this.historyStore = historyStore; - this.threadPool = threadPool; } private static String formatSnapshots(Map> snapshotMap) { @@ -244,49 +240,38 @@ void getAllRetainableSnapshots(Collection repositories, ActionListener { - final Map> snapshots = new ConcurrentHashMap<>(); - final CountDown countDown = new CountDown(repositories.size()); - final Runnable onComplete = () -> { - if (countDown.countDown()) { + client.admin().cluster() + .prepareGetSnapshots(repositories.toArray(Strings.EMPTY_ARRAY)) + // don't time out on this request to not produce failed SLM runs in case of a temporarily slow master node + .setMasterNodeTimeout(TimeValue.MAX_VALUE) + .setIgnoreUnavailable(true) + .execute(ActionListener.wrap(resp -> { if (logger.isTraceEnabled()) { - logger.trace("retrieved snapshots: {}", formatSnapshots(snapshots)); + logger.trace("retrieved snapshots: {}", + repositories.stream() + .flatMap(repo -> + resp.getSnapshots() + .stream() + .filter(info -> repo.equals(info.repository())) + .map(si -> si.snapshotId().getName()) + ).collect(Collectors.toList())); } + Map> snapshots = new HashMap<>(); + final Set retainableStates = + org.elasticsearch.core.Set.of(SnapshotState.SUCCESS, SnapshotState.FAILED, SnapshotState.PARTIAL); + repositories.forEach(repo -> { + snapshots.put(repo, + // Only return snapshots in the SUCCESS state + resp.getSnapshots().stream() + .filter(info -> repo.equals(info.repository()) && retainableStates.contains(info.state())) + .collect(Collectors.toList())); + }); listener.onResponse(snapshots); - } - }; - for (String repository : repositories) { - client.admin().cluster() - .prepareGetSnapshots(repository) - // don't time out on this request to not produce failed SLM runs in case of a temporarily slow master node - .setMasterNodeTimeout(TimeValue.MAX_VALUE) - .setIgnoreUnavailable(true) - .execute(ActionListener.wrap(resp -> { - final Set retainableStates = - new HashSet<>(Arrays.asList(SnapshotState.SUCCESS, SnapshotState.FAILED, SnapshotState.PARTIAL)); - try { - snapshots.compute(repository, (k, previousSnaps) -> { - if (previousSnaps != null) { - throw new IllegalStateException("duplicate snapshot retrieval for repository" + repository); - } - return resp.getSnapshots().stream() - .filter(info -> retainableStates.contains(info.state())) - .collect(Collectors.toList()); - }); - onComplete.run(); - } catch (Exception e) { - logger.error(new ParameterizedMessage("exception computing snapshots for repository {}", repository), e); - throw e; - } - }, - e -> { - logger.warn(new ParameterizedMessage("unable to retrieve snapshots for repository [{}]", repository), e); - onComplete.run(); - errorHandler.accept(e); - } - )); - } - }); + }, + e -> { + logger.debug(new ParameterizedMessage("unable to retrieve snapshots for [{}] repositories", repositories), e); + errorHandler.accept(e); + })); } static String getPolicyId(SnapshotInfo snapshotInfo) { diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionServiceTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionServiceTests.java index d1a892aac1246..d344c8bf823fe 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionServiceTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionServiceTests.java @@ -117,7 +117,7 @@ private static class FakeRetentionTask extends SnapshotRetentionTask { } FakeRetentionTask(Consumer onTrigger) { - super(fakeClient(), null, System::nanoTime, mock(SnapshotHistoryStore.class), mock(ThreadPool.class)); + super(fakeClient(), null, System::nanoTime, mock(SnapshotHistoryStore.class)); this.onTrigger = onTrigger; } diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionTaskTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionTaskTests.java index d95bff33de5de..7b42d1742b4ee 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionTaskTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionTaskTests.java @@ -266,8 +266,7 @@ private void retentionTaskTest(final boolean deletionSuccess) throws Exception { deletedSnapshotsInHistory.add(historyItem.getSnapshotName()); historyLatch.countDown(); }), - threadPool, - () -> { + () -> { List snaps = new ArrayList<>(2); snaps.add(eligibleSnapshot); snaps.add(ineligibleSnapshot); @@ -332,8 +331,8 @@ void doExecute(ActionType action, Request request, ActionListener fail("should never write history")), - threadPool); + (historyItem) -> fail("should never write history")) + ); AtomicReference errHandlerCalled = new AtomicReference<>(null); task.getAllRetainableSnapshots(Collections.singleton(repoId), new ActionListener>>() { @@ -388,8 +387,8 @@ void doExecute(ActionType action, Request request, ActionListener fail("should never write history")), - threadPool); + (historyItem) -> fail("should never write history")) + ); AtomicBoolean onFailureCalled = new AtomicBoolean(false); task.deleteSnapshot("policy", "foo", new SnapshotId("name", "uuid"), @@ -436,8 +435,7 @@ private void doTestSkipDuringMode(OperationMode mode) throws Exception { SnapshotRetentionTask task = new MockSnapshotRetentionTask(noOpClient, clusterService, new SnapshotLifecycleTaskTests.VerifyingHistoryStore(noOpClient, ZoneOffset.UTC, (historyItem) -> fail("should never write history")), - threadPool, - () -> { + () -> { fail("should not retrieve snapshots"); return null; }, @@ -476,8 +474,7 @@ private void doTestRunManuallyDuringMode(OperationMode mode) throws Exception { MockSnapshotRetentionTask task = new MockSnapshotRetentionTask(noOpClient, clusterService, new SnapshotLifecycleTaskTests.VerifyingHistoryStore(noOpClient, ZoneOffset.UTC, (historyItem) -> { }), - threadPool, - () -> { + () -> { retentionWasRun.set(true); return Collections.emptyMap(); }, @@ -525,11 +522,10 @@ private static class MockSnapshotRetentionTask extends SnapshotRetentionTask { MockSnapshotRetentionTask(Client client, ClusterService clusterService, SnapshotHistoryStore historyStore, - ThreadPool threadPool, Supplier>> snapshotRetriever, DeleteSnapshotMock deleteRunner, LongSupplier nanoSupplier) { - super(client, clusterService, nanoSupplier, historyStore, threadPool); + super(client, clusterService, nanoSupplier, historyStore); this.snapshotRetriever = snapshotRetriever; this.deleteRunner = deleteRunner; } diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authz/SnapshotUserRoleIntegTests.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authz/SnapshotUserRoleIntegTests.java index fe4dbf8429ad2..74b7f8213fe7f 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authz/SnapshotUserRoleIntegTests.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authz/SnapshotUserRoleIntegTests.java @@ -78,7 +78,8 @@ public void testSnapshotUserRoleCanSnapshotAndSeeAllIndices() { final GetSnapshotsResponse getSnapshotResponse = client.admin().cluster().prepareGetSnapshots("repo").get(); assertThat(getSnapshotResponse.getSnapshots().size(), is(1)); assertThat(getSnapshotResponse.getSnapshots().get(0).snapshotId().getName(), is("snap")); - assertThat(getSnapshotResponse.getSnapshots().get(0).indices(), containsInAnyOrder(INTERNAL_SECURITY_MAIN_INDEX_7, ordinaryIndex)); + assertThat(getSnapshotResponse.getSnapshots().get(0).indices(), containsInAnyOrder(INTERNAL_SECURITY_MAIN_INDEX_7, + ordinaryIndex)); } public void testSnapshotUserRoleIsReserved() {