Browse Source

Streamline dropping indices in get-snapshtos API (#94893)

Today when calling the get-snapshots API with `?indices=false` we still
do a bunch of unnecessary work to invert the index/snapshot mapping for
all snapshots. This commit pushes the index-dropping logic lower down to
skip this unnecessary work.
David Turner 2 years ago
parent
commit
b078317686

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

@@ -189,9 +189,10 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
                 order
             );
             final var snapshotInfos = sortedSnapshotsInRepos.snapshotInfos();
+            assert indices || snapshotInfos.stream().allMatch(snapshotInfo -> snapshotInfo.indices().isEmpty());
             final int finalRemaining = sortedSnapshotsInRepos.remaining() + remaining.get();
             return new GetSnapshotsResponse(
-                indices ? snapshotInfos : snapshotInfos.stream().map(SnapshotInfo::withoutIndices).toList(),
+                snapshotInfos,
                 failures,
                 finalRemaining > 0
                     ? GetSnapshotsRequest.After.from(snapshotInfos.get(snapshotInfos.size() - 1), sortBy).asQueryParam()
@@ -213,6 +214,7 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
                     sortBy,
                     after,
                     order,
+                    indices,
                     listeners.acquire((SnapshotsInRepo snapshotsInRepo) -> {
                         allSnapshotInfos.add(snapshotsInRepo.snapshotInfos());
                         remaining.addAndGet(snapshotsInRepo.remaining());
@@ -241,6 +243,7 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
         GetSnapshotsRequest.SortBy sortBy,
         @Nullable final GetSnapshotsRequest.After after,
         SortOrder order,
+        boolean indices,
         ActionListener<SnapshotsInRepo> listener
     ) {
         final Map<String, Snapshot> allSnapshotIds = new HashMap<>();
@@ -248,7 +251,7 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
         for (SnapshotInfo snapshotInfo : currentSnapshots(snapshotsInProgress, repo)) {
             Snapshot snapshot = snapshotInfo.snapshot();
             allSnapshotIds.put(snapshot.getSnapshotId().getName(), snapshot);
-            currentSnapshots.add(snapshotInfo);
+            currentSnapshots.add(snapshotInfo.maybeWithoutIndices(indices));
         }
 
         final StepListener<RepositoryData> repositoryDataListener = new StepListener<>();
@@ -273,6 +276,7 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
                 after,
                 order,
                 predicates,
+                indices,
                 listener
             ),
             listener::onFailure
@@ -313,6 +317,7 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
         @Nullable final GetSnapshotsRequest.After after,
         SortOrder order,
         SnapshotPredicates predicates,
+        boolean indices,
         ActionListener<SnapshotsInRepo> listener
     ) {
         if (task.notifyIfCancelled(listener)) {
@@ -387,6 +392,7 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
                 after,
                 order,
                 predicates,
+                indices,
                 listener
             );
         } else {
@@ -394,7 +400,7 @@ 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, order);
+                snapshotInfos = buildSimpleSnapshotInfos(toResolve, repo, repositoryData, currentSnapshots, sortBy, after, order, indices);
             } else {
                 // only want current snapshots
                 snapshotInfos = sortSnapshots(
@@ -412,10 +418,12 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
 
     /**
      * Returns a list of snapshots from repository sorted by snapshot creation date
-     *  @param snapshotsInProgress snapshots in progress in the cluster state
+     *
+     * @param snapshotsInProgress snapshots in progress in the cluster state
      * @param repositoryName      repository name
      * @param snapshotIds         snapshots for which to fetch snapshot information
      * @param ignoreUnavailable   if true, snapshots that could not be read will only be logged with a warning,
+     * @param indices             if false, drop the list of indices from each result
      */
     private void snapshots(
         SnapshotsInProgress snapshotsInProgress,
@@ -427,6 +435,7 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
         @Nullable GetSnapshotsRequest.After after,
         SortOrder order,
         SnapshotPredicates predicate,
+        boolean indices,
         ActionListener<SnapshotsInRepo> listener
     ) {
         if (task.notifyIfCancelled(listener)) {
@@ -444,7 +453,7 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
             if (snapshotIdsToIterate.remove(entry.snapshot().getSnapshotId())) {
                 final SnapshotInfo snapshotInfo = SnapshotInfo.inProgress(entry);
                 if (predicate.test(snapshotInfo)) {
-                    snapshotSet.add(snapshotInfo);
+                    snapshotSet.add(snapshotInfo.maybeWithoutIndices(indices));
                 }
             }
         }
@@ -474,7 +483,7 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
         repository.getSnapshotInfo(
             new GetSnapshotInfoContext(snapshotIdsToIterate, ignoreUnavailable == false, task::isCancelled, (context, snapshotInfo) -> {
                 if (predicate.test(snapshotInfo)) {
-                    snapshotInfos.add(snapshotInfo);
+                    snapshotInfos.add(snapshotInfo.maybeWithoutIndices(indices));
                 }
             }, allDoneListener)
         );
@@ -491,7 +500,8 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
         final List<SnapshotInfo> currentSnapshots,
         final GetSnapshotsRequest.SortBy sortBy,
         @Nullable final GetSnapshotsRequest.After after,
-        final SortOrder order
+        final SortOrder order,
+        boolean indices
     ) {
         List<SnapshotInfo> snapshotInfos = new ArrayList<>();
         for (SnapshotInfo snapshotInfo : currentSnapshots) {
@@ -500,10 +510,12 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
             }
         }
         Map<SnapshotId, List<String>> snapshotsToIndices = new HashMap<>();
-        for (IndexId indexId : repositoryData.getIndices().values()) {
-            for (SnapshotId snapshotId : repositoryData.getSnapshots(indexId)) {
-                if (toResolve.contains(new Snapshot(repoName, snapshotId))) {
-                    snapshotsToIndices.computeIfAbsent(snapshotId, (k) -> new ArrayList<>()).add(indexId.getName());
+        if (indices) {
+            for (IndexId indexId : repositoryData.getIndices().values()) {
+                for (SnapshotId snapshotId : repositoryData.getSnapshots(indexId)) {
+                    if (toResolve.contains(new Snapshot(repoName, snapshotId))) {
+                        snapshotsToIndices.computeIfAbsent(snapshotId, (k) -> new ArrayList<>()).add(indexId.getName());
+                    }
                 }
             }
         }

+ 2 - 2
server/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java

@@ -469,8 +469,8 @@ public final class SnapshotInfo implements Comparable<SnapshotInfo>, ToXContentF
         this.indexSnapshotDetails = Map.copyOf(indexSnapshotDetails);
     }
 
-    public SnapshotInfo withoutIndices() {
-        if (indices.isEmpty()) {
+    public SnapshotInfo maybeWithoutIndices(boolean retainIndices) {
+        if (retainIndices || indices.isEmpty()) {
             return this;
         }
         return new SnapshotInfo(