Fix Bug in Multiple Repo SLM Retention (#70047) (#70072)

We have to not try to create a delete run for repos that have nothing to delete. Also, added an assertion to make sure we don't try to create broken `GroupActionListener` which is always a bug. Adding the assertion allows testing this in an integration test easily. closes #70029
elastic · Mar 8, 2021 · af91d9d · af91d9d
1 parent 319b33d
commit af91d9d
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 1 deletion.
diff --git a/server/src/main/java/org/elasticsearch/action/support/GroupedActionListener.java b/server/src/main/java/org/elasticsearch/action/support/GroupedActionListener.java
@@ -37,6 +37,7 @@ public final class GroupedActionListener<T> implements ActionListener<T> {
      */
     public GroupedActionListener(ActionListener<Collection<T>> delegate, int groupSize) {
         if (groupSize <= 0) {
+            assert false : "illegal group size [" + groupSize + "]";
             throw new IllegalArgumentException("groupSize must be greater than 0 but was " + groupSize);
         }
         results = new AtomicArray<>(groupSize);

diff --git a/...c/internalClusterTest/java/org/elasticsearch/xpack/slm/SLMSnapshotBlockingIntegTests.java b/...c/internalClusterTest/java/org/elasticsearch/xpack/slm/SLMSnapshotBlockingIntegTests.java
@@ -290,6 +290,19 @@ public void testBasicPartialRetention() throws Exception {
         testUnsuccessfulSnapshotRetention(true);
     }
 
+    public void testRetentionWithMultipleRepositories() throws Exception {
+        disableRepoConsistencyCheck("test leaves behind an empty repository");
+        final String secondRepo = "other-repo";
+        createRepository(secondRepo, "fs");
+        final String policyId = "some-policy-id";
+        createSnapshotPolicy(policyId, "snap", NEVER_EXECUTE_CRON_SCHEDULE, secondRepo,
+                "*", true,
+                true, new SnapshotRetentionConfiguration(null, 1, 2));
+        logger.info("-->  start snapshot");
+        client().execute(ExecuteSnapshotLifecycleAction.INSTANCE, new ExecuteSnapshotLifecycleAction.Request(policyId)).get();
+        testUnsuccessfulSnapshotRetention(randomBoolean());
+    }
+
     private void testUnsuccessfulSnapshotRetention(boolean partialSuccess) throws Exception {
         final String indexName = "test-idx";
         final String policyId = "test-policy";

diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/slm/SnapshotRetentionTask.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/slm/SnapshotRetentionTask.java
@@ -316,7 +316,9 @@ void deleteSnapshots(Map<String, List<SnapshotInfo>> snapshotsToDelete,
         for (Map.Entry<String, List<SnapshotInfo>> entry : snapshotsToDelete.entrySet()) {
             String repo = entry.getKey();
             List<SnapshotInfo> snapshots = entry.getValue();
-            deleteSnapshots(slmStats, deleted, failed, repo, snapshots, allDeletesListener);
+            if (snapshots.isEmpty() == false) {
+                deleteSnapshots(slmStats, deleted, failed, repo, snapshots, allDeletesListener);
+            }
         }
     }