From 18fd911dd6758351796b31b90a9b7b50de49d147 Mon Sep 17 00:00:00 2001 From: Elena Stoeva Date: Thu, 27 Feb 2025 14:39:41 +0000 Subject: [PATCH 1/6] Add state query param to Get snapshots API --- .../apis/get-snapshot-api.asciidoc | 4 ++ .../rest-api-spec/api/snapshot.get.json | 4 ++ .../snapshots/GetSnapshotsIT.java | 52 +++++++++++++++++++ .../snapshots/get/GetSnapshotsRequest.java | 20 +++++++ .../get/GetSnapshotsRequestBuilder.java | 4 ++ .../get/TransportGetSnapshotsAction.java | 12 ++++- .../admin/cluster/RestGetSnapshotsAction.java | 1 + .../get/GetSnapshotsRequestTests.java | 6 +++ 8 files changed, 101 insertions(+), 2 deletions(-) diff --git a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc index f9eb6a27df039..a0b7ba796f61d 100644 --- a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc +++ b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc @@ -182,6 +182,10 @@ for those that were created by an SLM policy with a name starting with `policy-a 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. +`state`:: +(Optional, string) +Filter snapshots by state. Valid values are `SUCCESS`, `IN_PROGRESS`, `FAILED`, `PARTIAL`, or `INCOMPATIBLE`. + NOTE: The `after` parameter and `next` field allow for iterating through snapshots with some consistency guarantees regarding concurrent creation or deletion of snapshots. It is guaranteed that any snapshot that exists at the beginning of the iteration and is not concurrently deleted will be seen during the iteration. Snapshots concurrently created may be seen during an iteration. 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 23f5f737995d0..8362686e87f6a 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 @@ -85,6 +85,10 @@ "verbose":{ "type":"boolean", "description":"Whether to show verbose snapshot info or only show the basic info found in the repository index blob" + }, + "state": { + "type": "string", + "description": "Filter snapshots by state. Valid values are 'SUCCESS', 'IN_PROGRESS', 'FAILED', 'PARTIAL', or 'INCOMPATIBLE'." } } } diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java index c0e63520fee9e..5233ac3f5cf8e 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java @@ -69,6 +69,7 @@ import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.in; import static org.hamcrest.Matchers.is; +import static org.hamcrest.core.StringContains.containsString; public class GetSnapshotsIT extends AbstractSnapshotIntegTestCase { @@ -633,6 +634,57 @@ public void testRetrievingSnapshotsWhenRepositoryIsMissing() throws Exception { expectThrows(RepositoryMissingException.class, multiRepoFuture::actionGet); } + public void testFilterByState() throws Exception { + final String repoName = "test-repo"; + final Path repoPath = randomRepoPath(); + createRepository(repoName, "mock", repoPath); + maybeInitWithOldSnapshotVersion(repoName, repoPath); + + // Create a successful snapshot + String successSnapshot = "snapshot-success"; + createFullSnapshot(repoName, successSnapshot); + + // Create a snapshot in progress + String inProgressSnapshot = "snapshot-in-progress"; + blockAllDataNodes(repoName); + startFullSnapshot(repoName, inProgressSnapshot); + awaitNumberOfSnapshotsInProgress(1); + awaitClusterState( + state -> SnapshotsInProgress.get(state) + .asStream() + .flatMap(s -> s.shards().entrySet().stream()) + .allMatch( + e -> e.getKey().getIndexName().equals("test-index-1") == false + || e.getValue().state() == SnapshotsInProgress.ShardState.SUCCESS + ) + ); + + // Fetch all snapshots + GetSnapshotsResponse responseAll = clusterAdmin().prepareGetSnapshots(TEST_REQUEST_TIMEOUT, repoName).get(); + assertThat(responseAll.getSnapshots(), hasSize(2)); + + // Fetch snapshots with state=SUCCESS + GetSnapshotsResponse responseSuccess = clusterAdmin().prepareGetSnapshots(TEST_REQUEST_TIMEOUT, repoName) + .setState(String.valueOf(SnapshotState.SUCCESS)) + .get(); + assertThat(responseSuccess.getSnapshots(), hasSize(1)); + assertThat(responseSuccess.getSnapshots().get(0).state(), is(SnapshotState.SUCCESS)); + + // Fetch snapshots with state=IN_PROGRESS + GetSnapshotsResponse responseInProgress = clusterAdmin().prepareGetSnapshots(TEST_REQUEST_TIMEOUT, repoName) + .setState(String.valueOf(SnapshotState.IN_PROGRESS)) + .get(); + assertThat(responseInProgress.getSnapshots(), hasSize(1)); + assertThat(responseInProgress.getSnapshots().get(0).state(), is(SnapshotState.IN_PROGRESS)); + + // Fetch snapshots with invalid state + IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> clusterAdmin().prepareGetSnapshots(TEST_REQUEST_TIMEOUT, repoName).setState("INVALID_STATE").get() + ); + assertThat(e.getMessage(), containsString("state must be SUCCESS, IN_PROGRESS, FAILED, PARTIAL, or INCOMPATIBLE")); + } + // Create a snapshot that is guaranteed to have a unique start time and duration for tests around ordering by either. // Don't use this with more than 3 snapshots on platforms with low-resolution clocks as the durations could always collide there // causing an infinite loop 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 b99059e6711ca..e6f73ff592d63 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 @@ -19,6 +19,7 @@ import org.elasticsearch.core.Nullable; import org.elasticsearch.core.TimeValue; import org.elasticsearch.search.sort.SortOrder; +import org.elasticsearch.snapshots.SnapshotState; import org.elasticsearch.tasks.CancellableTask; import org.elasticsearch.tasks.Task; import org.elasticsearch.tasks.TaskId; @@ -77,6 +78,8 @@ public class GetSnapshotsRequest extends MasterNodeRequest private boolean includeIndexNames = true; + private String state; + public GetSnapshotsRequest(TimeValue masterNodeTimeout) { super(masterNodeTimeout); } @@ -118,6 +121,7 @@ public GetSnapshotsRequest(StreamInput in) throws IOException { if (in.getTransportVersion().onOrAfter(INDICES_FLAG_VERSION)) { includeIndexNames = in.readBoolean(); } + state = in.readOptionalString(); } @Override @@ -137,6 +141,7 @@ public void writeTo(StreamOutput out) throws IOException { if (out.getTransportVersion().onOrAfter(INDICES_FLAG_VERSION)) { out.writeBoolean(includeIndexNames); } + out.writeOptionalString(state); } @Override @@ -177,6 +182,12 @@ public ActionRequestValidationException validate() { } else if (after != null && fromSortValue != null) { validationException = addValidationError("can't use after and from_sort_value simultaneously", validationException); } + if (state != null && !(Arrays.stream(SnapshotState.values()).anyMatch(s -> s.name().equalsIgnoreCase(state)))) { + validationException = addValidationError( + "state must be SUCCESS, IN_PROGRESS, FAILED, PARTIAL, or INCOMPATIBLE", + validationException + ); + } return validationException; } @@ -342,6 +353,15 @@ public boolean verbose() { return verbose; } + public String state() { + return state; + } + + public GetSnapshotsRequest state(String state) { + this.state = state; + return this; + } + @Override public Task createTask(long id, String type, String action, TaskId parentTaskId, Map headers) { return new CancellableTask(id, type, action, getDescription(), parentTaskId, headers); diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestBuilder.java index 4ace7536a86e1..96ae685269f53 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestBuilder.java @@ -150,4 +150,8 @@ public GetSnapshotsRequestBuilder setIncludeIndexNames(boolean indices) { } + public GetSnapshotsRequestBuilder setState(String state) { + request.state(state); + return this; + } } 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 ec4a578ef25cd..b273949735994 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 @@ -162,7 +162,8 @@ protected void masterOperation( request.size(), SnapshotsInProgress.get(state), request.verbose(), - request.includeIndexNames() + request.includeIndexNames(), + request.state() ).runOperation(listener); } @@ -183,6 +184,7 @@ private class GetSnapshotsOperation { private final SnapshotNamePredicate snapshotNamePredicate; private final SnapshotPredicates fromSortValuePredicates; private final Predicate slmPolicyPredicate; + private final String state; // snapshot ordering/pagination private final SnapshotSortKey sortBy; @@ -226,7 +228,8 @@ private class GetSnapshotsOperation { int size, SnapshotsInProgress snapshotsInProgress, boolean verbose, - boolean indices + boolean indices, + String state ) { this.cancellableTask = cancellableTask; this.repositories = repositories; @@ -239,6 +242,7 @@ private class GetSnapshotsOperation { this.snapshotsInProgress = snapshotsInProgress; this.verbose = verbose; this.indices = indices; + this.state = state; this.snapshotNamePredicate = SnapshotNamePredicate.forSnapshots(ignoreUnavailable, snapshots); this.fromSortValuePredicates = SnapshotPredicates.forFromSortValue(fromSortValue, sortBy, order); @@ -573,6 +577,10 @@ private boolean matchesPredicates(SnapshotInfo snapshotInfo) { return false; } + if (state != null) { + return state.equalsIgnoreCase(snapshotInfo.state().toString()); + } + if (slmPolicyPredicate == SlmPolicyPredicate.MATCH_ALL_POLICIES) { return true; } diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestGetSnapshotsAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestGetSnapshotsAction.java index b3a6430822020..014bb09159d4f 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestGetSnapshotsAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestGetSnapshotsAction.java @@ -82,6 +82,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC final SortOrder order = SortOrder.fromString(request.param("order", getSnapshotsRequest.order().toString())); getSnapshotsRequest.order(order); getSnapshotsRequest.includeIndexNames(request.paramAsBoolean(INDEX_NAMES_XCONTENT_PARAM, getSnapshotsRequest.includeIndexNames())); + getSnapshotsRequest.state(request.param("state")); return channel -> new RestCancellableNodeClient(client, request.getHttpChannel()).admin() .cluster() .getSnapshots(getSnapshotsRequest, new RestRefCountedChunkedToXContentListener<>(channel)); diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestTests.java index b2b1396934992..879b39d45cc81 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestTests.java @@ -86,6 +86,12 @@ public void testValidateParameters() { final ActionRequestValidationException e = request.validate(); assertThat(e.getMessage(), containsString("can't use slm policy filter with verbose=false")); } + { + final GetSnapshotsRequest request = new GetSnapshotsRequest(TEST_REQUEST_TIMEOUT, "repo", "snapshot").state("foo") + .verbose(false); + final ActionRequestValidationException e = request.validate(); + assertThat(e.getMessage(), containsString("state must be SUCCESS, IN_PROGRESS, FAILED, PARTIAL, or INCOMPATIBLE")); + } } public void testGetDescription() { From d80ec190ae8f0516dd55beb716dcfbb4fdf4f97c Mon Sep 17 00:00:00 2001 From: Elena Stoeva Date: Fri, 28 Feb 2025 13:54:41 +0000 Subject: [PATCH 2/6] Fix checkstyle error --- .../action/admin/cluster/snapshots/get/GetSnapshotsRequest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 e6f73ff592d63..41d4a1bc7fb9b 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 @@ -182,7 +182,7 @@ public ActionRequestValidationException validate() { } else if (after != null && fromSortValue != null) { validationException = addValidationError("can't use after and from_sort_value simultaneously", validationException); } - if (state != null && !(Arrays.stream(SnapshotState.values()).anyMatch(s -> s.name().equalsIgnoreCase(state)))) { + if (state != null && Arrays.stream(SnapshotState.values()).noneMatch(s -> s.name().equalsIgnoreCase(state))) { validationException = addValidationError( "state must be SUCCESS, IN_PROGRESS, FAILED, PARTIAL, or INCOMPATIBLE", validationException From 712138fd9917207a72a48e84e3dd4aa068dbeaf8 Mon Sep 17 00:00:00 2001 From: Elena Stoeva Date: Fri, 28 Feb 2025 13:56:26 +0000 Subject: [PATCH 3/6] Resolve merge conflicts --- .../reference/snapshot-restore/apis/get-snapshot-api.asciidoc | 4 ---- 1 file changed, 4 deletions(-) diff --git a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc index a0b7ba796f61d..f9eb6a27df039 100644 --- a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc +++ b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc @@ -182,10 +182,6 @@ for those that were created by an SLM policy with a name starting with `policy-a 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. -`state`:: -(Optional, string) -Filter snapshots by state. Valid values are `SUCCESS`, `IN_PROGRESS`, `FAILED`, `PARTIAL`, or `INCOMPATIBLE`. - NOTE: The `after` parameter and `next` field allow for iterating through snapshots with some consistency guarantees regarding concurrent creation or deletion of snapshots. It is guaranteed that any snapshot that exists at the beginning of the iteration and is not concurrently deleted will be seen during the iteration. Snapshots concurrently created may be seen during an iteration. From 91ae28b6d21a0c7182828d621c061e7ce75a6eca Mon Sep 17 00:00:00 2001 From: Elena Stoeva Date: Fri, 28 Feb 2025 17:38:44 +0000 Subject: [PATCH 4/6] Resolve bwc test errors --- .../main/java/org/elasticsearch/TransportVersions.java | 1 + .../admin/cluster/snapshots/get/GetSnapshotsRequest.java | 9 +++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index 696fe6efe610c..df9b7e4403e72 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -206,6 +206,7 @@ static TransportVersion def(int id) { public static final TransportVersion ESQL_SERIALIZE_SOURCE_FUNCTIONS_WARNINGS = def(9_016_0_00); public static final TransportVersion ESQL_DRIVER_NODE_DESCRIPTION = def(9_017_0_00); public static final TransportVersion MULTI_PROJECT = def(9_018_0_00); + public static final TransportVersion STATE_PARAM_GET_SNAPSHOT = def(9_019_0_00); /* * STOP! READ THIS FIRST! No, really, 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 41d4a1bc7fb9b..ede0596ccb775 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 @@ -40,6 +40,7 @@ public class GetSnapshotsRequest extends MasterNodeRequest public static final boolean DEFAULT_VERBOSE_MODE = true; private static final TransportVersion INDICES_FLAG_VERSION = TransportVersions.V_8_3_0; + private static final TransportVersion STATE_FLAG_VERSION = TransportVersions.STATE_PARAM_GET_SNAPSHOT; public static final int NO_LIMIT = -1; @@ -121,7 +122,9 @@ public GetSnapshotsRequest(StreamInput in) throws IOException { if (in.getTransportVersion().onOrAfter(INDICES_FLAG_VERSION)) { includeIndexNames = in.readBoolean(); } - state = in.readOptionalString(); + if (in.getTransportVersion().onOrAfter(STATE_FLAG_VERSION)) { + state = in.readOptionalString(); + } } @Override @@ -141,7 +144,9 @@ public void writeTo(StreamOutput out) throws IOException { if (out.getTransportVersion().onOrAfter(INDICES_FLAG_VERSION)) { out.writeBoolean(includeIndexNames); } - out.writeOptionalString(state); + if (out.getTransportVersion().onOrAfter(STATE_FLAG_VERSION)) { + out.writeOptionalString(state); + } } @Override From d9f8faf0427f9046e6a959bd82c44cc8b3d88158 Mon Sep 17 00:00:00 2001 From: Elena Stoeva Date: Wed, 5 Mar 2025 13:55:35 +0000 Subject: [PATCH 5/6] Address feedback --- .../rest-api-spec/api/snapshot.get.json | 4 +- .../snapshots/GetSnapshotsIT.java | 69 +++++++++---------- .../snapshots/get/GetSnapshotsRequest.java | 17 ++--- .../get/GetSnapshotsRequestBuilder.java | 5 +- .../get/TransportGetSnapshotsAction.java | 30 +++----- .../admin/cluster/RestGetSnapshotsAction.java | 12 +++- .../snapshots/SnapshotState.java | 17 +++++ .../get/GetSnapshotsRequestTests.java | 6 -- 8 files changed, 84 insertions(+), 76 deletions(-) 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 8362686e87f6a..f40042c1b0dac 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 @@ -87,8 +87,8 @@ "description":"Whether to show verbose snapshot info or only show the basic info found in the repository index blob" }, "state": { - "type": "string", - "description": "Filter snapshots by state. Valid values are 'SUCCESS', 'IN_PROGRESS', 'FAILED', 'PARTIAL', or 'INCOMPATIBLE'." + "type": "list", + "description": "Filter snapshots by a comma-separated list of states. Valid state values are 'SUCCESS', 'IN_PROGRESS', 'FAILED', 'PARTIAL', or 'INCOMPATIBLE'." } } } diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java index 5233ac3f5cf8e..1d569db1c5a29 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java @@ -52,14 +52,7 @@ import java.nio.file.Files; import java.nio.file.Path; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.Set; +import java.util.*; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -638,51 +631,49 @@ public void testFilterByState() throws Exception { final String repoName = "test-repo"; final Path repoPath = randomRepoPath(); createRepository(repoName, "mock", repoPath); - maybeInitWithOldSnapshotVersion(repoName, repoPath); // Create a successful snapshot String successSnapshot = "snapshot-success"; createFullSnapshot(repoName, successSnapshot); - // Create a snapshot in progress - String inProgressSnapshot = "snapshot-in-progress"; - blockAllDataNodes(repoName); - startFullSnapshot(repoName, inProgressSnapshot); - awaitNumberOfSnapshotsInProgress(1); - awaitClusterState( - state -> SnapshotsInProgress.get(state) - .asStream() - .flatMap(s -> s.shards().entrySet().stream()) - .allMatch( - e -> e.getKey().getIndexName().equals("test-index-1") == false - || e.getValue().state() == SnapshotsInProgress.ShardState.SUCCESS - ) - ); - - // Fetch all snapshots - GetSnapshotsResponse responseAll = clusterAdmin().prepareGetSnapshots(TEST_REQUEST_TIMEOUT, repoName).get(); - assertThat(responseAll.getSnapshots(), hasSize(2)); - // Fetch snapshots with state=SUCCESS GetSnapshotsResponse responseSuccess = clusterAdmin().prepareGetSnapshots(TEST_REQUEST_TIMEOUT, repoName) - .setState(String.valueOf(SnapshotState.SUCCESS)) + .setState(EnumSet.of(SnapshotState.SUCCESS)) .get(); assertThat(responseSuccess.getSnapshots(), hasSize(1)); assertThat(responseSuccess.getSnapshots().get(0).state(), is(SnapshotState.SUCCESS)); + // Create a snapshot in progress + String inProgressSnapshot = "snapshot-in-progress"; + blockAllDataNodes(repoName); + startFullSnapshot(repoName, inProgressSnapshot); + awaitNumberOfSnapshotsInProgress(1); + // Fetch snapshots with state=IN_PROGRESS GetSnapshotsResponse responseInProgress = clusterAdmin().prepareGetSnapshots(TEST_REQUEST_TIMEOUT, repoName) - .setState(String.valueOf(SnapshotState.IN_PROGRESS)) + .setState(EnumSet.of(SnapshotState.IN_PROGRESS)) .get(); assertThat(responseInProgress.getSnapshots(), hasSize(1)); assertThat(responseInProgress.getSnapshots().get(0).state(), is(SnapshotState.IN_PROGRESS)); - // Fetch snapshots with invalid state + // Fetch snapshots with multiple states (SUCCESS, IN_PROGRESS) + GetSnapshotsResponse responseMultipleStates = clusterAdmin().prepareGetSnapshots(TEST_REQUEST_TIMEOUT, repoName) + .setState(EnumSet.of(SnapshotState.SUCCESS, SnapshotState.IN_PROGRESS)) + .get(); + assertThat(responseMultipleStates.getSnapshots(), hasSize(2)); + assertTrue(responseMultipleStates.getSnapshots().stream().map(SnapshotInfo::state).toList().contains(SnapshotState.SUCCESS)); + assertTrue(responseMultipleStates.getSnapshots().stream().map(SnapshotInfo::state).toList().contains(SnapshotState.IN_PROGRESS)); + + // Fetch all snapshots (without state) + GetSnapshotsResponse responseAll = clusterAdmin().prepareGetSnapshots(TEST_REQUEST_TIMEOUT, repoName).get(); + assertThat(responseAll.getSnapshots(), hasSize(2)); + + // Fetch snapshots with an invalid state IllegalArgumentException e = expectThrows( IllegalArgumentException.class, - () -> clusterAdmin().prepareGetSnapshots(TEST_REQUEST_TIMEOUT, repoName).setState("INVALID_STATE").get() + () -> clusterAdmin().prepareGetSnapshots(TEST_REQUEST_TIMEOUT, repoName).setState(EnumSet.of(SnapshotState.of("FOO"))).get() ); - assertThat(e.getMessage(), containsString("state must be SUCCESS, IN_PROGRESS, FAILED, PARTIAL, or INCOMPATIBLE")); + assertThat(e.getMessage(), containsString("Unknown state name [FOO]")); } // Create a snapshot that is guaranteed to have a unique start time and duration for tests around ordering by either. @@ -964,9 +955,15 @@ public void testAllFeatures() { // INDICES and by SHARDS. The actual sorting behaviour for these cases is tested elsewhere, here we're just checking that sorting // interacts correctly with the other parameters to the API. + final EnumSet state = EnumSet.of(randomFrom(SnapshotState.values())); + // Note: The selected state may not match any existing snapshots. + // The actual filtering behaviour for such cases is tested in the dedicated test. + // Here we're just checking that state interacts correctly with the other parameters to the API. + // compute the ordered sequence of snapshots which match the repository/snapshot name filters and SLM policy filter final var selectedSnapshots = snapshotInfos.stream() .filter(snapshotInfoPredicate) + .filter(s -> state.contains(s.state())) .sorted(sortKey.getSnapshotInfoComparator(order)) .toList(); @@ -975,7 +972,8 @@ public void testAllFeatures() { ) // apply sorting params .sort(sortKey) - .order(order); + .order(order) + .state(state); // sometimes use ?from_sort_value to skip some items; note that snapshots skipped in this way are subtracted from // GetSnapshotsResponse.totalCount whereas snapshots skipped by ?after and ?offset are not @@ -1062,7 +1060,8 @@ public void testAllFeatures() { .sort(sortKey) .order(order) .size(nextSize) - .after(SnapshotSortKey.decodeAfterQueryParam(nextRequestAfter)); + .after(SnapshotSortKey.decodeAfterQueryParam(nextRequestAfter)) + .state(state); final GetSnapshotsResponse nextResponse = safeAwait(l -> client().execute(TransportGetSnapshotsAction.TYPE, nextRequest, l)); assertEquals( 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 ede0596ccb775..bb68366081be2 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 @@ -26,6 +26,7 @@ import java.io.IOException; import java.util.Arrays; +import java.util.EnumSet; import java.util.Map; import static org.elasticsearch.action.ValidateActions.addValidationError; @@ -79,7 +80,7 @@ public class GetSnapshotsRequest extends MasterNodeRequest private boolean includeIndexNames = true; - private String state; + private EnumSet state = EnumSet.noneOf(SnapshotState.class); public GetSnapshotsRequest(TimeValue masterNodeTimeout) { super(masterNodeTimeout); @@ -123,7 +124,7 @@ public GetSnapshotsRequest(StreamInput in) throws IOException { includeIndexNames = in.readBoolean(); } if (in.getTransportVersion().onOrAfter(STATE_FLAG_VERSION)) { - state = in.readOptionalString(); + state = in.readEnumSet(SnapshotState.class); } } @@ -145,7 +146,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeBoolean(includeIndexNames); } if (out.getTransportVersion().onOrAfter(STATE_FLAG_VERSION)) { - out.writeOptionalString(state); + out.writeEnumSet(state); } } @@ -187,12 +188,6 @@ public ActionRequestValidationException validate() { } else if (after != null && fromSortValue != null) { validationException = addValidationError("can't use after and from_sort_value simultaneously", validationException); } - if (state != null && Arrays.stream(SnapshotState.values()).noneMatch(s -> s.name().equalsIgnoreCase(state))) { - validationException = addValidationError( - "state must be SUCCESS, IN_PROGRESS, FAILED, PARTIAL, or INCOMPATIBLE", - validationException - ); - } return validationException; } @@ -358,11 +353,11 @@ public boolean verbose() { return verbose; } - public String state() { + public EnumSet state() { return state; } - public GetSnapshotsRequest state(String state) { + public GetSnapshotsRequest state(EnumSet state) { this.state = state; return this; } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestBuilder.java index 96ae685269f53..24dc15f5788cc 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestBuilder.java @@ -15,6 +15,9 @@ import org.elasticsearch.core.Nullable; import org.elasticsearch.core.TimeValue; import org.elasticsearch.search.sort.SortOrder; +import org.elasticsearch.snapshots.SnapshotState; + +import java.util.EnumSet; /** * Get snapshots request builder @@ -150,7 +153,7 @@ public GetSnapshotsRequestBuilder setIncludeIndexNames(boolean indices) { } - public GetSnapshotsRequestBuilder setState(String state) { + public GetSnapshotsRequestBuilder setState(EnumSet state) { request.state(state); return this; } 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 034e8254321b8..a6fb425a56225 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 @@ -41,25 +41,14 @@ import org.elasticsearch.repositories.RepositoryMissingException; import org.elasticsearch.repositories.ResolvedRepositories; import org.elasticsearch.search.sort.SortOrder; -import org.elasticsearch.snapshots.Snapshot; -import org.elasticsearch.snapshots.SnapshotId; -import org.elasticsearch.snapshots.SnapshotInfo; -import org.elasticsearch.snapshots.SnapshotMissingException; -import org.elasticsearch.snapshots.SnapshotsService; +import org.elasticsearch.snapshots.*; import org.elasticsearch.tasks.CancellableTask; import org.elasticsearch.tasks.Task; import org.elasticsearch.tasks.TaskCancelledException; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.BiPredicate; import java.util.function.BooleanSupplier; @@ -182,7 +171,7 @@ private class GetSnapshotsOperation { private final SnapshotNamePredicate snapshotNamePredicate; private final SnapshotPredicates fromSortValuePredicates; private final Predicate slmPolicyPredicate; - private final String state; + private final EnumSet state; // snapshot ordering/pagination private final SnapshotSortKey sortBy; @@ -227,7 +216,7 @@ private class GetSnapshotsOperation { SnapshotsInProgress snapshotsInProgress, boolean verbose, boolean indices, - String state + EnumSet state ) { this.cancellableTask = cancellableTask; this.repositories = repositories; @@ -562,11 +551,16 @@ private boolean matchesPredicates(SnapshotId snapshotId, RepositoryData reposito return false; } + final var details = repositoryData.getSnapshotDetails(snapshotId); + + if (!state.isEmpty() && !state.contains(details.getSnapshotState())) { + return false; + } + if (slmPolicyPredicate == SlmPolicyPredicate.MATCH_ALL_POLICIES) { return true; } - final var details = repositoryData.getSnapshotDetails(snapshotId); return details == null || details.getSlmPolicy() == null || slmPolicyPredicate.test(details.getSlmPolicy()); } @@ -575,10 +569,6 @@ private boolean matchesPredicates(SnapshotInfo snapshotInfo) { return false; } - if (state != null) { - return state.equalsIgnoreCase(snapshotInfo.state().toString()); - } - if (slmPolicyPredicate == SlmPolicyPredicate.MATCH_ALL_POLICIES) { return true; } diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestGetSnapshotsAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestGetSnapshotsAction.java index 014bb09159d4f..2589d28f34078 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestGetSnapshotsAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestGetSnapshotsAction.java @@ -20,8 +20,11 @@ import org.elasticsearch.rest.action.RestCancellableNodeClient; import org.elasticsearch.rest.action.RestRefCountedChunkedToXContentListener; import org.elasticsearch.search.sort.SortOrder; +import org.elasticsearch.snapshots.SnapshotState; import java.io.IOException; +import java.util.Arrays; +import java.util.EnumSet; import java.util.List; import java.util.Set; @@ -82,7 +85,14 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC final SortOrder order = SortOrder.fromString(request.param("order", getSnapshotsRequest.order().toString())); getSnapshotsRequest.order(order); getSnapshotsRequest.includeIndexNames(request.paramAsBoolean(INDEX_NAMES_XCONTENT_PARAM, getSnapshotsRequest.includeIndexNames())); - getSnapshotsRequest.state(request.param("state")); + + final String stateString = request.param("state"); + if (stateString == null || stateString.isEmpty()) { + getSnapshotsRequest.state(EnumSet.noneOf(SnapshotState.class)); + } else { + getSnapshotsRequest.state(EnumSet.copyOf(Arrays.stream(stateString.split(",")).map(SnapshotState::of).toList())); + } + return channel -> new RestCancellableNodeClient(client, request.getHttpChannel()).admin() .cluster() .getSnapshots(getSnapshotsRequest, new RestRefCountedChunkedToXContentListener<>(channel)); diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotState.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotState.java index dcf90327e1098..df593d18f453b 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotState.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotState.java @@ -89,4 +89,21 @@ public static SnapshotState fromValue(byte value) { default -> throw new IllegalArgumentException("No snapshot state for value [" + value + "]"); }; } + + /** + * Generate snapshot state from a string (case-insensitive) + * + * @param name the state name + * @return state + */ + public static SnapshotState of(String name) { + return switch (name.toUpperCase()) { + case "IN_PROGRESS" -> IN_PROGRESS; + case "SUCCESS" -> SUCCESS; + case "FAILED" -> FAILED; + case "PARTIAL" -> PARTIAL; + case "INCOMPATIBLE" -> INCOMPATIBLE; + default -> throw new IllegalArgumentException("Unknown state name [" + name + "]"); + }; + } } diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestTests.java index 879b39d45cc81..b2b1396934992 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestTests.java @@ -86,12 +86,6 @@ public void testValidateParameters() { final ActionRequestValidationException e = request.validate(); assertThat(e.getMessage(), containsString("can't use slm policy filter with verbose=false")); } - { - final GetSnapshotsRequest request = new GetSnapshotsRequest(TEST_REQUEST_TIMEOUT, "repo", "snapshot").state("foo") - .verbose(false); - final ActionRequestValidationException e = request.validate(); - assertThat(e.getMessage(), containsString("state must be SUCCESS, IN_PROGRESS, FAILED, PARTIAL, or INCOMPATIBLE")); - } } public void testGetDescription() { From c2b19d69455ee493512c66cef7ea7c5dc9901f83 Mon Sep 17 00:00:00 2001 From: Elena Stoeva Date: Thu, 6 Mar 2025 14:59:45 +0000 Subject: [PATCH 6/6] Fix style errors --- .../elasticsearch/snapshots/GetSnapshotsIT.java | 10 +++++++++- .../get/TransportGetSnapshotsAction.java | 17 +++++++++++++++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java index 1d569db1c5a29..fcc9c055450ba 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java @@ -52,7 +52,15 @@ import java.nio.file.Files; import java.nio.file.Path; -import java.util.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.EnumSet; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; import java.util.function.Predicate; import java.util.stream.Collectors; 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 a6fb425a56225..b87e484cde9f5 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 @@ -41,14 +41,27 @@ import org.elasticsearch.repositories.RepositoryMissingException; import org.elasticsearch.repositories.ResolvedRepositories; import org.elasticsearch.search.sort.SortOrder; -import org.elasticsearch.snapshots.*; +import org.elasticsearch.snapshots.Snapshot; +import org.elasticsearch.snapshots.SnapshotId; +import org.elasticsearch.snapshots.SnapshotInfo; +import org.elasticsearch.snapshots.SnapshotMissingException; +import org.elasticsearch.snapshots.SnapshotState; +import org.elasticsearch.snapshots.SnapshotsService; import org.elasticsearch.tasks.CancellableTask; import org.elasticsearch.tasks.Task; import org.elasticsearch.tasks.TaskCancelledException; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; -import java.util.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.EnumSet; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.BiPredicate; import java.util.function.BooleanSupplier;