Skip to content

Commit 0920e21

Browse files
original-brownbearoriginal-brownbear
authored
Implement Sort By Repository Name in Get Snapshots API (#77049)
This one is the last sort column not yet implemented but used by Kibana.
1 parent 54d4737 commit 0920e21

File tree

6 files changed

+72
-13
lines changed

6 files changed

+72
-13
lines changed

‎docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,9 @@ Allows setting a sort order for the result. Defaults to `start_time`, i.e. sorti
118118
`name`::
119119
Sort snapshots by their name.
120120
121+
`repository`::
122+
Sort snapshots by their repository name and break ties by snapshot name.
123+
121124
`index_count`::
122125
Sort snapshots by the number of indices they contain and break ties by snapshot name.
123126

‎qa/smoke-test-http/src/test/java/org/elasticsearch/http/snapshots/RestGetSnapshotsIT.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,11 @@ private void doTestSortOrder(String repoName, Collection<String> allSnapshotName
9696
GetSnapshotsRequest.SortBy.FAILED_SHARDS,
9797
order
9898
);
99+
assertSnapshotListSorted(
100+
allSnapshotsSorted(allSnapshotNames, repoName, GetSnapshotsRequest.SortBy.REPOSITORY, order),
101+
GetSnapshotsRequest.SortBy.REPOSITORY,
102+
order
103+
);
99104
}
100105

101106
public void testResponseSizeLimit() throws Exception {

‎server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java‎

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,21 +38,36 @@ protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
3838
}
3939

4040
public void testSortBy() throws Exception {
41-
final String repoName = "test-repo";
41+
final String repoNameA = "test-repo-a";
4242
final Path repoPath = randomRepoPath();
43-
createRepository(repoName, "fs", repoPath);
44-
maybeInitWithOldSnapshotVersion(repoName, repoPath);
45-
final List<String> snapshotNamesWithoutIndex = createNSnapshots(repoName, randomIntBetween(3, 20));
43+
createRepository(repoNameA, "fs", repoPath);
44+
maybeInitWithOldSnapshotVersion(repoNameA, repoPath);
45+
final String repoNameB = "test-repo-b";
46+
createRepository(repoNameB, "fs");
47+
48+
final List<String> snapshotNamesWithoutIndexA = createNSnapshots(repoNameA, randomIntBetween(3, 20));
49+
final List<String> snapshotNamesWithoutIndexB = createNSnapshots(repoNameB, randomIntBetween(3, 20));
4650

4751
createIndexWithContent("test-index");
4852

49-
final List<String> snapshotNamesWithIndex = createNSnapshots(repoName, randomIntBetween(3, 20));
53+
final List<String> snapshotNamesWithIndexA = createNSnapshots(repoNameA, randomIntBetween(3, 20));
54+
final List<String> snapshotNamesWithIndexB = createNSnapshots(repoNameB, randomIntBetween(3, 20));
55+
56+
final Collection<String> allSnapshotNamesA = new HashSet<>(snapshotNamesWithIndexA);
57+
final Collection<String> allSnapshotNamesB = new HashSet<>(snapshotNamesWithIndexB);
58+
allSnapshotNamesA.addAll(snapshotNamesWithoutIndexA);
59+
allSnapshotNamesB.addAll(snapshotNamesWithoutIndexB);
5060

51-
final Collection<String> allSnapshotNames = new HashSet<>(snapshotNamesWithIndex);
52-
allSnapshotNames.addAll(snapshotNamesWithoutIndex);
61+
doTestSortOrder(repoNameA, allSnapshotNamesA, SortOrder.ASC);
62+
doTestSortOrder(repoNameA, allSnapshotNamesA, SortOrder.DESC);
5363

54-
doTestSortOrder(repoName, allSnapshotNames, SortOrder.ASC);
55-
doTestSortOrder(repoName, allSnapshotNames, SortOrder.DESC);
64+
doTestSortOrder(repoNameB, allSnapshotNamesB, SortOrder.ASC);
65+
doTestSortOrder(repoNameB, allSnapshotNamesB, SortOrder.DESC);
66+
67+
final Collection<String> allSnapshots = new HashSet<>(allSnapshotNamesA);
68+
allSnapshots.addAll(allSnapshotNamesB);
69+
doTestSortOrder("*", allSnapshots, SortOrder.ASC);
70+
doTestSortOrder("*", allSnapshots, SortOrder.DESC);
5671
}
5772

5873
private void doTestSortOrder(String repoName, Collection<String> allSnapshotNames, SortOrder order) {
@@ -88,6 +103,11 @@ private void doTestSortOrder(String repoName, Collection<String> allSnapshotName
88103
GetSnapshotsRequest.SortBy.FAILED_SHARDS,
89104
order
90105
);
106+
assertSnapshotListSorted(
107+
allSnapshotsSorted(allSnapshotNames, repoName, GetSnapshotsRequest.SortBy.REPOSITORY, order),
108+
GetSnapshotsRequest.SortBy.REPOSITORY,
109+
order
110+
);
91111
}
92112

