Skip to content

Commit

Permalink
Implement Sort By Repository Name in Get Snapshots API (elastic#77049)
Browse files Browse the repository at this point in the history 
This one is the last sort column not yet implemented but used by Kibana.
  • Loading branch information
@original-brownbear
original-brownbear committed Sep 1, 2021
1 parent eff6cd6 commit cb37117
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 13 deletions.
3 changes: 3 additions & 0 deletions docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,9 @@ Allows setting a sort order for the result. Defaults to `start_time`, i.e. sorti
`name`::
Sort snapshots by their name.
`repository`::
Sort snapshots by their repository name and break ties by snapshot name.
`index_count`::
Sort snapshots by the number of indices they contain and break ties by snapshot name.
Expand Down
5 changes: 5 additions & 0 deletions qa/smoke-test-http/src/test/java/org/elasticsearch/http/snapshots/RestGetSnapshotsIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ private void doTestSortOrder(String repoName, Collection<String> allSnapshotName
GetSnapshotsRequest.SortBy.FAILED_SHARDS,
order
);
assertSnapshotListSorted(
allSnapshotsSorted(allSnapshotNames, repoName, GetSnapshotsRequest.SortBy.REPOSITORY, order),
GetSnapshotsRequest.SortBy.REPOSITORY,
order
);
}

public void testResponseSizeLimit() throws Exception {
Expand Down
38 changes: 29 additions & 9 deletions server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,21 +38,36 @@ protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
}

public void testSortBy() throws Exception {
final String repoName = "test-repo";
final String repoNameA = "test-repo-a";
final Path repoPath = randomRepoPath();
createRepository(repoName, "fs", repoPath);
maybeInitWithOldSnapshotVersion(repoName, repoPath);
final List<String> snapshotNamesWithoutIndex = createNSnapshots(repoName, randomIntBetween(3, 20));
createRepository(repoNameA, "fs", repoPath);
maybeInitWithOldSnapshotVersion(repoNameA, repoPath);
final String repoNameB = "test-repo-b";
createRepository(repoNameB, "fs");

final List<String> snapshotNamesWithoutIndexA = createNSnapshots(repoNameA, randomIntBetween(3, 20));
final List<String> snapshotNamesWithoutIndexB = createNSnapshots(repoNameB, randomIntBetween(3, 20));

createIndexWithContent("test-index");

final List<String> snapshotNamesWithIndex = createNSnapshots(repoName, randomIntBetween(3, 20));
final List<String> snapshotNamesWithIndexA = createNSnapshots(repoNameA, randomIntBetween(3, 20));
final List<String> snapshotNamesWithIndexB = createNSnapshots(repoNameB, randomIntBetween(3, 20));

final Collection<String> allSnapshotNamesA = new HashSet<>(snapshotNamesWithIndexA);
final Collection<String> allSnapshotNamesB = new HashSet<>(snapshotNamesWithIndexB);
allSnapshotNamesA.addAll(snapshotNamesWithoutIndexA);
allSnapshotNamesB.addAll(snapshotNamesWithoutIndexB);

final Collection<String> allSnapshotNames = new HashSet<>(snapshotNamesWithIndex);
allSnapshotNames.addAll(snapshotNamesWithoutIndex);
doTestSortOrder(repoNameA, allSnapshotNamesA, SortOrder.ASC);
doTestSortOrder(repoNameA, allSnapshotNamesA, SortOrder.DESC);

doTestSortOrder(repoName, allSnapshotNames, SortOrder.ASC);
doTestSortOrder(repoName, allSnapshotNames, SortOrder.DESC);
doTestSortOrder(repoNameB, allSnapshotNamesB, SortOrder.ASC);
doTestSortOrder(repoNameB, allSnapshotNamesB, SortOrder.DESC);

final Collection<String> allSnapshots = new HashSet<>(allSnapshotNamesA);
allSnapshots.addAll(allSnapshotNamesB);
doTestSortOrder("*", allSnapshots, SortOrder.ASC);
doTestSortOrder("*", allSnapshots, SortOrder.DESC);
}

private void doTestSortOrder(String repoName, Collection<String> allSnapshotNames, SortOrder order) {
Expand Down Expand Up @@ -88,6 +103,11 @@ private void doTestSortOrder(String repoName, Collection<String> allSnapshotName
GetSnapshotsRequest.SortBy.FAILED_SHARDS,
order
);
assertSnapshotListSorted(
allSnapshotsSorted(allSnapshotNames, repoName, GetSnapshotsRequest.SortBy.REPOSITORY, order),
GetSnapshotsRequest.SortBy.REPOSITORY,
order
);
}

