Skip to content

Commit d275b24

Browse files
committed
Combine per-repo results in get-snapshots action
With elastic#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 b5a7bdf commit d275b24

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,
@@ -454,18 +446,7 @@ private void loadSnapshotInfos(Iterator<AsyncSnapshotInfo> asyncSnapshotInfoIter
454446
if (cancellableTask.notifyIfCancelled(listener)) {
455447
return;
456448
}
457-
final var repositoryTotalCount = new AtomicInteger();
458-
459-
final List<SnapshotInfo> snapshots = new ArrayList<>();
460-
final List<SnapshotInfo> syncSnapshots = Collections.synchronizedList(snapshots);
461-
462449
try (var listeners = new RefCountingListener(listener)) {
463-
final var iterationCompleteListener = listeners.acquire(ignored -> {
464-
totalCount.addAndGet(repositoryTotalCount.get());
465-
// no need to synchronize access to snapshots: all writes happen-before this read
466-
resultsCount.addAndGet(snapshots.size());
467-
allSnapshotInfos.add(snapshots);
468-
});
469450
ThrottledIterator.run(
470451
Iterators.failFast(asyncSnapshotInfoIterator, () -> cancellableTask.isCancelled() || listeners.isFailing()),
471452
(ref, asyncSnapshotInfo) -> {
@@ -474,9 +455,9 @@ private void loadSnapshotInfos(Iterator<AsyncSnapshotInfo> asyncSnapshotInfoIter
474455
@Override
475456
public void onResponse(SnapshotInfo snapshotInfo) {
476457
if (matchesPredicates(snapshotInfo)) {
477-
repositoryTotalCount.incrementAndGet();
458+
totalCount.incrementAndGet();
478459
if (afterPredicate.test(snapshotInfo)) {
479-
syncSnapshots.add(snapshotInfo.maybeWithoutIndices(indices));
460+
allSnapshotInfos.add(snapshotInfo.maybeWithoutIndices(indices));
480461
}
481462
}
482463
refListener.onResponse(null);
@@ -495,7 +476,7 @@ public void onFailure(Exception e) {
495476
},
496477
getSnapshotInfoExecutor.getMaxRunningTasks(),
497478
() -> {},
498-
() -> iterationCompleteListener.onResponse(null)
479+
() -> {}
499480
);
500481
}
501482
}
@@ -505,12 +486,11 @@ private GetSnapshotsResponse buildResponse() {
505486
cancellableTask.ensureNotCancelled();
506487
int remaining = 0;
507488
final var resultsStream = allSnapshotInfos.stream()
508-
.flatMap(Collection::stream)
509489
.peek(this::assertSatisfiesAllPredicates)
510490
.sorted(sortBy.getSnapshotInfoComparator(order))
511491
.skip(offset);
512492
final List<SnapshotInfo> snapshotInfos;
513-
if (size == GetSnapshotsRequest.NO_LIMIT || resultsCount.get() <= size) {
493+
if (size == GetSnapshotsRequest.NO_LIMIT || allSnapshotInfos.size() <= size) {
514494
snapshotInfos = resultsStream.toList();
515495
} else {
516496
snapshotInfos = new ArrayList<>(size);

0 commit comments

Comments
 (0)