From d356a4b603ad985583ad34410e11630973043ddc Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Sat, 14 Aug 2021 18:29:14 +0200 Subject: [PATCH] Implement Numeric Pagination in Get Snapshots API (#76532) * Return Total Result Count and Remaining Count in Get Snapshots Response (#76150) Add total result count and remaining count to get snapshots response. * Implement Numeric Offset Parameter in Get Snapshots API (#76233) Add numeric offset parameter to this API. Relates #74350 --- .../apis/get-snapshot-api.asciidoc | 82 ++++++++++++++++-- .../http/snapshots/RestGetSnapshotsIT.java | 86 ++++++++++++------- .../snapshots/GetSnapshotsIT.java | 76 +++++++++------- .../snapshots/get/GetSnapshotsRequest.java | 31 +++++++ .../get/GetSnapshotsRequestBuilder.java | 5 ++ .../snapshots/get/GetSnapshotsResponse.java | 51 ++++++++++- .../get/TransportGetSnapshotsAction.java | 84 +++++++++--------- .../admin/cluster/RestGetSnapshotsAction.java | 3 + .../get/GetSnapshotsRequestTests.java | 12 +++ .../get/GetSnapshotsResponseTests.java | 2 +- .../xpack/slm/SnapshotRetentionTaskTests.java | 3 +- 11 files changed, 325 insertions(+), 110 deletions(-) diff --git a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc index 4ed67e3cc14d6..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]] @@ -283,6 +288,15 @@ The snapshot `state` can be one of the following values: If the request contained a size limit and there might be more results, a `next` field will be added to the response and can be used as the `after` query parameter to fetch additional results. +`total`:: +(integer) +The total number of snapshots that match the request when ignoring size limit or `after` query parameter. + +`remaining`:: +(integer) +The number of remaining snapshots that were not returned due to size limits and that can be fetched by additional requests using the `next` +field value. + [[get-snapshot-api-example]] ==== {api-examples-title} @@ -322,7 +336,9 @@ The API returns the following response: "successful": 0 } } - ] + ], + "total": 1, + "remaining": 0 } ---- // TESTRESPONSE[s/"uuid": "vdRctLCxSketdKb54xw67g"/"uuid": $body.snapshots.0.uuid/] @@ -392,10 +408,12 @@ The API returns the following response: "total": 0, "failed": 0, "successful": 0 - } + }, } ], - "next": "c25hcHNob3RfMixteV9yZXBvc2l0b3J5LHNuYXBzaG90XzI=" + "next": "c25hcHNob3RfMixteV9yZXBvc2l0b3J5LHNuYXBzaG90XzI=", + "total": 3, + "remaining": 1 } ---- // TESTRESPONSE[s/"uuid": "dKb54xw67gvdRctLCxSket"/"uuid": $body.snapshots.0.uuid/] @@ -449,7 +467,59 @@ The API returns the following response: "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] +---- +{ + "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/] 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 95496e3e537c6..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 @@ -20,7 +20,6 @@ import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.json.JsonXContent; -import org.elasticsearch.core.Tuple; import org.elasticsearch.search.sort.SortOrder; import org.elasticsearch.snapshots.AbstractSnapshotIntegTestCase; import org.elasticsearch.snapshots.SnapshotInfo; @@ -106,31 +105,34 @@ private void doTestPagination(String repoName, GetSnapshotsRequest.SortBy sort, SortOrder order) throws IOException { final List allSnapshotsSorted = allSnapshotsSorted(names, repoName, sort, order); - final Tuple> batch1 = sortedWithLimit(repoName, sort, null, 2, order); - assertEquals(allSnapshotsSorted.subList(0, 2), batch1.v2()); - final Tuple> batch2 = sortedWithLimit(repoName, sort, batch1.v1(), 2, order); - assertEquals(allSnapshotsSorted.subList(2, 4), batch2.v2()); - final int lastBatch = names.size() - batch1.v2().size() - batch2.v2().size(); - final Tuple> batch3 = sortedWithLimit(repoName, sort, batch2.v1(), lastBatch, order); - assertEquals(batch3.v2(), allSnapshotsSorted.subList(batch1.v2().size() + batch2.v2().size(), names.size())); - final Tuple> batch3NoLimit = sortedWithLimit( + final GetSnapshotsResponse batch1 = sortedWithLimit(repoName, sort, null, 2, order); + assertEquals(allSnapshotsSorted.subList(0, 2), batch1.getSnapshots()); + final GetSnapshotsResponse batch2 = sortedWithLimit(repoName, 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); + assertEquals( + batch3.getSnapshots(), + allSnapshotsSorted.subList(batch1.getSnapshots().size() + batch2.getSnapshots().size(), names.size()) + ); + final GetSnapshotsResponse batch3NoLimit = sortedWithLimit( repoName, sort, - batch2.v1(), + batch2.next(), GetSnapshotsRequest.NO_LIMIT, order ); - assertNull(batch3NoLimit.v1()); - assertEquals(batch3.v2(), batch3NoLimit.v2()); - final Tuple> batch3LargeLimit = sortedWithLimit( + assertNull(batch3NoLimit.next()); + assertEquals(batch3.getSnapshots(), batch3NoLimit.getSnapshots()); + final GetSnapshotsResponse batch3LargeLimit = sortedWithLimit( repoName, sort, - batch2.v1(), + batch2.next(), lastBatch + randomIntBetween(1, 100), order ); - assertEquals(batch3.v2(), batch3LargeLimit.v2()); - assertNull(batch3LargeLimit.v1()); + assertEquals(batch3.getSnapshots(), batch3LargeLimit.getSnapshots()); + assertNull(batch3LargeLimit.next()); } public void testSortAndPaginateWithInProgress() throws Exception { @@ -180,16 +182,23 @@ private static void assertStablePagination(String repoName, final List allSorted = allSnapshotsSorted(allSnapshotNames, repoName, sort, order); for (int i = 1; i <= allSnapshotNames.size(); i++) { - final List subsetSorted = sortedWithLimit(repoName, sort, null, i, order).v2(); + final List subsetSorted = sortedWithLimit(repoName, sort, null, i, order).getSnapshots(); assertEquals(subsetSorted, allSorted.subList(0, i)); } for (int j = 0; j < allSnapshotNames.size(); j++) { final SnapshotInfo after = allSorted.get(j); for (int i = 1; i < allSnapshotNames.size() - j; i++) { - final List subsetSorted = sortedWithLimit( - repoName, sort, GetSnapshotsRequest.After.from(after, sort).asQueryParam(), i, order).v2(); + 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()); } } } @@ -203,9 +212,11 @@ private static List allSnapshotsSorted(Collection allSnaps if (order == SortOrder.DESC || randomBoolean()) { request.addParameter("order", order.toString()); } - final Response response = getRestClient().performRequest(request); - final List snapshotInfos = readSnapshotInfos(response).v2(); + final GetSnapshotsResponse getSnapshotsResponse = readSnapshotInfos(getRestClient().performRequest(request)); + final List snapshotInfos = getSnapshotsResponse.getSnapshots(); assertEquals(snapshotInfos.size(), allSnapshotNames.size()); + assertEquals(getSnapshotsResponse.totalCount(), allSnapshotNames.size()); + assertEquals(0, getSnapshotsResponse.remaining()); for (SnapshotInfo snapshotInfo : snapshotInfos) { assertThat(snapshotInfo.snapshotId().getName(), is(in(allSnapshotNames))); } @@ -216,20 +227,19 @@ private static Request baseGetSnapshotsRequest(String repoName) { return new Request(HttpGet.METHOD_NAME, "/_snapshot/" + repoName + "/*"); } - private static Tuple> readSnapshotInfos(Response response) throws IOException { + private static GetSnapshotsResponse readSnapshotInfos(Response response) throws IOException { try (InputStream input = response.getEntity().getContent(); XContentParser parser = JsonXContent.jsonXContent.createParser( NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, input)) { - final GetSnapshotsResponse getSnapshotsResponse = GetSnapshotsResponse.fromXContent(parser); - return Tuple.tuple(getSnapshotsResponse.next(), getSnapshotsResponse.getSnapshots()); + return GetSnapshotsResponse.fromXContent(parser); } } - private static Tuple> sortedWithLimit(String repoName, - GetSnapshotsRequest.SortBy sortBy, - String after, - int size, - SortOrder order) throws IOException { + private static GetSnapshotsResponse sortedWithLimit(String repoName, + 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()) { @@ -244,4 +254,22 @@ private static Tuple> 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 93c217e3cd6a0..06a00a6c7162c 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java @@ -15,7 +15,6 @@ import org.elasticsearch.action.admin.cluster.snapshots.get.GetSnapshotsRequestBuilder; import org.elasticsearch.action.admin.cluster.snapshots.get.GetSnapshotsResponse; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.core.Tuple; import org.elasticsearch.search.sort.SortOrder; import org.elasticsearch.threadpool.ThreadPool; @@ -97,31 +96,28 @@ 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 Tuple> batch1 = sortedWithLimit(repoName, sort, null, 2, order); - assertEquals(allSnapshotsSorted.subList(0, 2), batch1.v2()); - final Tuple> batch2 = sortedWithLimit(repoName, sort, batch1.v1(), 2, order); - assertEquals(allSnapshotsSorted.subList(2, 4), batch2.v2()); - final int lastBatch = names.size() - batch1.v2().size() - batch2.v2().size(); - final Tuple> batch3 = sortedWithLimit(repoName, sort, batch2.v1(), lastBatch, order); - assertEquals(batch3.v2(), allSnapshotsSorted.subList(batch1.v2().size() + batch2.v2().size(), names.size())); - final Tuple> batch3NoLimit = sortedWithLimit( - repoName, - sort, - batch2.v1(), - GetSnapshotsRequest.NO_LIMIT, - order + final GetSnapshotsResponse batch1 = sortedWithLimit(repoName, sort, null, 2, order); + assertEquals(allSnapshotsSorted.subList(0, 2), batch1.getSnapshots()); + final GetSnapshotsResponse batch2 = sortedWithLimit(repoName, 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); + assertEquals( + batch3.getSnapshots(), + allSnapshotsSorted.subList(batch1.getSnapshots().size() + batch2.getSnapshots().size(), names.size()) ); - assertNull(batch3NoLimit.v1()); - assertEquals(batch3.v2(), batch3NoLimit.v2()); - final Tuple> batch3LargeLimit = sortedWithLimit( + final GetSnapshotsResponse batch3NoLimit = sortedWithLimit(repoName, sort, batch2.next(), GetSnapshotsRequest.NO_LIMIT, order); + assertNull(batch3NoLimit.next()); + assertEquals(batch3.getSnapshots(), batch3NoLimit.getSnapshots()); + final GetSnapshotsResponse batch3LargeLimit = sortedWithLimit( repoName, sort, - batch2.v1(), + batch2.next(), lastBatch + randomIntBetween(1, 100), order ); - assertEquals(batch3.v2(), batch3LargeLimit.v2()); - assertNull(batch3LargeLimit.v1()); + assertEquals(batch3.getSnapshots(), batch3LargeLimit.getSnapshots()); + assertNull(batch3LargeLimit.next()); } public void testSortAndPaginateWithInProgress() throws Exception { @@ -187,21 +183,29 @@ private static void assertStablePagination(String repoName, Collection a final List allSorted = allSnapshotsSorted(allSnapshotNames, repoName, sort, order); for (int i = 1; i <= allSnapshotNames.size(); i++) { - final Tuple> subsetSorted = sortedWithLimit(repoName, sort, null, i, order); - assertEquals(allSorted.subList(0, i), subsetSorted.v2()); + final GetSnapshotsResponse subsetSorted = sortedWithLimit(repoName, sort, null, i, order); + assertEquals(allSorted.subList(0, i), subsetSorted.getSnapshots()); } for (int j = 0; j < allSnapshotNames.size(); j++) { final SnapshotInfo after = allSorted.get(j); for (int i = 1; i < allSnapshotNames.size() - j; i++) { - final List subsetSorted = sortedWithLimit( + final GetSnapshotsResponse getSnapshotsResponse = sortedWithLimit( repoName, sort, GetSnapshotsRequest.After.from(after, sort).asQueryParam(), i, order - ).v2(); + ); + 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()); } } } @@ -212,27 +216,35 @@ private static List allSnapshotsSorted( GetSnapshotsRequest.SortBy sortBy, SortOrder order ) { - final List snapshotInfos = sortedWithLimit(repoName, sortBy, null, GetSnapshotsRequest.NO_LIMIT, order).v2(); + final GetSnapshotsResponse getSnapshotsResponse = sortedWithLimit(repoName, sortBy, null, GetSnapshotsRequest.NO_LIMIT, order); + final List snapshotInfos = getSnapshotsResponse.getSnapshots(); assertEquals(snapshotInfos.size(), allSnapshotNames.size()); + assertEquals(getSnapshotsResponse.totalCount(), allSnapshotNames.size()); + assertEquals(0, getSnapshotsResponse.remaining()); for (SnapshotInfo snapshotInfo : snapshotInfos) { assertThat(snapshotInfo.snapshotId().getName(), is(in(allSnapshotNames))); } return snapshotInfos; } - private static Tuple> sortedWithLimit( + private static GetSnapshotsResponse sortedWithLimit( String repoName, GetSnapshotsRequest.SortBy sortBy, String after, int size, SortOrder order ) { - final GetSnapshotsResponse response = baseGetSnapshotsRequest(repoName).setAfter(after) - .setSort(sortBy) - .setSize(size) - .setOrder(order) - .get(); - return Tuple.tuple(response.next(), response.getSnapshots()); + 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 88ad1734b794f..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 @@ -43,6 +43,8 @@ public class GetSnapshotsRequest extends MasterNodeRequest public static final Version PAGINATED_GET_SNAPSHOTS_VERSION = Version.V_7_14_0; + public static final Version NUMERIC_PAGINATION_VERSION = Version.V_7_15_0; + public static final int NO_LIMIT = -1; /** @@ -50,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; @@ -113,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(); + } } } @@ -139,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() + "]"); } @@ -160,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; } @@ -299,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/GetSnapshotsResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponse.java index d022fdd1650e1..339455bd1ca2c 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponse.java @@ -34,11 +34,19 @@ */ public class GetSnapshotsResponse extends ActionResponse implements ToXContentObject { + private static final int UNKNOWN_COUNT = -1; + @SuppressWarnings("unchecked") private static final ConstructingObjectParser GET_SNAPSHOT_PARSER = new ConstructingObjectParser<>( GetSnapshotsResponse.class.getName(), true, - (args) -> new GetSnapshotsResponse((List) args[0], (Map) args[1], (String) args[2]) + (args) -> new GetSnapshotsResponse( + (List) args[0], + (Map) args[1], + (String) args[2], + args[3] == null ? UNKNOWN_COUNT : (int) args[3], + args[4] == null ? UNKNOWN_COUNT : (int) args[4] + ) ); static { @@ -53,6 +61,8 @@ public class GetSnapshotsResponse extends ActionResponse implements ToXContentOb new ParseField("failures") ); GET_SNAPSHOT_PARSER.declareStringOrNull(ConstructingObjectParser.optionalConstructorArg(), new ParseField("next")); + GET_SNAPSHOT_PARSER.declareIntOrNull(ConstructingObjectParser.optionalConstructorArg(), UNKNOWN_COUNT, new ParseField("total")); + GET_SNAPSHOT_PARSER.declareIntOrNull(ConstructingObjectParser.optionalConstructorArg(), UNKNOWN_COUNT, new ParseField("remaining")); } private final List snapshots; @@ -62,10 +72,22 @@ public class GetSnapshotsResponse extends ActionResponse implements ToXContentOb @Nullable private final String next; - public GetSnapshotsResponse(List snapshots, Map failures, @Nullable String next) { + private final int total; + + private final int remaining; + + public GetSnapshotsResponse( + List snapshots, + Map failures, + @Nullable String next, + final int total, + final int remaining + ) { this.snapshots = org.elasticsearch.core.List.copyOf(snapshots); this.failures = failures == null ? org.elasticsearch.core.Map.of() : org.elasticsearch.core.Map.copyOf(failures); this.next = next; + this.total = total; + this.remaining = remaining; } public GetSnapshotsResponse(StreamInput in) throws IOException { @@ -78,6 +100,13 @@ public GetSnapshotsResponse(StreamInput in) throws IOException { this.failures = Collections.emptyMap(); this.next = null; } + if (in.getVersion().onOrAfter(GetSnapshotsRequest.NUMERIC_PAGINATION_VERSION)) { + this.total = in.readVInt(); + this.remaining = in.readVInt(); + } else { + this.total = UNKNOWN_COUNT; + this.remaining = UNKNOWN_COUNT; + } } /** @@ -108,6 +137,14 @@ public boolean isFailed() { return failures.isEmpty() == false; } + public int totalCount() { + return total; + } + + public int remaining() { + return remaining; + } + @Override public void writeTo(StreamOutput out) throws IOException { out.writeList(snapshots); @@ -120,6 +157,10 @@ public void writeTo(StreamOutput out) throws IOException { throw failures.values().iterator().next(); } } + if (out.getVersion().onOrAfter(GetSnapshotsRequest.NUMERIC_PAGINATION_VERSION)) { + out.writeVInt(total); + out.writeVInt(remaining); + } } @Override @@ -145,6 +186,12 @@ public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params par if (next != null) { builder.field("next", next); } + if (total >= 0) { + builder.field("total", total); + } + if (remaining >= 0) { + builder.field("remaining", remaining); + } builder.endObject(); return builder; } 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 9b3e1c536f361..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,13 +140,14 @@ private void getMultipleReposSnapshotInfo( CancellableTask cancellableTask, GetSnapshotsRequest.SortBy sortBy, @Nullable GetSnapshotsRequest.After after, + int offset, int size, SortOrder order, ActionListener listener ) { // short-circuit if there are no repos, because we can not create GroupedActionListener of size 0 if (repos.isEmpty()) { - listener.onResponse(new GetSnapshotsResponse(Collections.emptyList(), Collections.emptyMap(), null)); + listener.onResponse(new GetSnapshotsResponse(Collections.emptyList(), Collections.emptyMap(), null, 0, 0)); return; } final GroupedActionListener, SnapshotsInRepo>> groupedActionListener = @@ -160,14 +162,21 @@ 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) + .filter(Objects::nonNull) + .mapToInt(s -> s.remaining) + .sum(); return new GetSnapshotsResponse( snapshotInfos, failures, - snInfos.hasMore || responses.stream().anyMatch(r -> r.v2() != null && r.v2().hasMore) + remaining > 0 ? GetSnapshotsRequest.After.from(snapshotInfos.get(snapshotInfos.size() - 1), sortBy).asQueryParam() - : null + : null, + responses.stream().map(Tuple::v2).filter(Objects::nonNull).mapToInt(s -> s.totalCount).sum(), + remaining ); }), repos.size()); @@ -182,7 +191,6 @@ private void getMultipleReposSnapshotInfo( cancellableTask, sortBy, after, - size, order, groupedActionListener.delegateResponse((groupedListener, e) -> { if (isMultiRepoRequest && e instanceof ElasticsearchException) { @@ -204,13 +212,12 @@ private void getSingleRepoSnapshotInfo( CancellableTask task, GetSnapshotsRequest.SortBy sortBy, @Nullable final GetSnapshotsRequest.After after, - int size, SortOrder order, ActionListener listener ) { final Map allSnapshotIds = new HashMap<>(); final List currentSnapshots = new ArrayList<>(); - for (SnapshotInfo snapshotInfo : sortedCurrentSnapshots(snapshotsInProgress, repo, sortBy, after, size, order).snapshotInfos) { + for (SnapshotInfo snapshotInfo : currentSnapshots(snapshotsInProgress, repo)) { Snapshot snapshot = snapshotInfo.snapshot(); allSnapshotIds.put(snapshot.getSnapshotId().getName(), snapshot); currentSnapshots.add(snapshotInfo); @@ -236,7 +243,6 @@ private void getSingleRepoSnapshotInfo( task, sortBy, after, - size, order, listener ), @@ -251,14 +257,7 @@ private void getSingleRepoSnapshotInfo( * @param repositoryName repository name * @return list of snapshots */ - private static SnapshotsInRepo sortedCurrentSnapshots( - SnapshotsInProgress snapshotsInProgress, - String repositoryName, - GetSnapshotsRequest.SortBy sortBy, - @Nullable final GetSnapshotsRequest.After after, - int size, - SortOrder order - ) { + private static List currentSnapshots(SnapshotsInProgress snapshotsInProgress, String repositoryName) { List snapshotList = new ArrayList<>(); List entries = SnapshotsService.currentSnapshots( snapshotsInProgress, @@ -268,7 +267,7 @@ private static SnapshotsInRepo sortedCurrentSnapshots( for (SnapshotsInProgress.Entry entry : entries) { snapshotList.add(new SnapshotInfo(entry)); } - return sortSnapshots(snapshotList, sortBy, after, size, order); + return snapshotList; } private void loadSnapshotInfos( @@ -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) @@ -497,11 +493,12 @@ private static SnapshotsInRepo buildSimpleSnapshotInfos( private static final Comparator BY_NAME = Comparator.comparing(sni -> sni.snapshotId().getName()); private static SnapshotsInRepo sortSnapshots( - List snapshotInfos, - GetSnapshotsRequest.SortBy sortBy, - @Nullable GetSnapshotsRequest.After after, - int size, - SortOrder order + final List snapshotInfos, + final GetSnapshotsRequest.SortBy sortBy, + final @Nullable GetSnapshotsRequest.After after, + final int offset, + final int size, + final SortOrder order ) { final Comparator comparator; switch (sortBy) { @@ -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,13 +557,18 @@ 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) { - infos = infos.limit(size + 1); + snapshots = Collections.unmodifiableList(allSnapshots.stream().limit(size + 1).collect(Collectors.toList())); + } else { + snapshots = allSnapshots; } - final List snapshots = Collections.unmodifiableList(infos.collect(Collectors.toList())); - boolean hasMore = size != GetSnapshotsRequest.NO_LIMIT && size < snapshots.size(); - return new SnapshotsInRepo(hasMore ? snapshots.subList(0, size) : snapshots, hasMore); + final List resultSet = size != GetSnapshotsRequest.NO_LIMIT && size < snapshots.size() + ? snapshots.subList(0, size) + : snapshots; + return new SnapshotsInRepo(resultSet, snapshotInfos.size(), allSnapshots.size() - resultSet.size()); } private static Predicate filterByLongOffset( @@ -595,13 +598,16 @@ private static int compareName(String name, String repoName, SnapshotInfo info) private static final class SnapshotsInRepo { - private final boolean hasMore; - private final List snapshotInfos; - SnapshotsInRepo(List snapshotInfos, boolean hasMore) { - this.hasMore = hasMore; + private final int totalCount; + + private final int remaining; + + SnapshotsInRepo(List snapshotInfos, int totalCount, int remaining) { this.snapshotInfos = snapshotInfos; + this.totalCount = totalCount; + this.remaining = remaining; } } } 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")); + } } } diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponseTests.java index af2de94e2c374..fd406cbe4ed43 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponseTests.java @@ -72,7 +72,7 @@ protected GetSnapshotsResponse createTestInstance() { ) ); } - return new GetSnapshotsResponse(snapshots, Collections.emptyMap(), null); + return new GetSnapshotsResponse(snapshots, Collections.emptyMap(), null, snapshots.size() + randomInt(), randomInt()); } @Override diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionTaskTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionTaskTests.java index 7b42d1742b4ee..f320438dd1d4d 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionTaskTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionTaskTests.java @@ -316,7 +316,8 @@ public void testErrStillRunsFailureHandlerWhenRetrieving() throws Exception { void doExecute(ActionType action, Request request, ActionListener listener) { if (request instanceof GetSnapshotsRequest) { logger.info("--> called"); - listener.onResponse((Response) new GetSnapshotsResponse(Collections.emptyList(), Collections.emptyMap(), null)); + listener.onResponse((Response) new GetSnapshotsResponse( + Collections.emptyList(), Collections.emptyMap(), null, 0, 0)); } else { super.doExecute(action, request, listener); }