Fixes get snapshot duplicates when asking for _all (#21340)

Before, when requesting to get the snapshots in a repository, if `_all` was
specified for the set of snapshots to get, any in-progress snapshots would
be returned twice.  This commit fixes the issue ensuring that each snapshot,
whether in-progress or completed, is only returned once when making a call to
get snapshots (GET /_snapshot/{repository_name}/_all).

This commit also enables `_current` to appear anywhere in 
the get snapshots list, and will be used as a pseudo regex to
mean "match any currently running snapshots".

Closes #21335

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

@@ -39,7 +39,7 @@ import org.elasticsearch.transport.TransportService;
 
 import java.util.ArrayList;
 import java.util.HashMap;
-import java.util.LinkedHashSet;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -80,25 +80,26 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
         try {
             final String repository = request.repository();
             List<SnapshotInfo> snapshotInfoBuilder = new ArrayList<>();
-            if (isAllSnapshots(request.snapshots())) {
+            final Map<String, SnapshotId> allSnapshotIds = new HashMap<>();
-                snapshotInfoBuilder.addAll(snapshotsService.currentSnapshots(repository));
+            final List<SnapshotId> currentSnapshotIds = new ArrayList<>();
-                snapshotInfoBuilder.addAll(snapshotsService.snapshots(repository,
+            for (SnapshotInfo snapshotInfo : snapshotsService.currentSnapshots(repository)) {
-                                                                      snapshotsService.snapshotIds(repository),
+                SnapshotId snapshotId = snapshotInfo.snapshotId();
-                                                                      request.ignoreUnavailable()));
+                allSnapshotIds.put(snapshotId.getName(), snapshotId);
-            } else if (isCurrentSnapshots(request.snapshots())) {
+                currentSnapshotIds.add(snapshotId);
-                snapshotInfoBuilder.addAll(snapshotsService.currentSnapshots(repository));
+            }
-            } else {
+            if (isCurrentSnapshotsOnly(request.snapshots()) == false) {
-                final Map<String, SnapshotId> allSnapshotIds = new HashMap<>();
-                for (SnapshotInfo snapshotInfo : snapshotsService.currentSnapshots(repository)) {
-                    SnapshotId snapshotId = snapshotInfo.snapshotId();
-                    allSnapshotIds.put(snapshotId.getName(), snapshotId);
-                }
                 for (SnapshotId snapshotId : snapshotsService.snapshotIds(repository)) {
                     allSnapshotIds.put(snapshotId.getName(), snapshotId);
                 }
-                final Set<SnapshotId> toResolve = new LinkedHashSet<>(); // maintain order
+            }
+            final Set<SnapshotId> toResolve = new HashSet<>();
+            if (isAllSnapshots(request.snapshots())) {
+                toResolve.addAll(allSnapshotIds.values());
+            } else {
                 for (String snapshotOrPattern : request.snapshots()) {
-                    if (Regex.isSimpleMatchPattern(snapshotOrPattern) == false) {
+                    if (GetSnapshotsRequest.CURRENT_SNAPSHOT.equalsIgnoreCase(snapshotOrPattern)) {
+                        toResolve.addAll(currentSnapshotIds);
+                    } else if (Regex.isSimpleMatchPattern(snapshotOrPattern) == false) {
                         if (allSnapshotIds.containsKey(snapshotOrPattern)) {
                             toResolve.add(allSnapshotIds.get(snapshotOrPattern));
                         } else if (request.ignoreUnavailable() == false) {
@@ -113,12 +114,12 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
                     }
                 }
 
-                if (toResolve.isEmpty() && request.ignoreUnavailable() == false) {
+                if (toResolve.isEmpty() && request.ignoreUnavailable() == false && isCurrentSnapshotsOnly(request.snapshots()) == false) {
                     throw new SnapshotMissingException(repository, request.snapshots()[0]);
                 }
-
-                snapshotInfoBuilder.addAll(snapshotsService.snapshots(repository, new ArrayList<>(toResolve), request.ignoreUnavailable()));
             }
+
+            snapshotInfoBuilder.addAll(snapshotsService.snapshots(repository, new ArrayList<>(toResolve), request.ignoreUnavailable()));
             listener.onResponse(new GetSnapshotsResponse(snapshotInfoBuilder));
         } catch (Exception e) {
             listener.onFailure(e);
@@ -129,7 +130,7 @@ public class TransportGetSnapshotsAction extends TransportMasterNodeAction<GetSn
         return (snapshots.length == 0) || (snapshots.length == 1 && GetSnapshotsRequest.ALL_SNAPSHOTS.equalsIgnoreCase(snapshots[0]));
     }
 
-    private boolean isCurrentSnapshots(String[] snapshots) {
+    private boolean isCurrentSnapshotsOnly(String[] snapshots) {
         return (snapshots.length == 1 && GetSnapshotsRequest.CURRENT_SNAPSHOT.equalsIgnoreCase(snapshots[0]));
     }
 }

core/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java

@@ -34,7 +34,6 @@ import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotIndexStat
 import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotStatus;
 import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotsStatusResponse;
 import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse;
-import org.elasticsearch.action.admin.cluster.storedscripts.GetStoredScriptRequest;
 import org.elasticsearch.action.admin.cluster.storedscripts.GetStoredScriptResponse;
 import org.elasticsearch.action.admin.indices.flush.FlushResponse;
 import org.elasticsearch.action.admin.indices.settings.get.GetSettingsResponse;
@@ -58,6 +57,7 @@ import org.elasticsearch.cluster.metadata.MetaDataIndexStateService;
 import org.elasticsearch.cluster.routing.IndexRoutingTable;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.Priority;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.collect.ImmutableOpenMap;
@@ -77,7 +77,6 @@ import org.elasticsearch.repositories.RepositoriesService;
 import org.elasticsearch.repositories.RepositoryData;
 import org.elasticsearch.repositories.RepositoryException;
 import org.elasticsearch.script.MockScriptEngine;
-import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.script.StoredScriptsIT;
 import org.elasticsearch.snapshots.mockstore.MockRepository;
 import org.elasticsearch.test.junit.annotations.TestLogging;
@@ -101,7 +100,6 @@ import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF
 import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS;
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.elasticsearch.index.IndexSettings.INDEX_REFRESH_INTERVAL_SETTING;
-import static org.elasticsearch.index.query.QueryBuilders.boolQuery;
 import static org.elasticsearch.index.query.QueryBuilders.matchQuery;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAliasesExist;
@@ -2505,8 +2503,28 @@ public class SharedClusterSnapshotRestoreIT extends AbstractSnapshotIntegTestCas
         }
         refresh();
 
+        // make sure we return only the in-progress snapshot when taking the first snapshot on a clean repository
+        // take initial snapshot with a block, making sure we only get 1 in-progress snapshot returned
+        // block a node so the create snapshot operation can remain in progress
+        final String initialBlockedNode = blockNodeWithIndex(repositoryName, indexName);
+        ListenableActionFuture<CreateSnapshotResponse> responseListener =
+            client.admin().cluster().prepareCreateSnapshot(repositoryName, "snap-on-empty-repo")
+                .setWaitForCompletion(false)
+                .setIndices(indexName)
+                .execute();
+        waitForBlock(initialBlockedNode, repositoryName, TimeValue.timeValueSeconds(60)); // wait for block to kick in
+        getSnapshotsResponse = client.admin().cluster()
+                                   .prepareGetSnapshots("test-repo")
+                                   .setSnapshots(randomFrom("_all", "_current", "snap-on-*", "*-on-empty-repo", "snap-on-empty-repo"))
+                                   .get();
+        assertEquals(1, getSnapshotsResponse.getSnapshots().size());
+        assertEquals("snap-on-empty-repo", getSnapshotsResponse.getSnapshots().get(0).snapshotId().getName());
+        unblockNode(repositoryName, initialBlockedNode); // unblock node
+        responseListener.actionGet(TimeValue.timeValueMillis(10000L)); // timeout after 10 seconds
+        client.admin().cluster().prepareDeleteSnapshot(repositoryName, "snap-on-empty-repo").get();
+
         final int numSnapshots = randomIntBetween(1, 3) + 1;
-        logger.info("--> take {} snapshot(s)", numSnapshots);
+        logger.info("--> take {} snapshot(s)", numSnapshots - 1);
         final String[] snapshotNames = new String[numSnapshots];
         for (int i = 0; i < numSnapshots - 1; i++) {
             final String snapshotName = randomAsciiOfLength(8).toLowerCase(Locale.ROOT);
@@ -2538,9 +2556,19 @@ public class SharedClusterSnapshotRestoreIT extends AbstractSnapshotIntegTestCas
 
         logger.info("--> get all snapshots with a current in-progress");
         // with ignore unavailable set to true, should not throw an exception
+        final List<String> snapshotsToGet = new ArrayList<>();
+        if (randomBoolean()) {
+            // use _current plus the individual names of the finished snapshots
+            snapshotsToGet.add("_current");
+            for (int i = 0; i < numSnapshots - 1; i++) {
+                snapshotsToGet.add(snapshotNames[i]);
+            }
+        } else {
+            snapshotsToGet.add("_all");
+        }
         getSnapshotsResponse = client.admin().cluster()
                                              .prepareGetSnapshots(repositoryName)
-                                             .addSnapshots("_all")
+                                             .setSnapshots(snapshotsToGet.toArray(Strings.EMPTY_ARRAY))
                                              .get();
         List<String> sortedNames = Arrays.asList(snapshotNames);
         Collections.sort(sortedNames);