From db29bec5a2121e3c782009004147a45d8d12f4e7 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 9 Aug 2021 16:23:43 +0200 Subject: [PATCH] Implement Numeric Offset Parameter in Get Snapshots API (#76233) Add numeric offset parameter to this API. Relates #74350 --- .../apis/get-snapshot-api.asciidoc | 59 ++++++++++++++++++- .../http/snapshots/RestGetSnapshotsIT.java | 30 ++++++++-- .../snapshots/GetSnapshotsIT.java | 21 +++++-- .../snapshots/get/GetSnapshotsRequest.java | 29 +++++++++ .../get/GetSnapshotsRequestBuilder.java | 5 ++ .../get/TransportGetSnapshotsAction.java | 24 ++++---- .../admin/cluster/RestGetSnapshotsAction.java | 3 + .../get/GetSnapshotsRequestTests.java | 12 ++++ 8 files changed, 158 insertions(+), 25 deletions(-) diff --git a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc index 89861e20caea9..60cf9b34d53c9 100644 --- a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc +++ b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc @@ -134,12 +134,17 @@ Sort order. Valid values are `asc` for ascending and `desc` for descending order (Optional, string) Offset identifier to start pagination from as returned by the `next` field in the response body. +`offset`:: +(Optional, integer) +Numeric offset to start pagination from based on the snapshots matching this request. Using a non-zero value for this parameter is mutually +exclusive with using the `after` parameter. Defaults to `0`. + 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 not concurrently deleted will be seen during the iteration. Snapshots concurrently created may be seen during an iteration. -NOTE: The pagination parameters `size`, `order`, `after` and `sort` are not supported when using `verbose=false` and the sort order for -requests with `verbose=false` is undefined. +NOTE: The pagination parameters `size`, `order`, `after`, `offset` and `sort` are not supported when using `verbose=false` and the sort +order for requests with `verbose=false` is undefined. [role="child_attributes"] [[get-snapshot-api-response-body]] @@ -435,6 +440,56 @@ GET /_snapshot/my_repository/snapshot*?size=2&sort=name&after=c25hcHNob3RfMixteV The API returns the following response: +[source,console-result] +---- +{ + "snapshots": [ + { + "snapshot": "snapshot_3", + "uuid": "dRctdKb54xw67gvLCxSket", + "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 + } + } + ], + "total": 3, + "remaining": 0 +} +---- +// TESTRESPONSE[s/"uuid": "dRctdKb54xw67gvLCxSket"/"uuid": $body.snapshots.0.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_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/] + +Alternatively, the same result could be retrieved by using an offset value of `2` to skip the two snapshot already seen. + +[source,console] +---- +GET /_snapshot/my_repository/snapshot*?size=2&sort=name&offset=2 +---- + +The API returns the following response: + [source,console-result] ---- { diff --git a/qa/smoke-test-http/src/test/java/org/elasticsearch/http/snapshots/RestGetSnapshotsIT.java b/qa/smoke-test-http/src/test/java/org/elasticsearch/http/snapshots/RestGetSnapshotsIT.java index d3949e6a98e13..70da8e782b887 100644 --- a/qa/smoke-test-http/src/test/java/org/elasticsearch/http/snapshots/RestGetSnapshotsIT.java +++ b/qa/smoke-test-http/src/test/java/org/elasticsearch/http/snapshots/RestGetSnapshotsIT.java @@ -191,10 +191,14 @@ private static void assertStablePagination(String repoName, for (int i = 1; i < allSnapshotNames.size() - j; i++) { final GetSnapshotsResponse getSnapshotsResponse = sortedWithLimit(repoName, sort, GetSnapshotsRequest.After.from(after, sort).asQueryParam(), i, order); + final GetSnapshotsResponse getSnapshotsResponseNumeric = sortedWithLimit(repoName, sort, j + 1, i, order); final List subsetSorted = getSnapshotsResponse.getSnapshots(); + assertEquals(subsetSorted, getSnapshotsResponseNumeric.getSnapshots()); assertEquals(subsetSorted, allSorted.subList(j + 1, j + i + 1)); assertEquals(allSnapshotNames.size(), getSnapshotsResponse.totalCount()); assertEquals(allSnapshotNames.size() - (j + i + 1), getSnapshotsResponse.remaining()); + assertEquals(getSnapshotsResponseNumeric.totalCount(), getSnapshotsResponse.totalCount()); + assertEquals(getSnapshotsResponseNumeric.remaining(), getSnapshotsResponse.remaining()); } } } @@ -232,10 +236,10 @@ private static GetSnapshotsResponse readSnapshotInfos(Response response) throws } private static GetSnapshotsResponse sortedWithLimit(String repoName, - GetSnapshotsRequest.SortBy sortBy, - String after, - int size, - SortOrder order) throws IOException { + GetSnapshotsRequest.SortBy sortBy, + String after, + int size, + SortOrder order) throws IOException { final Request request = baseGetSnapshotsRequest(repoName); request.addParameter("sort", sortBy.toString()); if (size != GetSnapshotsRequest.NO_LIMIT || randomBoolean()) { @@ -250,4 +254,22 @@ private static GetSnapshotsResponse sortedWithLimit(String repoName, final Response response = getRestClient().performRequest(request); return readSnapshotInfos(response); } + + private static GetSnapshotsResponse sortedWithLimit(String repoName, + GetSnapshotsRequest.SortBy sortBy, + int offset, + int size, + SortOrder order) throws IOException { + final Request request = baseGetSnapshotsRequest(repoName); + request.addParameter("sort", sortBy.toString()); + if (size != GetSnapshotsRequest.NO_LIMIT || randomBoolean()) { + request.addParameter("size", String.valueOf(size)); + } + request.addParameter("offset", String.valueOf(offset)); + if (order == SortOrder.DESC || randomBoolean()) { + request.addParameter("order", order.toString()); + } + final Response response = getRestClient().performRequest(request); + return readSnapshotInfos(response); + } } diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java index 0ee1be41b7849..06a00a6c7162c 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java @@ -197,11 +197,15 @@ private static void assertStablePagination(String repoName, Collection a i, order ); + final GetSnapshotsResponse getSnapshotsResponseNumeric = sortedWithLimit(repoName, sort, j + 1, i, order); final List subsetSorted = getSnapshotsResponse.getSnapshots(); + assertEquals(subsetSorted, getSnapshotsResponseNumeric.getSnapshots()); assertEquals(subsetSorted, allSorted.subList(j + 1, j + i + 1)); assertEquals(allSnapshotNames.size(), getSnapshotsResponse.totalCount()); assertEquals(allSnapshotNames.size() - (j + i + 1), getSnapshotsResponse.remaining()); assertEquals(subsetSorted, allSorted.subList(j + 1, j + i + 1)); + assertEquals(getSnapshotsResponseNumeric.totalCount(), getSnapshotsResponse.totalCount()); + assertEquals(getSnapshotsResponseNumeric.remaining(), getSnapshotsResponse.remaining()); } } } @@ -230,12 +234,17 @@ private static GetSnapshotsResponse sortedWithLimit( int size, SortOrder order ) { - final GetSnapshotsResponse response = baseGetSnapshotsRequest(repoName).setAfter(after) - .setSort(sortBy) - .setSize(size) - .setOrder(order) - .get(); - return response; + return baseGetSnapshotsRequest(repoName).setAfter(after).setSort(sortBy).setSize(size).setOrder(order).get(); + } + + private static GetSnapshotsResponse sortedWithLimit( + String repoName, + GetSnapshotsRequest.SortBy sortBy, + int offset, + int size, + SortOrder order + ) { + return baseGetSnapshotsRequest(repoName).setOffset(offset).setSort(sortBy).setSize(size).setOrder(order).get(); } private static GetSnapshotsRequestBuilder baseGetSnapshotsRequest(String repoName) { 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 9f7604784c1e9..801c24ed9daa6 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 @@ -52,6 +52,11 @@ public class GetSnapshotsRequest extends MasterNodeRequest */ private int size = NO_LIMIT; + /** + * Numeric offset at which to start fetching snapshots. Mutually exclusive with {@link After} if not equal to {@code 0}. + */ + private int offset = 0; + @Nullable private After after; @@ -115,6 +120,9 @@ public GetSnapshotsRequest(StreamInput in) throws IOException { sort = in.readEnum(SortBy.class); size = in.readVInt(); order = SortOrder.readFromStream(in); + if (in.getVersion().onOrAfter(NUMERIC_PAGINATION_VERSION)) { + offset = in.readVInt(); + } } } @@ -141,6 +149,13 @@ public void writeTo(StreamOutput out) throws IOException { out.writeEnum(sort); out.writeVInt(size); order.writeTo(out); + if (out.getVersion().onOrAfter(NUMERIC_PAGINATION_VERSION)) { + out.writeVInt(offset); + } else if (offset != 0) { + throw new IllegalArgumentException( + "can't use numeric offset in get snapshots request with node version [" + out.getVersion() + "]" + ); + } } else if (sort != SortBy.START_TIME || size != NO_LIMIT || after != null || order != SortOrder.ASC) { throw new IllegalArgumentException("can't use paginated get snapshots request with node version [" + out.getVersion() + "]"); } @@ -162,12 +177,17 @@ public ActionRequestValidationException validate() { if (size > 0) { validationException = addValidationError("can't use size limit with verbose=false", validationException); } + if (offset > 0) { + validationException = addValidationError("can't use offset with verbose=false", validationException); + } if (after != null) { validationException = addValidationError("can't use after with verbose=false", validationException); } if (order != SortOrder.ASC) { validationException = addValidationError("can't use non-default sort order with verbose=false", validationException); } + } else if (after != null && offset > 0) { + validationException = addValidationError("can't use after and offset simultaneously", validationException); } return validationException; } @@ -301,6 +321,15 @@ public int size() { return size; } + public int offset() { + return offset; + } + + public GetSnapshotsRequest offset(int offset) { + this.offset = offset; + return this; + } + public SortOrder order() { return order; } 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 ebe0df0227bf5..6e57b60c4e2c2 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 @@ -141,6 +141,11 @@ public GetSnapshotsRequestBuilder setSize(int size) { return this; } + public GetSnapshotsRequestBuilder setOffset(int offset) { + request.offset(offset); + return this; + } + public GetSnapshotsRequestBuilder setOrder(SortOrder order) { request.order(order); 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 86a48b3437076..c589ce1d0e984 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 @@ -123,6 +123,7 @@ protected void masterOperation( (CancellableTask) task, request.sort(), request.after(), + request.offset(), request.size(), request.order(), listener @@ -139,6 +140,7 @@ private void getMultipleReposSnapshotInfo( CancellableTask cancellableTask, GetSnapshotsRequest.SortBy sortBy, @Nullable GetSnapshotsRequest.After after, + int offset, int size, SortOrder order, ActionListener listener @@ -160,7 +162,7 @@ private void getMultipleReposSnapshotInfo( .map(Tuple::v1) .filter(Objects::nonNull) .collect(Collectors.toMap(Tuple::v1, Tuple::v2)); - final SnapshotsInRepo snInfos = sortSnapshots(allSnapshots, sortBy, after, size, order); + final SnapshotsInRepo snInfos = sortSnapshots(allSnapshots, sortBy, after, offset, size, order); final List snapshotInfos = snInfos.snapshotInfos; final int remaining = snInfos.remaining + responses.stream() .map(Tuple::v2) @@ -189,7 +191,6 @@ private void getMultipleReposSnapshotInfo( cancellableTask, sortBy, after, - size, order, groupedActionListener.delegateResponse((groupedListener, e) -> { if (isMultiRepoRequest && e instanceof ElasticsearchException) { @@ -211,7 +212,6 @@ private void getSingleRepoSnapshotInfo( CancellableTask task, GetSnapshotsRequest.SortBy sortBy, @Nullable final GetSnapshotsRequest.After after, - int size, SortOrder order, ActionListener listener ) { @@ -243,7 +243,6 @@ private void getSingleRepoSnapshotInfo( task, sortBy, after, - size, order, listener ), @@ -283,7 +282,6 @@ private void loadSnapshotInfos( CancellableTask task, GetSnapshotsRequest.SortBy sortBy, @Nullable final GetSnapshotsRequest.After after, - int size, SortOrder order, ActionListener listener ) { @@ -333,7 +331,6 @@ private void loadSnapshotInfos( task, sortBy, after, - size, order, listener ); @@ -341,14 +338,15 @@ private void loadSnapshotInfos( final SnapshotsInRepo snapshotInfos; if (repositoryData != null) { // want non-current snapshots as well, which are found in the repository data - snapshotInfos = buildSimpleSnapshotInfos(toResolve, repo, repositoryData, currentSnapshots, sortBy, after, size, order); + snapshotInfos = buildSimpleSnapshotInfos(toResolve, repo, repositoryData, currentSnapshots, sortBy, after, order); } else { // only want current snapshots snapshotInfos = sortSnapshots( currentSnapshots.stream().map(SnapshotInfo::basic).collect(Collectors.toList()), sortBy, after, - size, + 0, + GetSnapshotsRequest.NO_LIMIT, order ); } @@ -371,7 +369,6 @@ private void snapshots( CancellableTask task, GetSnapshotsRequest.SortBy sortBy, @Nullable GetSnapshotsRequest.After after, - int size, SortOrder order, ActionListener listener ) { @@ -401,7 +398,7 @@ private void snapshots( final ActionListener allDoneListener = listener.delegateFailure((l, v) -> { final ArrayList snapshotList = new ArrayList<>(snapshotInfos); snapshotList.addAll(snapshotSet); - listener.onResponse(sortSnapshots(snapshotList, sortBy, after, size, order)); + listener.onResponse(sortSnapshots(snapshotList, sortBy, after, 0, GetSnapshotsRequest.NO_LIMIT, order)); }); if (snapshotIdsToIterate.isEmpty()) { allDoneListener.onResponse(null); @@ -451,7 +448,6 @@ private static SnapshotsInRepo buildSimpleSnapshotInfos( final List currentSnapshots, final GetSnapshotsRequest.SortBy sortBy, @Nullable final GetSnapshotsRequest.After after, - final int size, final SortOrder order ) { List snapshotInfos = new ArrayList<>(); @@ -481,7 +477,7 @@ private static SnapshotsInRepo buildSimpleSnapshotInfos( ) ); } - return sortSnapshots(snapshotInfos, sortBy, after, size, order); + return sortSnapshots(snapshotInfos, sortBy, after, 0, GetSnapshotsRequest.NO_LIMIT, order); } private static final Comparator BY_START_TIME = Comparator.comparingLong(SnapshotInfo::startTime) @@ -500,6 +496,7 @@ private static SnapshotsInRepo sortSnapshots( final List snapshotInfos, final GetSnapshotsRequest.SortBy sortBy, final @Nullable GetSnapshotsRequest.After after, + final int offset, final int size, final SortOrder order ) { @@ -524,6 +521,7 @@ private static SnapshotsInRepo sortSnapshots( Stream infos = snapshotInfos.stream(); if (after != null) { + assert offset == 0 : "can't combine after and offset but saw [" + after + "] and offset [" + offset + "]"; final Predicate isAfter; final String snapshotName = after.snapshotName(); final String repoName = after.repoName(); @@ -559,7 +557,7 @@ private static SnapshotsInRepo sortSnapshots( } infos = infos.filter(isAfter); } - infos = infos.sorted(order == SortOrder.DESC ? comparator.reversed() : comparator); + infos = infos.sorted(order == SortOrder.DESC ? comparator.reversed() : comparator).skip(offset); final List allSnapshots = infos.collect(Collectors.toList()); final List snapshots; if (size != GetSnapshotsRequest.NO_LIMIT) { 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 a4284711928fb..59a7958f3cb2a 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 @@ -59,10 +59,13 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC getSnapshotsRequest.sort(sort); final int size = request.paramAsInt("size", getSnapshotsRequest.size()); getSnapshotsRequest.size(size); + final int offset = request.paramAsInt("offset", getSnapshotsRequest.offset()); + getSnapshotsRequest.offset(offset); final String afterString = request.param("after"); if (afterString != null) { getSnapshotsRequest.after(GetSnapshotsRequest.After.fromQueryParam(afterString)); } + final SortOrder order = SortOrder.fromString(request.param("order", getSnapshotsRequest.order().toString())); getSnapshotsRequest.order(order); getSnapshotsRequest.masterNodeTimeout(request.paramAsTime("master_timeout", getSnapshotsRequest.masterNodeTimeout())); 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 03fe362f2ee39..9f6ee5b256bc9 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 @@ -32,6 +32,11 @@ public void testValidateParameters() { final ActionRequestValidationException e = request.validate(); assertThat(e.getMessage(), containsString("can't use size limit with verbose=false")); } + { + final GetSnapshotsRequest request = new GetSnapshotsRequest("repo", "snapshot").verbose(false).offset(randomIntBetween(1, 500)); + final ActionRequestValidationException e = request.validate(); + assertThat(e.getMessage(), containsString("can't use offset with verbose=false")); + } { final GetSnapshotsRequest request = new GetSnapshotsRequest("repo", "snapshot").verbose(false) .sort(GetSnapshotsRequest.SortBy.INDICES); @@ -49,5 +54,12 @@ public void testValidateParameters() { final ActionRequestValidationException e = request.validate(); assertThat(e.getMessage(), containsString("can't use after with verbose=false")); } + { + final GetSnapshotsRequest request = new GetSnapshotsRequest("repo", "snapshot").after( + new GetSnapshotsRequest.After("foo", "repo", "bar") + ).offset(randomIntBetween(1, 500)); + final ActionRequestValidationException e = request.validate(); + assertThat(e.getMessage(), containsString("can't use after and offset simultaneously")); + } } }