93113
public void testResponseSizeLimit() throws Exception {

‎server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public class GetSnapshotsRequest extends MasterNodeRequest<GetSnapshotsRequest>
4646

4747
public static final Version NUMERIC_PAGINATION_VERSION = Version.V_7_15_0;
4848

49-
private static final Version SORT_BY_SHARD_COUNTS_VERSION = Version.V_7_16_0;
49+
private static final Version SORT_BY_SHARDS_OR_REPO_VERSION = Version.V_7_16_0;
5050

5151
public static final int NO_LIMIT = -1;
5252

@@ -138,8 +138,11 @@ public void writeTo(StreamOutput out) throws IOException {
138138
out.writeBoolean(verbose);
139139
if (out.getVersion().onOrAfter(PAGINATED_GET_SNAPSHOTS_VERSION)) {
140140
out.writeOptionalWriteable(after);
141-
if ((sort == SortBy.SHARDS || sort == SortBy.FAILED_SHARDS) && out.getVersion().before(SORT_BY_SHARD_COUNTS_VERSION)) {
142-
throw new IllegalArgumentException("can't use sort by shard count with node version [" + out.getVersion() + "]");
141+
if ((sort == SortBy.SHARDS || sort == SortBy.FAILED_SHARDS || sort == SortBy.REPOSITORY)
142+
&& out.getVersion().before(SORT_BY_SHARDS_OR_REPO_VERSION)) {
143+
throw new IllegalArgumentException(
144+
"can't use sort by shard count or repository name with node version [" + out.getVersion() + "]"
145+
);
143146
}
144147
out.writeEnum(sort);
145148
out.writeVInt(size);
@@ -327,7 +330,8 @@ public enum SortBy {
327330
DURATION("duration"),
328331
INDICES("index_count"),
329332
SHARDS("shard_count"),
330-
FAILED_SHARDS("failed_shard_count");
333+
FAILED_SHARDS("failed_shard_count"),
334+
REPOSITORY("repository");
331335

332336
private final String param;
333337

@@ -354,6 +358,8 @@ public static SortBy of(String value) {
354358
return SHARDS;
355359
case "failed_shard_count":
356360
return FAILED_SHARDS;
361+
case "repository":
362+
return REPOSITORY;
357363
default:
358364
throw new IllegalArgumentException("unknown sort order [" + value + "]");
359365
}
@@ -405,6 +411,9 @@ public static After from(@Nullable SnapshotInfo snapshotInfo, SortBy sortBy) {
405411
case FAILED_SHARDS:
406412
afterValue = String.valueOf(snapshotInfo.failedShards());
407413
break;
414+
case REPOSITORY:
415+
afterValue = snapshotInfo.repository();
416+
break;
408417
default:
409418
throw new AssertionError("unknown sort column [" + sortBy + "]");
410419
}

‎server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,9 @@ private static SnapshotsInRepo buildSimpleSnapshotInfos(
492492

493493
private static final Comparator<SnapshotInfo> BY_NAME = Comparator.comparing(sni -> sni.snapshotId().getName());
494494

495+
private static final Comparator<SnapshotInfo> BY_REPOSITORY = Comparator.comparing(SnapshotInfo::repository)
496+
.thenComparing(SnapshotInfo::snapshotId);
497+
495498
private static SnapshotsInRepo sortSnapshots(
496499
final List<SnapshotInfo> snapshotInfos,
497500
final GetSnapshotsRequest.SortBy sortBy,
@@ -520,6 +523,9 @@ private static SnapshotsInRepo sortSnapshots(
520523
case FAILED_SHARDS:
521524
comparator = BY_FAILED_SHARDS_COUNT;
522525
break;
526+
case REPOSITORY:
527+
comparator = BY_REPOSITORY;
528+
break;
523529
default:
524530
throw new AssertionError("unexpected sort column [" + sortBy + "]");
525531
}
@@ -570,6 +576,11 @@ private static SnapshotsInRepo sortSnapshots(
570576
order
571577
);
572578
break;
579+
case REPOSITORY:
580+
isAfter = order == SortOrder.ASC
581+
? (info -> compareRepositoryName(snapshotName, repoName, info) < 0)
582+
: (info -> compareRepositoryName(snapshotName, repoName, info) > 0);
583+
break;
573584
default:
574585
throw new AssertionError("unexpected sort column [" + sortBy + "]");
575586
}
@@ -606,6 +617,14 @@ private static Predicate<SnapshotInfo> filterByLongOffset(
606617
};
607618
}
608619

620+
private static int compareRepositoryName(String name, String repoName, SnapshotInfo info) {
621+
final int res = repoName.compareTo(info.repository());
622+
if (res != 0) {
623+
return res;
624+
}
625+
return name.compareTo(info.snapshotId().getName());
626+
}
627+
609628
private static int compareName(String name, String repoName, SnapshotInfo info) {
610629
final int res = name.compareTo(info.snapshotId().getName());
611630
if (res != 0) {

‎test/framework/src/main/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -711,6 +711,9 @@ public static void assertSnapshotListSorted(List<SnapshotInfo> snapshotInfos, @N
711711
case FAILED_SHARDS:
712712
assertion = (s1, s2) -> assertThat(s2.failedShards(), greaterThanOrEqualTo(s1.failedShards()));
713713
break;
714+
case REPOSITORY:
715+
assertion = (s1, s2) -> assertThat(s2.repository(), greaterThanOrEqualTo(s1.repository()));
716+
break;
714717
default:
715718
throw new AssertionError("unknown sort column [" + sort + "]");
716719
}

0 commit comments

Comments
 (0)