public void testResponseSizeLimit() throws Exception {
Expand Down
17 changes: 13 additions & 4 deletions ...c/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public class GetSnapshotsRequest extends MasterNodeRequest<GetSnapshotsRequest>

public static final Version NUMERIC_PAGINATION_VERSION = Version.V_7_15_0;

private static final Version SORT_BY_SHARD_COUNTS_VERSION = Version.V_7_16_0;
private static final Version SORT_BY_SHARDS_OR_REPO_VERSION = Version.V_7_16_0;

public static final int NO_LIMIT = -1;

Expand Down Expand Up @@ -149,8 +149,11 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeBoolean(verbose);
if (out.getVersion().onOrAfter(PAGINATED_GET_SNAPSHOTS_VERSION)) {
out.writeOptionalWriteable(after);
if ((sort == SortBy.SHARDS || sort == SortBy.FAILED_SHARDS) && out.getVersion().before(SORT_BY_SHARD_COUNTS_VERSION)) {
throw new IllegalArgumentException("can't use sort by shard count with node version [" + out.getVersion() + "]");
if ((sort == SortBy.SHARDS || sort == SortBy.FAILED_SHARDS || sort == SortBy.REPOSITORY)
&& out.getVersion().before(SORT_BY_SHARDS_OR_REPO_VERSION)) {
throw new IllegalArgumentException(
"can't use sort by shard count or repository name with node version [" + out.getVersion() + "]"
);
}
out.writeEnum(sort);
out.writeVInt(size);
Expand Down Expand Up @@ -363,7 +366,8 @@ public enum SortBy {
DURATION("duration"),
INDICES("index_count"),
SHARDS("shard_count"),
FAILED_SHARDS("failed_shard_count");
FAILED_SHARDS("failed_shard_count"),
REPOSITORY("repository");

private final String param;

Expand All @@ -390,6 +394,8 @@ public static SortBy of(String value) {
return SHARDS;
case "failed_shard_count":
return FAILED_SHARDS;
case "repository":
return REPOSITORY;
default:
throw new IllegalArgumentException("unknown sort order [" + value + "]");
}
Expand Down Expand Up @@ -441,6 +447,9 @@ public static After from(@Nullable SnapshotInfo snapshotInfo, SortBy sortBy) {
case FAILED_SHARDS:
afterValue = String.valueOf(snapshotInfo.failedShards());
break;
case REPOSITORY:
afterValue = snapshotInfo.repository();
break;
default:
throw new AssertionError("unknown sort column [" + sortBy + "]");
}
Expand Down
19 changes: 19 additions & 0 deletions ...ava/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,9 @@ private static SnapshotsInRepo buildSimpleSnapshotInfos(

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

private static final Comparator<SnapshotInfo> BY_REPOSITORY = Comparator.comparing(SnapshotInfo::repository)
.thenComparing(SnapshotInfo::snapshotId);

private static SnapshotsInRepo sortSnapshots(
final List<SnapshotInfo> snapshotInfos,
final GetSnapshotsRequest.SortBy sortBy,
Expand Down Expand Up @@ -526,6 +529,9 @@ private static SnapshotsInRepo sortSnapshots(
case FAILED_SHARDS:
comparator = BY_FAILED_SHARDS_COUNT;
break;
case REPOSITORY:
comparator = BY_REPOSITORY;
break;
default:
throw new AssertionError("unexpected sort column [" + sortBy + "]");
}
Expand Down Expand Up @@ -576,6 +582,11 @@ private static SnapshotsInRepo sortSnapshots(
order
);
break;
case REPOSITORY:
isAfter = order == SortOrder.ASC
? (info -> compareRepositoryName(snapshotName, repoName, info) < 0)
: (info -> compareRepositoryName(snapshotName, repoName, info) > 0);
break;
default:
throw new AssertionError("unexpected sort column [" + sortBy + "]");
}
Expand Down Expand Up @@ -612,6 +623,14 @@ private static Predicate<SnapshotInfo> filterByLongOffset(
};
}

private static int compareRepositoryName(String name, String repoName, SnapshotInfo info) {
final int res = repoName.compareTo(info.repository());
if (res != 0) {
return res;
}
return name.compareTo(info.snapshotId().getName());
}

private static int compareName(String name, String repoName, SnapshotInfo info) {
final int res = name.compareTo(info.snapshotId().getName());
if (res != 0) {
Expand Down
3 changes: 3 additions & 0 deletions test/framework/src/main/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -706,6 +706,9 @@ public static void assertSnapshotListSorted(List<SnapshotInfo> snapshotInfos, @N
case FAILED_SHARDS:
assertion = (s1, s2) -> assertThat(s2.failedShards(), greaterThanOrEqualTo(s1.failedShards()));
break;
case REPOSITORY:
assertion = (s1, s2) -> assertThat(s2.repository(), greaterThanOrEqualTo(s1.repository()));
break;
default:
throw new AssertionError("unknown sort column [" + sort + "]");
}
Expand Down

0 comments on commit cb37117

Please sign in to comment.