fix test by retaining old generations

Author: ywangd

server/src/main/java/org/elasticsearch/repositories/FinalizeSnapshotContext.java

@@ -19,8 +19,10 @@
 import org.elasticsearch.snapshots.SnapshotInfo;
 import org.elasticsearch.snapshots.SnapshotsService;
 
+import java.util.Collection;
 import java.util.Map;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 /**
  * Context for finalizing a snapshot.
@@ -106,8 +108,21 @@ public ClusterState updatedClusterState(ClusterState state) {
         final ClusterState updatedState = SnapshotsService.stateWithoutSnapshot(state, snapshotInfo.snapshot(), updatedShardGenerations);
         // Now that the updated cluster state may have changed in-progress shard snapshots' shard generations to the latest shard
         // generation, let's mark any now unreferenced shard generations as obsolete and ready to be deleted.
+
+        final Collection<IndexId> deletedIndices = updatedShardGenerations.deletedIndices.indices();
         obsoleteGenerations.set(
-            SnapshotsInProgress.get(updatedState).obsoleteGenerations(snapshotInfo.repository(), SnapshotsInProgress.get(state))
+            SnapshotsInProgress.get(updatedState)
+                .obsoleteGenerations(snapshotInfo.repository(), SnapshotsInProgress.get(state))
+                .entrySet()
+                .stream()
+                // We want to keep both old and new generations for deleted indices, so we filter them out here to avoid deletion.
+                // We need the old generations because they are what get recorded in the RepositoryData.
+                // We also need the new generations a future finalization may build upon them. It may ends up not being used at all
+                // when current batch of in-progress snapshots are completed and no new index of the same name is created.
+                // That is also OK. It means we have some redundant shard generations in the repository and they will be deleted
+                // when a snapshot deletion runs.
+                .filter(e -> deletedIndices.contains(e.getKey().index()) == false)
+                .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue))
         );
         return updatedState;
     }

server/src/test/java/org/elasticsearch/snapshots/SnapshotResiliencyTests.java

@@ -1629,6 +1629,10 @@ public void testDeleteIndexBetweenSuccessAndFinalization() {
                         }
                         l.onResponse(null);
                     }));
+            })
+            .andThen(l -> {
+                // delete the snapshot that has the deleted index so it triggers cleanup of redundant shard generations
+                client.admin().cluster().prepareDeleteSnapshot(TEST_REQUEST_TIMEOUT, repoName, "first-snapshot").execute(l.map(r -> null));
             });
 
         deterministicTaskQueue.runAllRunnableTasks();