Skip to content

Commit fe1b7f1

Browse files
authored
Combine per-repo results in get-snapshots action (#111004)
With #107191 we can now safely accumulate results from all targetted repositories as they're built, rather than staging each repository's results in intermediate lists in case of failure.
1 parent 6749371 commit fe1b7f1

File tree

1 file changed

+5
-25
lines changed

1 file changed

+5
-25
lines changed

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

Lines changed: 5 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -53,14 +53,12 @@
5353
import org.elasticsearch.transport.TransportService;
5454

5555
import java.util.ArrayList;
56-
import java.util.Collection;
5756
import java.util.Collections;
5857
import java.util.HashMap;
5958
import java.util.HashSet;
6059
import java.util.Iterator;
6160
import java.util.List;
6261
import java.util.Map;
63-
import java.util.Queue;
6462
import java.util.Set;
6563
import java.util.concurrent.atomic.AtomicInteger;
6664
import java.util.function.BiPredicate;
@@ -182,19 +180,13 @@ private class GetSnapshotsOperation {
182180
private final GetSnapshotInfoExecutor getSnapshotInfoExecutor;
183181

184182
// results
185-
private final Queue<List<SnapshotInfo>> allSnapshotInfos = ConcurrentCollections.newQueue();
183+
private final List<SnapshotInfo> allSnapshotInfos = Collections.synchronizedList(new ArrayList<>());
186184

187185
/**
188186
* Accumulates number of snapshots that match the name/fromSortValue/slmPolicy predicates, to be returned in the response.
189187
*/
190188
private final AtomicInteger totalCount = new AtomicInteger();
191189

192-
/**
193-
* Accumulates the number of snapshots that match the name/fromSortValue/slmPolicy/after predicates, for sizing the final result
194-
* list.
195-
*/
196-
private final AtomicInteger resultsCount = new AtomicInteger();
197-
198190
GetSnapshotsOperation(
199191
CancellableTask cancellableTask,
200192
List<RepositoryMetadata> repositories,
@@ -438,18 +430,7 @@ private void loadSnapshotInfos(Iterator<AsyncSnapshotInfo> asyncSnapshotInfoIter
438430
if (cancellableTask.notifyIfCancelled(listener)) {
439431
return;
440432
}
441-
final var repositoryTotalCount = new AtomicInteger();
442-
443-
final List<SnapshotInfo> snapshots = new ArrayList<>();
444-
final List<SnapshotInfo> syncSnapshots = Collections.synchronizedList(snapshots);
445-
446433
try (var listeners = new RefCountingListener(listener)) {
447-
final var iterationCompleteListener = listeners.acquire(ignored -> {
448-
totalCount.addAndGet(repositoryTotalCount.get());
449-
// no need to synchronize access to snapshots: all writes happen-before this read
450-
resultsCount.addAndGet(snapshots.size());
451-
allSnapshotInfos.add(snapshots);
452-
});
453434
ThrottledIterator.run(
454435
Iterators.failFast(asyncSnapshotInfoIterator, () -> cancellableTask.isCancelled() || listeners.isFailing()),
455436
(ref, asyncSnapshotInfo) -> {
@@ -458,9 +439,9 @@ private void loadSnapshotInfos(Iterator<AsyncSnapshotInfo> asyncSnapshotInfoIter
458439
@Override
459440
public void onResponse(SnapshotInfo snapshotInfo) {
460441
if (matchesPredicates(snapshotInfo)) {
461-
repositoryTotalCount.incrementAndGet();
442+
totalCount.incrementAndGet();
462443
if (afterPredicate.test(snapshotInfo)) {
463-
syncSnapshots.add(snapshotInfo.maybeWithoutIndices(indices));
444+
allSnapshotInfos.add(snapshotInfo.maybeWithoutIndices(indices));
464445
}
465446
}
466447
refListener.onResponse(null);
@@ -479,7 +460,7 @@ public void onFailure(Exception e) {
479460
},
480461
getSnapshotInfoExecutor.getMaxRunningTasks(),
481462
() -> {},
482-
() -> iterationCompleteListener.onResponse(null)
463+
() -> {}
483464
);
484465
}
485466
}
@@ -489,12 +470,11 @@ private GetSnapshotsResponse buildResponse() {
489470
cancellableTask.ensureNotCancelled();
490471
int remaining = 0;
491472
final var resultsStream = allSnapshotInfos.stream()
492-
.flatMap(Collection::stream)
493473
.peek(this::assertSatisfiesAllPredicates)
494474
.sorted(sortBy.getSnapshotInfoComparator(order))
495475
.skip(offset);
496476
final List<SnapshotInfo> snapshotInfos;
497-
if (size == GetSnapshotsRequest.NO_LIMIT || resultsCount.get() <= size) {
477+
if (size == GetSnapshotsRequest.NO_LIMIT || allSnapshotInfos.size() <= size) {
498478
snapshotInfos = resultsStream.toList();
499479
} else {
500480
snapshotInfos = new ArrayList<>(size);

0 commit comments

Comments
 (0)