From 47d0a8ac4ea1ef32b9c8a82ae163cc79d91a4856 Mon Sep 17 00:00:00 2001
From: Armin Braun <me@obrown.io>
Date: Wed, 8 Sep 2021 14:09:30 +0200
Subject: [PATCH 1/2] Make use of new Get Snapshots Filtering in SLM Retention

We can save ourselves the trouble of even looking at snapshots that don't have a relevant
policy on the SLM side now.
---
 .../org/elasticsearch/xpack/slm/SnapshotRetentionTask.java  | 6 ++++--
 .../elasticsearch/xpack/slm/SnapshotRetentionTaskTests.java | 3 ++-
 2 files changed, 6 insertions(+), 3 deletions(-)

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
index 1e2d22846fe6c..902a365e1ef02 100644
--- 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
@@ -137,7 +137,7 @@ public void triggered(SchedulerEngine.Event event) {
             // Finally, asynchronously retrieve all the snapshots, deleting them serially,
             // before updating the cluster state with the new metrics and setting 'running'
             // back to false
-            getAllRetainableSnapshots(repositioriesToFetch, new ActionListener<>() {
+            getAllRetainableSnapshots(repositioriesToFetch, policiesWithRetention.keySet(), new ActionListener<>() {
                 @Override
                 public void onResponse(Map<String, List<SnapshotInfo>> allSnapshots) {
                     if (logger.isTraceEnabled()) {
@@ -232,7 +232,8 @@ static boolean snapshotEligibleForDeletion(SnapshotInfo snapshot, Map<String, Li
         return eligible;
     }
 
-    void getAllRetainableSnapshots(Collection<String> repositories, ActionListener<Map<String, List<SnapshotInfo>>> listener,
+    void getAllRetainableSnapshots(Collection<String> repositories, Set<String> policies,
+                                   ActionListener<Map<String, List<SnapshotInfo>>> listener,
                                    Consumer<Exception> errorHandler) {
         if (repositories.isEmpty()) {
             // Skip retrieving anything if there are no repositories to fetch
@@ -245,6 +246,7 @@ void getAllRetainableSnapshots(Collection<String> repositories, ActionListener<M
             // don't time out on this request to not produce failed SLM runs in case of a temporarily slow master node
             .setMasterNodeTimeout(TimeValue.MAX_VALUE)
             .setIgnoreUnavailable(true)
+            .setPolicies(policies.toArray(Strings.EMPTY_ARRAY))
             .execute(ActionListener.wrap(resp -> {
                     if (logger.isTraceEnabled()) {
                         logger.trace("retrieved snapshots: {}",
diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionTaskTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionTaskTests.java
index d12139ea14d21..accaf9843bb6d 100644
--- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionTaskTests.java
+++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionTaskTests.java
@@ -335,7 +335,7 @@ void doExecute(ActionType<Response> action, Request request, ActionListener<Resp
                     (historyItem) -> fail("should never write history")));
 
             AtomicReference<Exception> errHandlerCalled = new AtomicReference<>(null);
-            task.getAllRetainableSnapshots(Collections.singleton(repoId), new ActionListener<>() {
+            task.getAllRetainableSnapshots(Collections.singleton(repoId), Set.of(policyId), new ActionListener<>() {
                 @Override
                 public void onResponse(Map<String, List<SnapshotInfo>> stringListMap) {
                     logger.info("--> forcing failure");
@@ -528,6 +528,7 @@ private static class MockSnapshotRetentionTask extends SnapshotRetentionTask {
 
         @Override
         void getAllRetainableSnapshots(Collection<String> repositories,
+                                       Set<String> policies,
                                        ActionListener<Map<String, List<SnapshotInfo>>> listener,
                                        Consumer<Exception> errorHandler) {
             listener.onResponse(this.snapshotRetriever.get());

From c028deb5c5fb26df501d1c06e591bec2fc765322 Mon Sep 17 00:00:00 2001
From: Armin Braun <me@obrown.io>
Date: Wed, 8 Sep 2021 16:24:59 +0200
Subject: [PATCH 2/2] redundant check

---
 .../org/elasticsearch/xpack/slm/SnapshotRetentionTask.java   | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

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
index d0cbcfe4576f3..6a685e42a64ab 100644
--- 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
@@ -253,10 +253,7 @@ void getAllRetainableSnapshots(Collection<String> repositories,
                     Map<String, List<SnapshotInfo>> snapshots = new HashMap<>();
                     for (SnapshotInfo info : resp.getSnapshots()) {
                         if (RETAINABLE_STATES.contains(info.state()) && info.userMetadata() != null) {
-                            final Object policy = info.userMetadata().get(POLICY_ID_METADATA_FIELD);
-                            if (policy instanceof String && policies.contains(policy)) {
-                                snapshots.computeIfAbsent(info.repository(), repo -> new ArrayList<>()).add(info);
-                            }
+                            snapshots.computeIfAbsent(info.repository(), repo -> new ArrayList<>()).add(info);
                         }
                     }
                     listener.onResponse(snapshots);