Browse Source

Implement Numeric Offset Parameter in Get Snapshots API (#76233)

Add numeric offset parameter to this API.

Relates #74350
Armin Braun 4 years ago
parent
commit
ffaa0f2742

+ 57 - 2
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_id>,
+      "version": <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>/"version_id": $body.snapshots.0.version_id/]
+// TESTRESPONSE[s/"version": <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]
 ----
 {

+ 26 - 4
qa/smoke-test-http/src/test/java/org/elasticsearch/http/snapshots/RestGetSnapshotsIT.java

@@ -191,10 +191,14 @@ public class RestGetSnapshotsIT extends AbstractSnapshotRestTestCase {
             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<SnapshotInfo> 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 @@ public class RestGetSnapshotsIT extends AbstractSnapshotRestTestCase {
     }
 
     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 @@ public class RestGetSnapshotsIT extends AbstractSnapshotRestTestCase {
         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);
+    }
 }

+ 15 - 6
server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java

@@ -197,11 +197,15 @@ public class GetSnapshotsIT extends AbstractSnapshotIntegTestCase {
                     i,
                     order
                 );
+                final GetSnapshotsResponse getSnapshotsResponseNumeric = sortedWithLimit(repoName, sort, j + 1, i, order);
                 final List<SnapshotInfo> 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 @@ public class GetSnapshotsIT extends AbstractSnapshotIntegTestCase {
         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) {

+ 29 - 0
server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java

@@ -52,6 +52,11 @@ public class GetSnapshotsRequest extends MasterNodeRequest<GetSnapshotsRequest>
      */
     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;
 
@@ -104,6 +109,9 @@ public class GetSnapshotsRequest extends MasterNodeRequest<GetSnapshotsRequest>
             sort = in.readEnum(SortBy.class);
             size = in.readVInt();
             order = SortOrder.readFromStream(in);
+            if (in.getVersion().onOrAfter(NUMERIC_PAGINATION_VERSION)) {
+                offset = in.readVInt();
+            }
         }
     }
 
@@ -130,6 +138,13 @@ public class GetSnapshotsRequest extends MasterNodeRequest<GetSnapshotsRequest>
             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() + "]");
         }
@@ -151,12 +166,17 @@ public class GetSnapshotsRequest extends MasterNodeRequest<GetSnapshotsRequest>
             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;
     }
@@ -265,6 +285,15 @@ public class GetSnapshotsRequest extends MasterNodeRequest<GetSnapshotsRequest>
         return size;
     }
 
+    public int offset() {
+        return offset;
+    }
+
+    public GetSnapshotsRequest offset(int offset) {
+        this.offset = offset;
+        return this;
+    }
+
     public SortOrder order() {
         return order;
     }

+ 5 - 0
server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestBuilder.java

@@ -116,6 +116,11 @@ public class GetSnapshotsRequestBuilder extends MasterNodeOperationRequestBuilde
         return this;
     }
 
+    public GetSnapshotsRequestBuilder setOffset(int offset) {
+        request.offset(offset);
+        return this;
+    }
+
     public GetSnapshotsRequestBuilder setOrder(SortOrder order) {
         request.order(order);
         return this;

+ 11 - 13
server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java

@@ -117,6 +117,7 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
             (CancellableTask) task,
             request.sort(),
             request.after(),
+            request.offset(),
             request.size(),
             request.order(),
             listener
@@ -133,6 +134,7 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
         CancellableTask cancellableTask,
         GetSnapshotsRequest.SortBy sortBy,
         @Nullable GetSnapshotsRequest.After after,
+        int offset,
         int size,
         SortOrder order,
         ActionListener<GetSnapshotsResponse> listener
@@ -154,7 +156,7 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
                     .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<SnapshotInfo> snapshotInfos = snInfos.snapshotInfos;
                 final int remaining = snInfos.remaining + responses.stream()
                     .map(Tuple::v2)
@@ -183,7 +185,6 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
                 cancellableTask,
                 sortBy,
                 after,
-                size,
                 order,
                 groupedActionListener.delegateResponse((groupedListener, e) -> {
                     if (isMultiRepoRequest && e instanceof ElasticsearchException) {
@@ -205,7 +206,6 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
         CancellableTask task,
         GetSnapshotsRequest.SortBy sortBy,
         @Nullable final GetSnapshotsRequest.After after,
-        int size,
         SortOrder order,
         ActionListener<SnapshotsInRepo> listener
     ) {
@@ -237,7 +237,6 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
                 task,
                 sortBy,
                 after,
-                size,
                 order,
                 listener
             ),
@@ -277,7 +276,6 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
         CancellableTask task,
         GetSnapshotsRequest.SortBy sortBy,
         @Nullable final GetSnapshotsRequest.After after,
-        int size,
         SortOrder order,
         ActionListener<SnapshotsInRepo> listener
     ) {
@@ -327,7 +325,6 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
                 task,
                 sortBy,
                 after,
-                size,
                 order,
                 listener
             );
@@ -335,14 +332,15 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
             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
                 );
             }
@@ -365,7 +363,6 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
         CancellableTask task,
         GetSnapshotsRequest.SortBy sortBy,
         @Nullable GetSnapshotsRequest.After after,
-        int size,
         SortOrder order,
         ActionListener<SnapshotsInRepo> listener
     ) {
@@ -395,7 +392,7 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
         final ActionListener<Void> allDoneListener = listener.delegateFailure((l, v) -> {
             final ArrayList<SnapshotInfo> 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);
@@ -445,7 +442,6 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
         final List<SnapshotInfo> currentSnapshots,
         final GetSnapshotsRequest.SortBy sortBy,
         @Nullable final GetSnapshotsRequest.After after,
-        final int size,
         final SortOrder order
     ) {
         List<SnapshotInfo> snapshotInfos = new ArrayList<>();
@@ -475,7 +471,7 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
                 )
             );
         }
-        return sortSnapshots(snapshotInfos, sortBy, after, size, order);
+        return sortSnapshots(snapshotInfos, sortBy, after, 0, GetSnapshotsRequest.NO_LIMIT, order);
     }
 
     private static final Comparator<SnapshotInfo> BY_START_TIME = Comparator.comparingLong(SnapshotInfo::startTime)
@@ -494,6 +490,7 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
         final List<SnapshotInfo> snapshotInfos,
         final GetSnapshotsRequest.SortBy sortBy,
         final @Nullable GetSnapshotsRequest.After after,
+        final int offset,
         final int size,
         final SortOrder order
     ) {
@@ -518,6 +515,7 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
         Stream<SnapshotInfo> infos = snapshotInfos.stream();
 
         if (after != null) {
+            assert offset == 0 : "can't combine after and offset but saw [" + after + "] and offset [" + offset + "]";
             final Predicate<SnapshotInfo> isAfter;
             final String snapshotName = after.snapshotName();
             final String repoName = after.repoName();
@@ -553,7 +551,7 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
             }
             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<SnapshotInfo> allSnapshots = infos.collect(Collectors.toUnmodifiableList());
         final List<SnapshotInfo> snapshots;
         if (size != GetSnapshotsRequest.NO_LIMIT) {

+ 3 - 0
server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestGetSnapshotsAction.java

@@ -58,10 +58,13 @@ public class RestGetSnapshotsAction extends BaseRestHandler {
         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()));

+ 12 - 0
server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestTests.java

@@ -32,6 +32,11 @@ public class GetSnapshotsRequestTests extends ESTestCase {
             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 class GetSnapshotsRequestTests extends ESTestCase {
             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"));
+        }
     }
 }