-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Implement from_sort_value Parameter in Get Snapshots API #77618
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e2529d5
130344a
baec0ea
6f0ae18
0fa188f
2ece405
7b30c21
def381c
e4fd7fc
089b4b4
960a2c1
e34c61c
59fb31d
f7ec41a
108367a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,8 +34,11 @@ | |
| import java.util.HashSet; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Set; | ||
|
|
||
| import static org.elasticsearch.snapshots.AbstractSnapshotIntegTestCase.assertSnapshotListSorted; | ||
| import static org.elasticsearch.snapshots.AbstractSnapshotIntegTestCase.matchAllPattern; | ||
| import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; | ||
| import static org.hamcrest.Matchers.empty; | ||
| import static org.hamcrest.Matchers.in; | ||
| import static org.hamcrest.Matchers.is; | ||
|
|
@@ -241,6 +244,77 @@ public void testFilterBySLMPolicy() throws Exception { | |
| ); | ||
| } | ||
|
|
||
| public void testSortAfterStartTime() throws Exception { | ||
| final String repoName = "test-repo"; | ||
| AbstractSnapshotIntegTestCase.createRepository(logger, repoName, "fs"); | ||
| final HashSet<Long> startTimes = new HashSet<>(); | ||
| final SnapshotInfo snapshot1 = createFullSnapshotWithUniqueStartTime(repoName, "snapshot-1", startTimes); | ||
| final SnapshotInfo snapshot2 = createFullSnapshotWithUniqueStartTime(repoName, "snapshot-2", startTimes); | ||
| final SnapshotInfo snapshot3 = createFullSnapshotWithUniqueStartTime(repoName, "snapshot-3", startTimes); | ||
|
|
||
| final List<SnapshotInfo> allSnapshotInfo = clusterAdmin().prepareGetSnapshots(matchAllPattern()) | ||
| .setSnapshots(matchAllPattern()) | ||
| .setSort(GetSnapshotsRequest.SortBy.START_TIME) | ||
| .get() | ||
| .getSnapshots(); | ||
| assertThat(allSnapshotInfo, is(List.of(snapshot1, snapshot2, snapshot3))); | ||
|
|
||
| final long startTime1 = snapshot1.startTime(); | ||
| final long startTime2 = snapshot2.startTime(); | ||
| final long startTime3 = snapshot3.startTime(); | ||
|
|
||
| assertThat(allAfterStartTimeAscending(startTime1 - 1), is(allSnapshotInfo)); | ||
| assertThat(allAfterStartTimeAscending(startTime1), is(allSnapshotInfo)); | ||
| assertThat(allAfterStartTimeAscending(startTime2), is(List.of(snapshot2, snapshot3))); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we sure that
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe not on Windows where the clock is less accurate actually ... let me make sure they will.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adjusted the code to ensure the timestamps never collide now.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure I see that here in the rest test case, only in the internal cluster test? Maybe I missed it?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh right missed this one, adding a fix here as well :) |
||
| assertThat(allAfterStartTimeAscending(startTime3), is(List.of(snapshot3))); | ||
| assertThat(allAfterStartTimeAscending(startTime3 + 1), empty()); | ||
|
|
||
| final List<SnapshotInfo> allSnapshotInfoDesc = clusterAdmin().prepareGetSnapshots(matchAllPattern()) | ||
| .setSnapshots(matchAllPattern()) | ||
| .setSort(GetSnapshotsRequest.SortBy.START_TIME) | ||
| .setOrder(SortOrder.DESC) | ||
| .get() | ||
| .getSnapshots(); | ||
| assertThat(allSnapshotInfoDesc, is(List.of(snapshot3, snapshot2, snapshot1))); | ||
|
|
||
| assertThat(allBeforeStartTimeDescending(startTime3 + 1), is(allSnapshotInfoDesc)); | ||
| assertThat(allBeforeStartTimeDescending(startTime3), is(allSnapshotInfoDesc)); | ||
| assertThat(allBeforeStartTimeDescending(startTime2), is(List.of(snapshot2, snapshot1))); | ||
| assertThat(allBeforeStartTimeDescending(startTime1), is(List.of(snapshot1))); | ||
| assertThat(allBeforeStartTimeDescending(startTime1 - 1), empty()); | ||
| } | ||
|
|
||
| // create a snapshot that is guaranteed to have a unique start time | ||
| private SnapshotInfo createFullSnapshotWithUniqueStartTime(String repoName, String snapshotName, Set<Long> forbiddenStartTimes) { | ||
| while (true) { | ||
| final SnapshotInfo snapshotInfo = AbstractSnapshotIntegTestCase.createFullSnapshot(logger, repoName, snapshotName); | ||
| if (forbiddenStartTimes.contains(snapshotInfo.startTime())) { | ||
| logger.info("--> snapshot start time collided"); | ||
| assertAcked(clusterAdmin().prepareDeleteSnapshot(repoName, snapshotName).get()); | ||
| } else { | ||
| assertTrue(forbiddenStartTimes.add(snapshotInfo.startTime())); | ||
| return snapshotInfo; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private List<SnapshotInfo> allAfterStartTimeAscending(long timestamp) throws IOException { | ||
| final Request request = baseGetSnapshotsRequest("*"); | ||
| request.addParameter("sort", GetSnapshotsRequest.SortBy.START_TIME.toString()); | ||
| request.addParameter("from_sort_value", String.valueOf(timestamp)); | ||
| final Response response = getRestClient().performRequest(request); | ||
| return readSnapshotInfos(response).getSnapshots(); | ||
| } | ||
|
|
||
| private List<SnapshotInfo> allBeforeStartTimeDescending(long timestamp) throws IOException { | ||
| final Request request = baseGetSnapshotsRequest("*"); | ||
| request.addParameter("sort", GetSnapshotsRequest.SortBy.START_TIME.toString()); | ||
| request.addParameter("from_sort_value", String.valueOf(timestamp)); | ||
| request.addParameter("order", SortOrder.DESC.toString()); | ||
| final Response response = getRestClient().performRequest(request); | ||
| return readSnapshotInfos(response).getSnapshots(); | ||
| } | ||
|
|
||
| private static List<SnapshotInfo> getAllSnapshotsForPolicies(String... policies) throws IOException { | ||
| final Request requestWithPolicy = new Request(HttpGet.METHOD_NAME, "/_snapshot/*/*"); | ||
| requestWithPolicy.addParameter("slm_policy_filter", Strings.arrayToCommaDelimitedString(policies)); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect that sorting after a given start/end time will be the most common use case. Maybe it's worth showing as an example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd love to but I don't see a way of doing this with a test because the timestamp on the request would have to be dynamic right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or low enough to always return some snapshots in the response. But that's just a suggestion, let's ignore if that's too complicated or ugly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair point :) Added a docs run for this with a low timestamp now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!