Reduce copying in GetSnapshotsOperation#snapshots()

[DaveCTurner]

No need to create a set (the values are distinct anyway), make a
separate synchronized list, populate them both, and finally copy them
both again into another concatenated list. We can just make the final
list up front.

[elasticsearchmachine]

Pinging @elastic/es-distributed (Team:Distributed)

[DaveCTurner]

Reducing the unnecessary copying here is clearly good in its own right, but this change is also motivated by wanting to take more control over the SnapshotInfo retrieval process rather than just handing a list of snapshot IDs off to repository.getSnapshotInfo(). In particular if we've already found size matching snapshots then (depending on the chosen sort key) we might be able to tell in advance that certain other snapshots won't make it to the final list, avoiding the need to retrieve their SnapshotInfo blobs at all. That's the main reason for introducing a RefCountingListener here - it's not necessary as written yet but it will be important when we're tracking the individual retrievals ourselves.

[DaveCTurner]

#105769 is what I mean by taking more control of the SnapshotInfo retrieval process. That PR branch includes the changes in this one so there's quite some extra noise.

[idegtiarenko]

No need to create a set (the values are distinct anyway)

snapshotIds is defined as collection in the method signature

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

Line 399 in b95cb8c

private void snapshots(String repositoryName, Collection<SnapshotId> snapshotIds, ActionListener<SnapshotsInRepo> listener) {

that is only called from

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

Line 378 in b95cb8c

snapshots(repo, toResolve.stream().map(Snapshot::getSnapshotId).toList(), listener);

where toResolve

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

Line 328 in b95cb8c

final Set<Snapshot> toResolve = new HashSet<>();

is defined as a HashSet. I think it is worth to update the method signature to accept Set<SnapshotId> snapshotIds to better reflect that there are no duplicates expected.

[DaveCTurner]

I know what you mean, but (a) nothing particularly bad happens if there are duplicates here, and (b) we'd have to change that .toList() into a .collect(Collectors.toSet()) which is quite a bit more expensive. This whole thing will go away eventually anyway so I'd rather leave it as-is for now.