From 654eeb161c005cf8aaf529814b195fd0c4dd5c97 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 6 Sep 2021 14:24:08 +0200 Subject: [PATCH 1/9] Implement Exclude Patterns for Snapshot- and Repository Names in Get Snapshots API It's in the title. Adds support for exclude patterns combined with wildcard requests similar to what we support for index names. --- .../apis/get-snapshot-api.asciidoc | 87 ++++++++++++++- .../snapshots/GetSnapshotsIT.java | 100 +++++++++++++++--- .../get/TransportGetRepositoriesAction.java | 20 +++- .../get/TransportGetSnapshotsAction.java | 20 +++- 4 files changed, 203 insertions(+), 24 deletions(-) diff --git a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc index a708a20bd8b24..46aca43d579dd 100644 --- a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc +++ b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc @@ -60,14 +60,15 @@ Use the get snapshot API to return information about one or more snapshots, incl ``:: (Required, string) Comma-separated list of snapshot repository names used to limit the request. -Wildcard (`*`) expressions are supported. +Wildcard (`*`) expressions are supported including combining wildcards with exclude patterns starting in `-`. + To get information about all snapshot repositories registered in the cluster, omit this parameter or use `*` or `_all`. ``:: (Required, string) -Comma-separated list of snapshot names to retrieve. Also accepts wildcards (`*`). +Comma-separated list of snapshot names to retrieve. Also accepts wildcards (`*`) as well as combining wildcards and exclude patterns +starting in `-`. + * To get information about all snapshots in a registered repository, use a wildcard (`*`) or `_all`. * To get information about any snapshots that are currently running, use `_current`. @@ -538,4 +539,84 @@ The API returns the following response: // 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.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/] \ No newline at end of file +// TESTRESPONSE[s/"duration_in_millis": 0/"duration_in_millis": $body.snapshots.0.duration_in_millis/] + +The following request returns information for all snapshots with prefix `snapshot` in the `my_repository` repository, +except for the one named `snapshot_3` + +[source,console] +---- +GET /_snapshot/my_repository/snapshot*,-snapshot_3?sort=name +---- + +The API returns the following response: + +[source,console-result] +---- +{ + "snapshots": [ + { + "snapshot": "snapshot_1", + "uuid": "dKb54xw67gvdRctLCxSket", + "repository": "my_repository", + "version_id": , + "version": , + "indices": [], + "data_streams": [], + "feature_states": [], + "include_global_state": true, + "state": "SUCCESS", + "start_time": "2020-07-06T21:55:18.129Z", + "start_time_in_millis": 1593093628850, + "end_time": "2020-07-06T21:55:18.129Z", + "end_time_in_millis": 1593094752018, + "duration_in_millis": 0, + "failures": [], + "shards": { + "total": 0, + "failed": 0, + "successful": 0 + } + }, + { + "snapshot": "snapshot_2", + "uuid": "vdRctLCxSketdKb54xw67g", + "repository": "my_repository", + "version_id": , + "version": , + "indices": [], + "data_streams": [], + "feature_states": [], + "include_global_state": true, + "state": "SUCCESS", + "start_time": "2020-07-06T21:55:18.130Z", + "start_time_in_millis": 1593093628851, + "end_time": "2020-07-06T21:55:18.130Z", + "end_time_in_millis": 1593094752019, + "duration_in_millis": 1, + "failures": [], + "shards": { + "total": 0, + "failed": 0, + "successful": 0 + }, + } + ], + "total": 2, + "remaining": 0 +} +---- +// TESTRESPONSE[s/"uuid": "dKb54xw67gvdRctLCxSket"/"uuid": $body.snapshots.0.uuid/] +// TESTRESPONSE[s/"uuid": "vdRctLCxSketdKb54xw67g"/"uuid": $body.snapshots.1.uuid/] +// TESTRESPONSE[s/"version_id": /"version_id": $body.snapshots.0.version_id/] +// 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": "2020-07-06T21:55:18.130Z"/"start_time": $body.snapshots.1.start_time/] +// TESTRESPONSE[s/"start_time_in_millis": 1593093628850/"start_time_in_millis": $body.snapshots.0.start_time_in_millis/] +// TESTRESPONSE[s/"start_time_in_millis": 1593093628851/"start_time_in_millis": $body.snapshots.1.start_time_in_millis/] +// TESTRESPONSE[s/"end_time": "2020-07-06T21:55:18.129Z"/"end_time": $body.snapshots.0.end_time/] +// TESTRESPONSE[s/"end_time": "2020-07-06T21:55:18.130Z"/"end_time": $body.snapshots.1.end_time/] +// TESTRESPONSE[s/"end_time_in_millis": 1593094752018/"end_time_in_millis": $body.snapshots.0.end_time_in_millis/] +// TESTRESPONSE[s/"end_time_in_millis": 1593094752019/"end_time_in_millis": $body.snapshots.1.end_time_in_millis/] +// TESTRESPONSE[s/"duration_in_millis": 0/"duration_in_millis": $body.snapshots.0.duration_in_millis/] +// TESTRESPONSE[s/"duration_in_millis": 1/"duration_in_millis": $body.snapshots.1.duration_in_millis/] diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java index e558dfbd0bcac..ba9163bf29547 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java @@ -23,6 +23,7 @@ import java.util.Collection; import java.util.HashSet; import java.util.List; +import java.util.Set; import static org.hamcrest.Matchers.in; import static org.hamcrest.Matchers.is; @@ -208,6 +209,67 @@ public void testPaginationRequiresVerboseListing() throws Exception { ); } + public void testExcludePatterns() throws Exception { + final String repoName1 = "test-repo-1"; + final String repoName2 = "test-repo-2"; + final String otherRepo = "other-repo"; + createRepository(repoName1, "fs"); + createRepository(repoName2, "fs"); + createRepository(otherRepo, "fs"); + + final List namesRepo1 = createNSnapshots(repoName1, randomIntBetween(1, 5)); + final List namesRepo2 = createNSnapshots(repoName2, randomIntBetween(1, 5)); + final List namesOtherRepo = createNSnapshots(otherRepo, randomIntBetween(1, 5)); + + final Collection allSnapshotNames = new HashSet<>(namesRepo1); + allSnapshotNames.addAll(namesRepo2); + final Collection allSnapshotNamesWithoutOther = Set.copyOf(allSnapshotNames); + allSnapshotNames.addAll(namesOtherRepo); + + final SortOrder order = SortOrder.DESC; + final List allSorted = allSnapshotsSorted(allSnapshotNames, "*", GetSnapshotsRequest.SortBy.REPOSITORY, order); + final List allSortedWithoutOther = allSnapshotsSorted( + allSnapshotNamesWithoutOther, + "*,-" + otherRepo, + GetSnapshotsRequest.SortBy.REPOSITORY, + order + ); + assertThat(allSorted.subList(0, allSnapshotNamesWithoutOther.size()), is(allSortedWithoutOther)); + + final List allInOther = allSnapshotsSorted( + namesOtherRepo, + "*,-test-repo-*", + GetSnapshotsRequest.SortBy.REPOSITORY, + order + ); + assertThat(allSorted.subList(allSnapshotNamesWithoutOther.size(), allSorted.size()), is(allInOther)); + + final String otherPrefixSnapshot1 = "other-prefix-snapshot-1"; + createFullSnapshot(otherRepo, otherPrefixSnapshot1); + final String otherPrefixSnapshot2 = "other-prefix-snapshot-2"; + createFullSnapshot(otherRepo, otherPrefixSnapshot2); + + final String patternOtherRepo = randomBoolean() ? otherRepo : "*,-test-repo-*"; + final List allInOtherWithoutOtherPrefix = allSnapshotsSorted( + namesOtherRepo, + patternOtherRepo, + GetSnapshotsRequest.SortBy.REPOSITORY, + order, + "-other*" + ); + assertThat(allInOtherWithoutOtherPrefix, is(allInOther)); + + final List allInOtherWithoutOtherExplicit = allSnapshotsSorted( + namesOtherRepo, + patternOtherRepo, + GetSnapshotsRequest.SortBy.REPOSITORY, + order, + "-" + otherPrefixSnapshot1, + "-" + otherPrefixSnapshot2 + ); + assertThat(allInOtherWithoutOtherExplicit, is(allInOther)); + } + private static void assertStablePagination(String repoName, Collection allSnapshotNames, GetSnapshotsRequest.SortBy sort) { final SortOrder order = randomFrom(SortOrder.values()); final List allSorted = allSnapshotsSorted(allSnapshotNames, repoName, sort, order); @@ -244,9 +306,17 @@ private static List allSnapshotsSorted( Collection allSnapshotNames, String repoName, GetSnapshotsRequest.SortBy sortBy, - SortOrder order + SortOrder order, + String... extraPatterns ) { - final GetSnapshotsResponse getSnapshotsResponse = sortedWithLimit(repoName, sortBy, null, GetSnapshotsRequest.NO_LIMIT, order); + final GetSnapshotsResponse getSnapshotsResponse = sortedWithLimit( + repoName, + sortBy, + null, + GetSnapshotsRequest.NO_LIMIT, + order, + extraPatterns + ); final List snapshotInfos = getSnapshotsResponse.getSnapshots(); assertEquals(snapshotInfos.size(), allSnapshotNames.size()); assertEquals(getSnapshotsResponse.totalCount(), allSnapshotNames.size()); @@ -262,9 +332,15 @@ private static GetSnapshotsResponse sortedWithLimit( GetSnapshotsRequest.SortBy sortBy, String after, int size, - SortOrder order + SortOrder order, + String... extraPatterns ) { - return baseGetSnapshotsRequest(repoName).setAfter(after).setSort(sortBy).setSize(size).setOrder(order).get(); + return baseGetSnapshotsRequest(repoName).setAfter(after) + .setSort(sortBy) + .setSize(size) + .setOrder(order) + .addSnapshots(extraPatterns) + .get(); } private static GetSnapshotsResponse sortedWithLimit( @@ -277,18 +353,8 @@ private static GetSnapshotsResponse sortedWithLimit( return baseGetSnapshotsRequest(repoName).setOffset(offset).setSort(sortBy).setSize(size).setOrder(order).get(); } - private static GetSnapshotsRequestBuilder baseGetSnapshotsRequest(String repoName) { - final GetSnapshotsRequestBuilder builder = clusterAdmin().prepareGetSnapshots(repoName); - // exclude old version snapshot from test assertions every time and do a prefixed query in either case half the time - if (randomBoolean() - || clusterAdmin().prepareGetSnapshots(repoName) - .setSnapshots(AbstractSnapshotIntegTestCase.OLD_VERSION_SNAPSHOT_PREFIX + "*") - .setIgnoreUnavailable(true) - .get() - .getSnapshots() - .isEmpty() == false) { - builder.setSnapshots(RANDOM_SNAPSHOT_NAME_PREFIX + "*"); - } - return builder; + private static GetSnapshotsRequestBuilder baseGetSnapshotsRequest(String repoNamePattern) { + return clusterAdmin().prepareGetSnapshots(repoNamePattern.split(",")) + .setSnapshots("*", "-" + AbstractSnapshotIntegTestCase.OLD_VERSION_SNAPSHOT_PREFIX + "*"); } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/get/TransportGetRepositoriesAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/get/TransportGetRepositoriesAction.java index a6c9f9c717b96..c5289f2a9e991 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/get/TransportGetRepositoriesAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/get/TransportGetRepositoriesAction.java @@ -26,6 +26,7 @@ import org.elasticsearch.transport.TransportService; import java.util.ArrayList; +import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Set; @@ -83,11 +84,25 @@ public static List getRepositories(ClusterState state, Strin if (repoNames.length == 0 || (repoNames.length == 1 && ("_all".equals(repoNames[0]) || "*".equals(repoNames[0])))) { return repositories.repositories(); } else { - Set repositoriesToGet = new LinkedHashSet<>(); // to keep insertion order + boolean seenWildcard = false; + final Set repositoriesToGet = new LinkedHashSet<>(); // to keep insertion order + final Set repositoriesNotToGet = new HashSet<>(); for (String repositoryOrPattern : repoNames) { - if (Regex.isSimpleMatchPattern(repositoryOrPattern) == false) { + if (seenWildcard && repositoryOrPattern.length() > 1 && repositoryOrPattern.startsWith("-")) { + final String positivePattern = repositoryOrPattern.substring(1); + if (Regex.isSimpleMatchPattern(positivePattern)) { + for (RepositoryMetadata repository : repositories.repositories()) { + if (Regex.simpleMatch(positivePattern, repository.name())) { + repositoriesNotToGet.add(repository.name()); + } + } + } else { + repositoriesNotToGet.add(positivePattern); + } + } else if (Regex.isSimpleMatchPattern(repositoryOrPattern) == false) { repositoriesToGet.add(repositoryOrPattern); } else { + seenWildcard = true; for (RepositoryMetadata repository : repositories.repositories()) { if (Regex.simpleMatch(repositoryOrPattern, repository.name())) { repositoriesToGet.add(repository.name()); @@ -95,6 +110,7 @@ public static List getRepositories(ClusterState state, Strin } } } + repositoriesToGet.removeAll(repositoriesNotToGet); List repositoryListBuilder = new ArrayList<>(); for (String repository : repositoriesToGet) { RepositoryMetadata repositoryMetadata = repositories.repository(repository); diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java index 7ac984014a770..fc4a62a1f80ec 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java @@ -290,11 +290,26 @@ private void loadSnapshotInfos( } final Set toResolve = new HashSet<>(); + final Set toExclude = new HashSet<>(); + boolean seenWildcard = false; if (isAllSnapshots(snapshots)) { toResolve.addAll(allSnapshotIds.values()); } else { for (String snapshotOrPattern : snapshots) { - if (GetSnapshotsRequest.CURRENT_SNAPSHOT.equalsIgnoreCase(snapshotOrPattern)) { + if (seenWildcard && snapshotOrPattern.length() > 1 && snapshotOrPattern.startsWith("-")) { + final String positivePattern = snapshotOrPattern.substring(1); + if (Regex.isSimpleMatchPattern(positivePattern)) { + for (Map.Entry entry : allSnapshotIds.entrySet()) { + if (Regex.simpleMatch(positivePattern, entry.getKey())) { + toExclude.add(entry.getValue()); + } + } + } else { + if (allSnapshotIds.containsKey(positivePattern)) { + toExclude.add(allSnapshotIds.get(positivePattern)); + } + } + } else if (GetSnapshotsRequest.CURRENT_SNAPSHOT.equalsIgnoreCase(snapshotOrPattern)) { toResolve.addAll(currentSnapshots.stream().map(SnapshotInfo::snapshot).collect(Collectors.toList())); } else if (Regex.isSimpleMatchPattern(snapshotOrPattern) == false) { if (allSnapshotIds.containsKey(snapshotOrPattern)) { @@ -303,6 +318,7 @@ private void loadSnapshotInfos( throw new SnapshotMissingException(repo, snapshotOrPattern); } } else { + seenWildcard = true; for (Map.Entry entry : allSnapshotIds.entrySet()) { if (Regex.simpleMatch(snapshotOrPattern, entry.getKey())) { toResolve.add(entry.getValue()); @@ -310,7 +326,7 @@ private void loadSnapshotInfos( } } } - + toResolve.removeAll(toExclude); if (toResolve.isEmpty() && ignoreUnavailable == false && isCurrentSnapshotsOnly(snapshots) == false) { throw new SnapshotMissingException(repo, snapshots[0]); } From 6e4f0d9f76a14d61b2a877b3bda39702616f7bd8 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 8 Sep 2021 11:12:39 +0200 Subject: [PATCH 2/9] make logic for resolving things nicer --- .../get/TransportGetRepositoriesAction.java | 65 +++++++++---------- .../snapshots/get/GetSnapshotsRequest.java | 1 - .../get/TransportGetSnapshotsAction.java | 58 ++++++++--------- .../rest/action/cat/RestSnapshotAction.java | 6 +- 4 files changed, 62 insertions(+), 68 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/get/TransportGetRepositoriesAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/get/TransportGetRepositoriesAction.java index c5289f2a9e991..9b787a26134bf 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/get/TransportGetRepositoriesAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/get/TransportGetRepositoriesAction.java @@ -18,6 +18,7 @@ import org.elasticsearch.cluster.metadata.RepositoriesMetadata; import org.elasticsearch.cluster.metadata.RepositoryMetadata; import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.regex.Regex; import org.elasticsearch.repositories.RepositoryMissingException; @@ -26,7 +27,6 @@ import org.elasticsearch.transport.TransportService; import java.util.ArrayList; -import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Set; @@ -36,6 +36,8 @@ */ public class TransportGetRepositoriesAction extends TransportMasterNodeReadAction { + public static final String ALL_PATTERN = "_all"; + @Inject public TransportGetRepositoriesAction( TransportService transportService, @@ -57,6 +59,11 @@ public TransportGetRepositoriesAction( ); } + public static boolean isMatchAll(String[] patterns) { + return (patterns.length == 0) + || (patterns.length == 1 && (ALL_PATTERN.equalsIgnoreCase(patterns[0]) || Regex.isMatchAllPattern(patterns[0]))); + } + @Override protected ClusterBlockException checkBlock(GetRepositoriesRequest request, ClusterState state) { return state.blocks().globalBlockedException(ClusterBlockLevel.METADATA_READ); @@ -81,45 +88,37 @@ protected void masterOperation( */ public static List getRepositories(ClusterState state, String[] repoNames) { RepositoriesMetadata repositories = state.metadata().custom(RepositoriesMetadata.TYPE, RepositoriesMetadata.EMPTY); - if (repoNames.length == 0 || (repoNames.length == 1 && ("_all".equals(repoNames[0]) || "*".equals(repoNames[0])))) { + if (isMatchAll(repoNames)) { return repositories.repositories(); - } else { - boolean seenWildcard = false; - final Set repositoriesToGet = new LinkedHashSet<>(); // to keep insertion order - final Set repositoriesNotToGet = new HashSet<>(); - for (String repositoryOrPattern : repoNames) { - if (seenWildcard && repositoryOrPattern.length() > 1 && repositoryOrPattern.startsWith("-")) { - final String positivePattern = repositoryOrPattern.substring(1); - if (Regex.isSimpleMatchPattern(positivePattern)) { - for (RepositoryMetadata repository : repositories.repositories()) { - if (Regex.simpleMatch(positivePattern, repository.name())) { - repositoriesNotToGet.add(repository.name()); - } - } - } else { - repositoriesNotToGet.add(positivePattern); - } - } else if (Regex.isSimpleMatchPattern(repositoryOrPattern) == false) { - repositoriesToGet.add(repositoryOrPattern); - } else { + } + final List includePatterns = new ArrayList<>(); + final List excludePatterns = new ArrayList<>(); + boolean seenWildcard = false; + for (String repositoryOrPattern : repoNames) { + if (seenWildcard && repositoryOrPattern.length() > 1 && repositoryOrPattern.startsWith("-")) { + excludePatterns.add(repositoryOrPattern.substring(1)); + } else { + if (Regex.isSimpleMatchPattern(repositoryOrPattern)) { seenWildcard = true; - for (RepositoryMetadata repository : repositories.repositories()) { - if (Regex.simpleMatch(repositoryOrPattern, repository.name())) { - repositoriesToGet.add(repository.name()); - } + } else { + if (repositories.repository(repositoryOrPattern) == null) { + throw new RepositoryMissingException(repositoryOrPattern); } } + includePatterns.add(repositoryOrPattern); } - repositoriesToGet.removeAll(repositoriesNotToGet); - List repositoryListBuilder = new ArrayList<>(); - for (String repository : repositoriesToGet) { - RepositoryMetadata repositoryMetadata = repositories.repository(repository); - if (repositoryMetadata == null) { - throw new RepositoryMissingException(repository); + } + final String[] excludes = excludePatterns.toArray(Strings.EMPTY_ARRAY); + final Set repositoryListBuilder = new LinkedHashSet<>(); // to keep insertion order + for (String repositoryOrPattern : includePatterns) { + for (RepositoryMetadata repository : repositories.repositories()) { + if (repositoryListBuilder.contains(repository) == false + && Regex.simpleMatch(repositoryOrPattern, repository.name()) + && Regex.simpleMatch(excludes, repository.name()) == false) { + repositoryListBuilder.add(repository); } - repositoryListBuilder.add(repositoryMetadata); } - return repositoryListBuilder; } + return List.copyOf(repositoryListBuilder); } } 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 54314bc872bd2..706d23fc1fb5f 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 @@ -36,7 +36,6 @@ */ public class GetSnapshotsRequest extends MasterNodeRequest { - public static final String ALL_SNAPSHOTS = "_all"; public static final String CURRENT_SNAPSHOT = "_current"; public static final String NO_POLICY_PATTERN = "_none"; public static final boolean DEFAULT_VERBOSE_MODE = true; diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java index 74f0c06473d3d..aaef3addb157b 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java @@ -297,43 +297,41 @@ private void loadSnapshotInfos( } final Set toResolve = new HashSet<>(); - final Set toExclude = new HashSet<>(); - boolean seenWildcard = false; - if (isAllSnapshots(snapshots)) { + if (TransportGetRepositoriesAction.isMatchAll(snapshots)) { toResolve.addAll(allSnapshotIds.values()); } else { + final List includePatterns = new ArrayList<>(); + final List excludePatterns = new ArrayList<>(); + boolean seenWildcard = false; for (String snapshotOrPattern : snapshots) { if (seenWildcard && snapshotOrPattern.length() > 1 && snapshotOrPattern.startsWith("-")) { - final String positivePattern = snapshotOrPattern.substring(1); - if (Regex.isSimpleMatchPattern(positivePattern)) { - for (Map.Entry entry : allSnapshotIds.entrySet()) { - if (Regex.simpleMatch(positivePattern, entry.getKey())) { - toExclude.add(entry.getValue()); - } - } - } else { - if (allSnapshotIds.containsKey(positivePattern)) { - toExclude.add(allSnapshotIds.get(positivePattern)); - } - } - } else if (GetSnapshotsRequest.CURRENT_SNAPSHOT.equalsIgnoreCase(snapshotOrPattern)) { - toResolve.addAll(currentSnapshots.stream().map(SnapshotInfo::snapshot).collect(Collectors.toList())); - } else if (Regex.isSimpleMatchPattern(snapshotOrPattern) == false) { - if (allSnapshotIds.containsKey(snapshotOrPattern)) { - toResolve.add(allSnapshotIds.get(snapshotOrPattern)); - } else if (ignoreUnavailable == false) { - throw new SnapshotMissingException(repo, snapshotOrPattern); - } + excludePatterns.add(snapshotOrPattern.substring(1)); } else { - seenWildcard = true; - for (Map.Entry entry : allSnapshotIds.entrySet()) { - if (Regex.simpleMatch(snapshotOrPattern, entry.getKey())) { - toResolve.add(entry.getValue()); + if (Regex.isSimpleMatchPattern(snapshotOrPattern)) { + seenWildcard = true; + includePatterns.add(snapshotOrPattern); + } else if (GetSnapshotsRequest.CURRENT_SNAPSHOT.equalsIgnoreCase(snapshotOrPattern)) { + toResolve.addAll(currentSnapshots.stream().map(SnapshotInfo::snapshot).collect(Collectors.toList())); + } else { + final Snapshot found = allSnapshotIds.get(snapshotOrPattern); + if (found != null) { + toResolve.add(found); + } else if (ignoreUnavailable == false) { + throw new SnapshotMissingException(repo, snapshotOrPattern); } } } } - toResolve.removeAll(toExclude); + final String[] includes = includePatterns.toArray(Strings.EMPTY_ARRAY); + final String[] excludes = excludePatterns.toArray(Strings.EMPTY_ARRAY); + for (Map.Entry entry : allSnapshotIds.entrySet()) { + final Snapshot snapshot = entry.getValue(); + if (toResolve.contains(snapshot) == false + && Regex.simpleMatch(includes, entry.getKey()) + && Regex.simpleMatch(excludes, entry.getKey()) == false) { + toResolve.add(snapshot); + } + } if (toResolve.isEmpty() && ignoreUnavailable == false && isCurrentSnapshotsOnly(snapshots) == false) { throw new SnapshotMissingException(repo, snapshots[0]); } @@ -456,10 +454,6 @@ public void onFailure(Exception e) { ); } - private boolean isAllSnapshots(String[] snapshots) { - return (snapshots.length == 0) || (snapshots.length == 1 && GetSnapshotsRequest.ALL_SNAPSHOTS.equalsIgnoreCase(snapshots[0])); - } - private boolean isCurrentSnapshotsOnly(String[] snapshots) { return (snapshots.length == 1 && GetSnapshotsRequest.CURRENT_SNAPSHOT.equalsIgnoreCase(snapshots[0])); } diff --git a/server/src/main/java/org/elasticsearch/rest/action/cat/RestSnapshotAction.java b/server/src/main/java/org/elasticsearch/rest/action/cat/RestSnapshotAction.java index ae0fd58006dad..f3b24b2855fa6 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/cat/RestSnapshotAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/cat/RestSnapshotAction.java @@ -10,6 +10,7 @@ import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.action.admin.cluster.repositories.get.TransportGetRepositoriesAction; import org.elasticsearch.action.admin.cluster.snapshots.get.GetSnapshotsRequest; import org.elasticsearch.action.admin.cluster.snapshots.get.GetSnapshotsResponse; import org.elasticsearch.client.node.NodeClient; @@ -49,9 +50,10 @@ public String getName() { @Override protected RestChannelConsumer doCatRequest(final RestRequest request, NodeClient client) { + final String[] matchAll = {TransportGetRepositoriesAction.ALL_PATTERN}; GetSnapshotsRequest getSnapshotsRequest = new GetSnapshotsRequest() - .repositories(request.paramAsStringArray("repository", new String[]{"_all"})) - .snapshots(new String[]{GetSnapshotsRequest.ALL_SNAPSHOTS}); + .repositories(request.paramAsStringArray("repository", matchAll)) + .snapshots(matchAll); getSnapshotsRequest.ignoreUnavailable(request.paramAsBoolean("ignore_unavailable", getSnapshotsRequest.ignoreUnavailable())); From 081633a98b6d58222abd4fa23c9bd4041e8bd31e Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 8 Sep 2021 11:25:45 +0200 Subject: [PATCH 3/9] align pattern usage --- .../snapshots/GetSnapshotsIT.java | 88 ++++++++++--------- 1 file changed, 48 insertions(+), 40 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java index 85ef2bb46610f..ea1804cf23630 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java @@ -76,38 +76,39 @@ public void testSortBy() throws Exception { private void doTestSortOrder(String repoName, Collection allSnapshotNames, SortOrder order) { final List defaultSorting = clusterAdmin().prepareGetSnapshots(repoName).setOrder(order).get().getSnapshots(); assertSnapshotListSorted(defaultSorting, null, order); + final String[] repos = { repoName }; assertSnapshotListSorted( - allSnapshotsSorted(allSnapshotNames, repoName, GetSnapshotsRequest.SortBy.NAME, order), + allSnapshotsSorted(allSnapshotNames, repos, GetSnapshotsRequest.SortBy.NAME, order), GetSnapshotsRequest.SortBy.NAME, order ); assertSnapshotListSorted( - allSnapshotsSorted(allSnapshotNames, repoName, GetSnapshotsRequest.SortBy.DURATION, order), + allSnapshotsSorted(allSnapshotNames, repos, GetSnapshotsRequest.SortBy.DURATION, order), GetSnapshotsRequest.SortBy.DURATION, order ); assertSnapshotListSorted( - allSnapshotsSorted(allSnapshotNames, repoName, GetSnapshotsRequest.SortBy.INDICES, order), + allSnapshotsSorted(allSnapshotNames, repos, GetSnapshotsRequest.SortBy.INDICES, order), GetSnapshotsRequest.SortBy.INDICES, order ); assertSnapshotListSorted( - allSnapshotsSorted(allSnapshotNames, repoName, GetSnapshotsRequest.SortBy.START_TIME, order), + allSnapshotsSorted(allSnapshotNames, repos, GetSnapshotsRequest.SortBy.START_TIME, order), GetSnapshotsRequest.SortBy.START_TIME, order ); assertSnapshotListSorted( - allSnapshotsSorted(allSnapshotNames, repoName, GetSnapshotsRequest.SortBy.SHARDS, order), + allSnapshotsSorted(allSnapshotNames, repos, GetSnapshotsRequest.SortBy.SHARDS, order), GetSnapshotsRequest.SortBy.SHARDS, order ); assertSnapshotListSorted( - allSnapshotsSorted(allSnapshotNames, repoName, GetSnapshotsRequest.SortBy.FAILED_SHARDS, order), + allSnapshotsSorted(allSnapshotNames, repos, GetSnapshotsRequest.SortBy.FAILED_SHARDS, order), GetSnapshotsRequest.SortBy.FAILED_SHARDS, order ); assertSnapshotListSorted( - allSnapshotsSorted(allSnapshotNames, repoName, GetSnapshotsRequest.SortBy.REPOSITORY, order), + allSnapshotsSorted(allSnapshotNames, repos, GetSnapshotsRequest.SortBy.REPOSITORY, order), GetSnapshotsRequest.SortBy.REPOSITORY, order ); @@ -128,22 +129,23 @@ public void testResponseSizeLimit() throws Exception { } private void doTestPagination(String repoName, List names, GetSnapshotsRequest.SortBy sort, SortOrder order) { - final List allSnapshotsSorted = allSnapshotsSorted(names, repoName, sort, order); - final GetSnapshotsResponse batch1 = sortedWithLimit(repoName, sort, null, 2, order); + final String[] repos = { repoName }; + final List allSnapshotsSorted = allSnapshotsSorted(names, repos, sort, order); + final GetSnapshotsResponse batch1 = sortedWithLimit(repos, sort, null, 2, order); assertEquals(allSnapshotsSorted.subList(0, 2), batch1.getSnapshots()); - final GetSnapshotsResponse batch2 = sortedWithLimit(repoName, sort, batch1.next(), 2, order); + final GetSnapshotsResponse batch2 = sortedWithLimit(repos, sort, batch1.next(), 2, order); assertEquals(allSnapshotsSorted.subList(2, 4), batch2.getSnapshots()); final int lastBatch = names.size() - batch1.getSnapshots().size() - batch2.getSnapshots().size(); - final GetSnapshotsResponse batch3 = sortedWithLimit(repoName, sort, batch2.next(), lastBatch, order); + final GetSnapshotsResponse batch3 = sortedWithLimit(repos, sort, batch2.next(), lastBatch, order); assertEquals( batch3.getSnapshots(), allSnapshotsSorted.subList(batch1.getSnapshots().size() + batch2.getSnapshots().size(), names.size()) ); - final GetSnapshotsResponse batch3NoLimit = sortedWithLimit(repoName, sort, batch2.next(), GetSnapshotsRequest.NO_LIMIT, order); + final GetSnapshotsResponse batch3NoLimit = sortedWithLimit(repos, sort, batch2.next(), GetSnapshotsRequest.NO_LIMIT, order); assertNull(batch3NoLimit.next()); assertEquals(batch3.getSnapshots(), batch3NoLimit.getSnapshots()); final GetSnapshotsResponse batch3LargeLimit = sortedWithLimit( - repoName, + repos, sort, batch2.next(), lastBatch + randomIntBetween(1, 100), @@ -173,18 +175,19 @@ public void testSortAndPaginateWithInProgress() throws Exception { } awaitNumberOfSnapshotsInProgress(inProgressCount); - assertStablePagination(repoName, allSnapshotNames, GetSnapshotsRequest.SortBy.START_TIME); - assertStablePagination(repoName, allSnapshotNames, GetSnapshotsRequest.SortBy.NAME); - assertStablePagination(repoName, allSnapshotNames, GetSnapshotsRequest.SortBy.INDICES); + final String[] repos = { repoName }; + assertStablePagination(repos, allSnapshotNames, GetSnapshotsRequest.SortBy.START_TIME); + assertStablePagination(repos, allSnapshotNames, GetSnapshotsRequest.SortBy.NAME); + assertStablePagination(repos, allSnapshotNames, GetSnapshotsRequest.SortBy.INDICES); unblockAllDataNodes(repoName); for (ActionFuture inProgressSnapshot : inProgressSnapshots) { assertSuccessful(inProgressSnapshot); } - assertStablePagination(repoName, allSnapshotNames, GetSnapshotsRequest.SortBy.START_TIME); - assertStablePagination(repoName, allSnapshotNames, GetSnapshotsRequest.SortBy.NAME); - assertStablePagination(repoName, allSnapshotNames, GetSnapshotsRequest.SortBy.INDICES); + assertStablePagination(repos, allSnapshotNames, GetSnapshotsRequest.SortBy.START_TIME); + assertStablePagination(repos, allSnapshotNames, GetSnapshotsRequest.SortBy.NAME); + assertStablePagination(repos, allSnapshotNames, GetSnapshotsRequest.SortBy.INDICES); } public void testPaginationRequiresVerboseListing() throws Exception { @@ -229,10 +232,15 @@ public void testExcludePatterns() throws Exception { allSnapshotNames.addAll(namesOtherRepo); final SortOrder order = SortOrder.DESC; - final List allSorted = allSnapshotsSorted(allSnapshotNames, "*", GetSnapshotsRequest.SortBy.REPOSITORY, order); + final List allSorted = allSnapshotsSorted( + allSnapshotNames, + new String[] { "*" }, + GetSnapshotsRequest.SortBy.REPOSITORY, + order + ); final List allSortedWithoutOther = allSnapshotsSorted( allSnapshotNamesWithoutOther, - "*,-" + otherRepo, + new String[] { "*", "-" + otherRepo }, GetSnapshotsRequest.SortBy.REPOSITORY, order ); @@ -240,7 +248,7 @@ public void testExcludePatterns() throws Exception { final List allInOther = allSnapshotsSorted( namesOtherRepo, - "*,-test-repo-*", + new String[] { "*", "-test-repo-*" }, GetSnapshotsRequest.SortBy.REPOSITORY, order ); @@ -251,7 +259,7 @@ public void testExcludePatterns() throws Exception { final String otherPrefixSnapshot2 = "other-prefix-snapshot-2"; createFullSnapshot(otherRepo, otherPrefixSnapshot2); - final String patternOtherRepo = randomBoolean() ? otherRepo : "*,-test-repo-*"; + final String[] patternOtherRepo = randomBoolean() ? new String[] { otherRepo } : new String[] { "*", "-test-repo-*" }; final List allInOtherWithoutOtherPrefix = allSnapshotsSorted( namesOtherRepo, patternOtherRepo, @@ -330,12 +338,12 @@ private static List getAllSnapshotsForPolicies(String... policies) .getSnapshots(); } - private static void assertStablePagination(String repoName, Collection allSnapshotNames, GetSnapshotsRequest.SortBy sort) { + private static void assertStablePagination(String[] repoNames, Collection allSnapshotNames, GetSnapshotsRequest.SortBy sort) { final SortOrder order = randomFrom(SortOrder.values()); - final List allSorted = allSnapshotsSorted(allSnapshotNames, repoName, sort, order); + final List allSorted = allSnapshotsSorted(allSnapshotNames, repoNames, sort, order); for (int i = 1; i <= allSnapshotNames.size(); i++) { - final GetSnapshotsResponse subsetSorted = sortedWithLimit(repoName, sort, null, i, order); + final GetSnapshotsResponse subsetSorted = sortedWithLimit(repoNames, sort, null, i, order); assertEquals(allSorted.subList(0, i), subsetSorted.getSnapshots()); } @@ -343,13 +351,13 @@ private static void assertStablePagination(String repoName, Collection a final SnapshotInfo after = allSorted.get(j); for (int i = 1; i < allSnapshotNames.size() - j; i++) { final GetSnapshotsResponse getSnapshotsResponse = sortedWithLimit( - repoName, + repoNames, sort, GetSnapshotsRequest.After.from(after, sort).asQueryParam(), i, order ); - final GetSnapshotsResponse getSnapshotsResponseNumeric = sortedWithLimit(repoName, sort, j + 1, i, order); + final GetSnapshotsResponse getSnapshotsResponseNumeric = sortedWithLimit(repoNames, sort, j + 1, i, order); final List subsetSorted = getSnapshotsResponse.getSnapshots(); assertEquals(subsetSorted, getSnapshotsResponseNumeric.getSnapshots()); assertEquals(subsetSorted, allSorted.subList(j + 1, j + i + 1)); @@ -364,18 +372,18 @@ private static void assertStablePagination(String repoName, Collection a private static List allSnapshotsSorted( Collection allSnapshotNames, - String repoName, + String[] repoNames, GetSnapshotsRequest.SortBy sortBy, SortOrder order, - String... extraPatterns + String... namePatterns ) { final GetSnapshotsResponse getSnapshotsResponse = sortedWithLimit( - repoName, + repoNames, sortBy, null, GetSnapshotsRequest.NO_LIMIT, order, - extraPatterns + namePatterns ); final List snapshotInfos = getSnapshotsResponse.getSnapshots(); assertEquals(snapshotInfos.size(), allSnapshotNames.size()); @@ -388,33 +396,33 @@ private static List allSnapshotsSorted( } private static GetSnapshotsResponse sortedWithLimit( - String repoName, + String[] repoNames, GetSnapshotsRequest.SortBy sortBy, String after, int size, SortOrder order, - String... extraPatterns + String... namePatterns ) { - return baseGetSnapshotsRequest(repoName).setAfter(after) + return baseGetSnapshotsRequest(repoNames).setAfter(after) .setSort(sortBy) .setSize(size) .setOrder(order) - .addSnapshots(extraPatterns) + .addSnapshots(namePatterns) .get(); } private static GetSnapshotsResponse sortedWithLimit( - String repoName, + String[] repoNames, GetSnapshotsRequest.SortBy sortBy, int offset, int size, SortOrder order ) { - return baseGetSnapshotsRequest(repoName).setOffset(offset).setSort(sortBy).setSize(size).setOrder(order).get(); + return baseGetSnapshotsRequest(repoNames).setOffset(offset).setSort(sortBy).setSize(size).setOrder(order).get(); } - private static GetSnapshotsRequestBuilder baseGetSnapshotsRequest(String repoNamePattern) { - return clusterAdmin().prepareGetSnapshots(repoNamePattern.split(",")) + private static GetSnapshotsRequestBuilder baseGetSnapshotsRequest(String[] repoNames) { + return clusterAdmin().prepareGetSnapshots(repoNames) .setSnapshots("*", "-" + AbstractSnapshotIntegTestCase.OLD_VERSION_SNAPSHOT_PREFIX + "*"); } } From 74e76f61d8ddd3a1a295a0cd849ad0ce80b7b6ef Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 8 Sep 2021 12:22:33 +0200 Subject: [PATCH 4/9] test with dash --- .../snapshots/GetSnapshotsIT.java | 84 +++++++++++++++++-- 1 file changed, 78 insertions(+), 6 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java index ea1804cf23630..4ae421236983d 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java @@ -10,6 +10,7 @@ import org.elasticsearch.action.ActionFuture; import org.elasticsearch.action.ActionRequestValidationException; +import org.elasticsearch.action.admin.cluster.repositories.get.TransportGetRepositoriesAction; 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.GetSnapshotsRequestBuilder; @@ -27,6 +28,7 @@ import java.util.Set; import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.in; import static org.hamcrest.Matchers.is; @@ -280,12 +282,82 @@ public void testExcludePatterns() throws Exception { assertThat(allInOtherWithoutOtherExplicit, is(allInOther)); } + public void testNamesStartingInDash() { + final String repoName1 = "test-repo"; + final String weirdRepo1 = "-weird-repo-1"; + final String weirdRepo2 = "-weird-repo-2"; + createRepository(repoName1, "fs"); + createRepository(weirdRepo1, "fs"); + createRepository(weirdRepo2, "fs"); + + final String snapshotName = "test-snapshot"; + final String weirdSnapshot1 = "-weird-snapshot-1"; + final String weirdSnapshot2 = "-weird-snapshot-2"; + + final SnapshotInfo snapshotInRepo1 = createFullSnapshot(repoName1, snapshotName); + final SnapshotInfo weirdSnapshot1InRepo1 = createFullSnapshot(repoName1, weirdSnapshot1); + createFullSnapshot(repoName1, weirdSnapshot2); + + final SnapshotInfo snapshotInWeird1 = createFullSnapshot(weirdRepo1, snapshotName); + final SnapshotInfo weirdSnapshot1InWeird1 = createFullSnapshot(weirdRepo1, weirdSnapshot1); + createFullSnapshot(weirdRepo1, weirdSnapshot2); + + final SnapshotInfo snapshotInWeird2 = createFullSnapshot(weirdRepo2, snapshotName); + final SnapshotInfo weirdSnapshot1InWeird2 = createFullSnapshot(weirdRepo2, weirdSnapshot1); + createFullSnapshot(weirdRepo2, weirdSnapshot2); + + final List allSnapshots = clusterAdmin().prepareGetSnapshots(matchAllPattern()) + .setSort(GetSnapshotsRequest.SortBy.REPOSITORY) + .get() + .getSnapshots(); + assertThat(allSnapshots, hasSize(9)); + + final List allSnapshotsByAll = getAllByPatterns(matchAllPattern(), matchAllPattern()); + assertThat(allSnapshotsByAll, is(allSnapshots)); + assertThat(getAllByPatterns(matchAllPattern(), new String[] { snapshotName, weirdSnapshot1, weirdSnapshot2 }), is(allSnapshots)); + assertThat(getAllByPatterns(new String[] { repoName1, weirdRepo1, weirdRepo2 }, matchAllPattern()), is(allSnapshots)); + + assertThat( + getAllByPatterns(matchAllPattern(), new String[] { snapshotName }), + is(List.of(snapshotInWeird1, snapshotInWeird2, snapshotInRepo1)) + ); + assertThat( + getAllByPatterns(matchAllPattern(), new String[] { weirdSnapshot1 }), + is(List.of(weirdSnapshot1InWeird1, weirdSnapshot1InWeird2, weirdSnapshot1InRepo1)) + ); + assertThat( + getAllByPatterns(matchAllPattern(), new String[] { snapshotName, weirdSnapshot1 }), + is( + List.of( + weirdSnapshot1InWeird1, + snapshotInWeird1, + weirdSnapshot1InWeird2, + snapshotInWeird2, + weirdSnapshot1InRepo1, + snapshotInRepo1 + ) + ) + ); + } + + private static String[] matchAllPattern() { + return randomBoolean() ? new String[] { "*" } : new String[] { TransportGetRepositoriesAction.ALL_PATTERN }; + } + + private List getAllByPatterns(String[] repos, String[] snapshots) { + return clusterAdmin().prepareGetSnapshots(repos) + .setSnapshots(snapshots) + .setSort(GetSnapshotsRequest.SortBy.REPOSITORY) + .get() + .getSnapshots(); + } + public void testFilterBySLMPolicy() throws Exception { final String repoName = "test-repo"; createRepository(repoName, "fs"); createNSnapshots(repoName, randomIntBetween(1, 5)); - final List snapshotsWithoutPolicy = clusterAdmin().prepareGetSnapshots("*") - .setSnapshots("*") + final List snapshotsWithoutPolicy = clusterAdmin().prepareGetSnapshots(matchAllPattern()) + .setSnapshots(matchAllPattern()) .setSort(GetSnapshotsRequest.SortBy.NAME) .get() .getSnapshots(); @@ -320,8 +392,8 @@ public void testFilterBySLMPolicy() throws Exception { assertThat(getAllSnapshotsForPolicies(policyName, otherPolicyName), is(List.of(withOtherPolicy, withPolicy))); assertThat(getAllSnapshotsForPolicies(policyName, otherPolicyName, "no-such-policy*"), is(List.of(withOtherPolicy, withPolicy))); - final List allSnapshots = clusterAdmin().prepareGetSnapshots("*") - .setSnapshots("*") + final List allSnapshots = clusterAdmin().prepareGetSnapshots(matchAllPattern()) + .setSnapshots(matchAllPattern()) .setSort(GetSnapshotsRequest.SortBy.NAME) .get() .getSnapshots(); @@ -330,8 +402,8 @@ public void testFilterBySLMPolicy() throws Exception { } private static List getAllSnapshotsForPolicies(String... policies) { - return clusterAdmin().prepareGetSnapshots("*") - .setSnapshots("*") + return clusterAdmin().prepareGetSnapshots(matchAllPattern()) + .setSnapshots(matchAllPattern()) .setPolicies(policies) .setSort(GetSnapshotsRequest.SortBy.NAME) .get() From de70f55fdc877d5625fec3a93b05287c85c66908 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 8 Sep 2021 12:37:20 +0200 Subject: [PATCH 5/9] CR comments --- .../snapshot-restore/apis/get-snapshot-api.asciidoc | 8 ++++---- .../java/org/elasticsearch/snapshots/GetSnapshotsIT.java | 7 +++++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc index d157effe4c0ea..c6ceeeadff3f8 100644 --- a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc +++ b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc @@ -60,7 +60,7 @@ Use the get snapshot API to return information about one or more snapshots, incl ``:: (Required, string) Comma-separated list of snapshot repository names used to limit the request. -Wildcard (`*`) expressions are supported including combining wildcards with exclude patterns starting in `-`. +Wildcard (`*`) expressions are supported including combining wildcards with exclude patterns starting with `-`. + To get information about all snapshot repositories registered in the cluster, omit this parameter or use `*` or `_all`. @@ -68,7 +68,7 @@ cluster, omit this parameter or use `*` or `_all`. ``:: (Required, string) Comma-separated list of snapshot names to retrieve. Also accepts wildcards (`*`) as well as combining wildcards and exclude patterns -starting in `-`. +starting with `-`. + * To get information about all snapshots in a registered repository, use a wildcard (`*`) or `_all`. * To get information about any snapshots that are currently running, use `_current`. @@ -152,8 +152,8 @@ exclusive with using the `after` parameter. Defaults to `0`. `slm_policy_filter`:: (Optional, string) Filter snapshots by a comma-separated list of SLM policy names that snapshots belong to. Also accepts wildcards (`\*`) and combinations -of wildcards followed by exclude patterns starting in `-`. For example, the pattern `*,-policy-a-\*` will return all snapshots except -for those that were created by an SLM policy with a name starting in `policy-a-`. Note that the wildcard pattern `*` matches all snapshots +of wildcards followed by exclude patterns starting with `-`. For example, the pattern `*,-policy-a-\*` will return all snapshots except +for those that were created by an SLM policy with a name starting with `policy-a-`. Note that the wildcard pattern `*` matches all snapshots created by an SLM policy but not those snapshots that were not created by an SLM policy. To include snapshots not created by an SLM policy you can use the special pattern `_none` that will match all snapshots without an SLM policy. diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java index 4ae421236983d..f23171543f728 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java @@ -246,7 +246,7 @@ public void testExcludePatterns() throws Exception { GetSnapshotsRequest.SortBy.REPOSITORY, order ); - assertThat(allSorted.subList(0, allSnapshotNamesWithoutOther.size()), is(allSortedWithoutOther)); + assertThat(allSortedWithoutOther, is(allSorted.subList(0, allSnapshotNamesWithoutOther.size()))); final List allInOther = allSnapshotsSorted( namesOtherRepo, @@ -254,7 +254,7 @@ public void testExcludePatterns() throws Exception { GetSnapshotsRequest.SortBy.REPOSITORY, order ); - assertThat(allSorted.subList(allSnapshotNamesWithoutOther.size(), allSorted.size()), is(allInOther)); + assertThat(allInOther, is(allSorted.subList(allSnapshotNamesWithoutOther.size(), allSorted.size()))); final String otherPrefixSnapshot1 = "other-prefix-snapshot-1"; createFullSnapshot(otherRepo, otherPrefixSnapshot1); @@ -280,6 +280,9 @@ public void testExcludePatterns() throws Exception { "-" + otherPrefixSnapshot2 ); assertThat(allInOtherWithoutOtherExplicit, is(allInOther)); + + assertThat(clusterAdmin().prepareGetSnapshots(matchAllPattern()).setSnapshots("other*", "-o*").get().getSnapshots(), empty()); + assertThat(clusterAdmin().prepareGetSnapshots("other*", "-o*").setSnapshots(matchAllPattern()).get().getSnapshots(), empty()); } public void testNamesStartingInDash() { From fc296ed596ec3cc9f9256b15c9302765cb634e63 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Thu, 9 Sep 2021 12:53:16 +0200 Subject: [PATCH 6/9] tests --- .../apis/get-repo-api.asciidoc | 2 +- .../apis/get-snapshot-api.asciidoc | 3 +- .../snapshots/GetSnapshotsIT.java | 34 +++++++++++++++++-- 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/docs/reference/snapshot-restore/apis/get-repo-api.asciidoc b/docs/reference/snapshot-restore/apis/get-repo-api.asciidoc index 7360f550b46b4..68c3b3dd0ec51 100644 --- a/docs/reference/snapshot-restore/apis/get-repo-api.asciidoc +++ b/docs/reference/snapshot-restore/apis/get-repo-api.asciidoc @@ -46,7 +46,7 @@ GET /_snapshot/my_repository ``:: (Optional, string) Comma-separated list of snapshot repository names used to limit the request. -Wildcard (`*`) expressions are supported. +Wildcard (`*`) expressions are supported including combining wildcards with exclude patterns starting with `-`. + To get information about all snapshot repositories registered in the cluster, omit this parameter or use `*` or `_all`. diff --git a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc index c6ceeeadff3f8..50a73fd8f95fa 100644 --- a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc +++ b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc @@ -67,8 +67,7 @@ cluster, omit this parameter or use `*` or `_all`. ``:: (Required, string) -Comma-separated list of snapshot names to retrieve. Also accepts wildcards (`*`) as well as combining wildcards and exclude patterns -starting with `-`. +Wildcard (`*`) expressions are supported including combining wildcards with exclude patterns starting with `-`. + * To get information about all snapshots in a registered repository, use a wildcard (`*`) or `_all`. * To get information about any snapshots that are currently running, use `_current`. diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java index f23171543f728..d68202312d30c 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java @@ -299,15 +299,15 @@ public void testNamesStartingInDash() { final SnapshotInfo snapshotInRepo1 = createFullSnapshot(repoName1, snapshotName); final SnapshotInfo weirdSnapshot1InRepo1 = createFullSnapshot(repoName1, weirdSnapshot1); - createFullSnapshot(repoName1, weirdSnapshot2); + final SnapshotInfo weirdSnapshot2InRepo1 = createFullSnapshot(repoName1, weirdSnapshot2); final SnapshotInfo snapshotInWeird1 = createFullSnapshot(weirdRepo1, snapshotName); final SnapshotInfo weirdSnapshot1InWeird1 = createFullSnapshot(weirdRepo1, weirdSnapshot1); - createFullSnapshot(weirdRepo1, weirdSnapshot2); + final SnapshotInfo weirdSnapshot2InWeird1 = createFullSnapshot(weirdRepo1, weirdSnapshot2); final SnapshotInfo snapshotInWeird2 = createFullSnapshot(weirdRepo2, snapshotName); final SnapshotInfo weirdSnapshot1InWeird2 = createFullSnapshot(weirdRepo2, weirdSnapshot1); - createFullSnapshot(weirdRepo2, weirdSnapshot2); + final SnapshotInfo weirdSnapshot2InWeird2 = createFullSnapshot(weirdRepo2, weirdSnapshot2); final List allSnapshots = clusterAdmin().prepareGetSnapshots(matchAllPattern()) .setSort(GetSnapshotsRequest.SortBy.REPOSITORY) @@ -341,6 +341,34 @@ public void testNamesStartingInDash() { ) ) ); + assertThat(getAllByPatterns(matchAllPattern(), new String[] { "non-existing*", weirdSnapshot1 }), empty()); + assertThat( + getAllByPatterns(matchAllPattern(), new String[] { "*", "--weird-snapshot-1" }), + is( + List.of( + weirdSnapshot2InWeird1, + snapshotInWeird1, + weirdSnapshot2InWeird2, + snapshotInWeird2, + weirdSnapshot2InRepo1, + snapshotInRepo1 + ) + ) + ); + assertThat( + getAllByPatterns(matchAllPattern(), new String[] { "-*" }), + is( + List.of( + weirdSnapshot1InWeird1, + weirdSnapshot2InWeird1, + weirdSnapshot1InWeird2, + weirdSnapshot2InWeird2, + weirdSnapshot1InRepo1, + weirdSnapshot2InRepo1 + + ) + ) + ); } private static String[] matchAllPattern() { From bcecd6fdb3a5fc93fbae3ff075ae1103ae87be75 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Fri, 10 Sep 2021 11:03:51 +0200 Subject: [PATCH 7/9] desired concrete name + wildcard behavior --- .../java/org/elasticsearch/snapshots/GetSnapshotsIT.java | 4 ++++ .../cluster/snapshots/get/TransportGetSnapshotsAction.java | 6 ++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java index d68202312d30c..9381393b3000b 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java @@ -283,6 +283,10 @@ public void testExcludePatterns() throws Exception { assertThat(clusterAdmin().prepareGetSnapshots(matchAllPattern()).setSnapshots("other*", "-o*").get().getSnapshots(), empty()); assertThat(clusterAdmin().prepareGetSnapshots("other*", "-o*").setSnapshots(matchAllPattern()).get().getSnapshots(), empty()); + assertThat(clusterAdmin().prepareGetSnapshots("other*", otherRepo, "-o*").setSnapshots(matchAllPattern()).get() + .getSnapshots(), empty()); + assertThat(clusterAdmin().prepareGetSnapshots(matchAllPattern()).setSnapshots("non-existing*", otherPrefixSnapshot1, "-o*") + .get().getSnapshots(), empty()); } public void testNamesStartingInDash() { diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java index aaef3addb157b..52ce6e6cd4beb 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java @@ -313,12 +313,10 @@ private void loadSnapshotInfos( } else if (GetSnapshotsRequest.CURRENT_SNAPSHOT.equalsIgnoreCase(snapshotOrPattern)) { toResolve.addAll(currentSnapshots.stream().map(SnapshotInfo::snapshot).collect(Collectors.toList())); } else { - final Snapshot found = allSnapshotIds.get(snapshotOrPattern); - if (found != null) { - toResolve.add(found); - } else if (ignoreUnavailable == false) { + if (ignoreUnavailable == false && allSnapshotIds.containsKey(snapshotOrPattern) == false) { throw new SnapshotMissingException(repo, snapshotOrPattern); } + includePatterns.add(snapshotOrPattern); } } } From ffe74847f5af178408840320c1b5ed2515e84a58 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Fri, 10 Sep 2021 11:24:25 +0200 Subject: [PATCH 8/9] _current is a wildcard pattern now --- .../snapshots/GetSnapshotsIT.java | 23 +++++++++++++++---- .../get/TransportGetSnapshotsAction.java | 12 +++++++++- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java index 9381393b3000b..d6a124ff8f587 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java @@ -182,6 +182,14 @@ public void testSortAndPaginateWithInProgress() throws Exception { assertStablePagination(repos, allSnapshotNames, GetSnapshotsRequest.SortBy.NAME); assertStablePagination(repos, allSnapshotNames, GetSnapshotsRequest.SortBy.INDICES); + assertThat( + clusterAdmin().prepareGetSnapshots(matchAllPattern()) + .setSnapshots(GetSnapshotsRequest.CURRENT_SNAPSHOT, "-snap*") + .get() + .getSnapshots(), + empty() + ); + unblockAllDataNodes(repoName); for (ActionFuture inProgressSnapshot : inProgressSnapshots) { assertSuccessful(inProgressSnapshot); @@ -283,10 +291,17 @@ public void testExcludePatterns() throws Exception { assertThat(clusterAdmin().prepareGetSnapshots(matchAllPattern()).setSnapshots("other*", "-o*").get().getSnapshots(), empty()); assertThat(clusterAdmin().prepareGetSnapshots("other*", "-o*").setSnapshots(matchAllPattern()).get().getSnapshots(), empty()); - assertThat(clusterAdmin().prepareGetSnapshots("other*", otherRepo, "-o*").setSnapshots(matchAllPattern()).get() - .getSnapshots(), empty()); - assertThat(clusterAdmin().prepareGetSnapshots(matchAllPattern()).setSnapshots("non-existing*", otherPrefixSnapshot1, "-o*") - .get().getSnapshots(), empty()); + assertThat( + clusterAdmin().prepareGetSnapshots("other*", otherRepo, "-o*").setSnapshots(matchAllPattern()).get().getSnapshots(), + empty() + ); + assertThat( + clusterAdmin().prepareGetSnapshots(matchAllPattern()) + .setSnapshots("non-existing*", otherPrefixSnapshot1, "-o*") + .get() + .getSnapshots(), + empty() + ); } public void testNamesStartingInDash() { diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java index 52ce6e6cd4beb..709ac08a15869 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java @@ -302,6 +302,7 @@ private void loadSnapshotInfos( } else { final List includePatterns = new ArrayList<>(); final List excludePatterns = new ArrayList<>(); + boolean hasCurrent = false; boolean seenWildcard = false; for (String snapshotOrPattern : snapshots) { if (seenWildcard && snapshotOrPattern.length() > 1 && snapshotOrPattern.startsWith("-")) { @@ -311,7 +312,8 @@ private void loadSnapshotInfos( seenWildcard = true; includePatterns.add(snapshotOrPattern); } else if (GetSnapshotsRequest.CURRENT_SNAPSHOT.equalsIgnoreCase(snapshotOrPattern)) { - toResolve.addAll(currentSnapshots.stream().map(SnapshotInfo::snapshot).collect(Collectors.toList())); + hasCurrent = true; + seenWildcard = true; } else { if (ignoreUnavailable == false && allSnapshotIds.containsKey(snapshotOrPattern) == false) { throw new SnapshotMissingException(repo, snapshotOrPattern); @@ -330,6 +332,14 @@ private void loadSnapshotInfos( toResolve.add(snapshot); } } + if (hasCurrent) { + for (SnapshotInfo snapshotInfo : currentSnapshots) { + final Snapshot snapshot = snapshotInfo.snapshot(); + if (Regex.simpleMatch(excludes, snapshot.getSnapshotId().getName()) == false) { + toResolve.add(snapshot); + } + } + } if (toResolve.isEmpty() && ignoreUnavailable == false && isCurrentSnapshotsOnly(snapshots) == false) { throw new SnapshotMissingException(repo, snapshots[0]); } From e7aa9660f72500e227ecb67edec9b16b2011b3f7 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Fri, 10 Sep 2021 13:54:17 +0200 Subject: [PATCH 9/9] missing sentence --- docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc index 50a73fd8f95fa..6ff2fa1506c95 100644 --- a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc +++ b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc @@ -67,6 +67,7 @@ cluster, omit this parameter or use `*` or `_all`. ``:: (Required, string) +Comma-separated list of snapshot names to retrieve. Wildcard (`*`) expressions are supported including combining wildcards with exclude patterns starting with `-`. + * To get information about all snapshots in a registered repository, use a wildcard (`*`) or `_all`.