[7.17] [ILM] More resilient when a policy is added to searchable snapshot (#102741) (#103070)

gmarouli · web-flow · commit 253f1f430f09 · 2023-12-06T20:08:15.000+02:00
* Backport #102741
diff --git a/docs/changelog/102741.yaml b/docs/changelog/102741.yaml
@@ -0,0 +1,6 @@
+pr: 102741
+summary: "[ILM] More resilient when a policy is added to searchable snapshot"
+area: ILM+SLM
+type: bug
+issues:
+ - 101958
diff --git a/docs/reference/ilm/actions/ilm-delete.asciidoc b/docs/reference/ilm/actions/ilm-delete.asciidoc
@@ -16,6 +16,11 @@ Defaults to `true`.
 This option is applicable when the <<ilm-searchable-snapshot,searchable
 snapshot>> action is used in any previous phase.
 
+WARNING: If a policy with a searchable snapshot action is applied on an existing searchable snapshot index,
+the snapshot backing this index will NOT be deleted because it was not created by this policy. If you want
+to clean this snapshot, please delete it manually after the index is deleted using the <<delete-snapshot-api, delete snapshot API>>, you
+can find the repository and snapshot name using the <<indices-get-index, get index API>>.
+
 [[ilm-delete-action-ex]]
 ==== Example
 
diff --git a/server/src/main/java/org/elasticsearch/snapshots/SearchableSnapshotsSettings.java b/server/src/main/java/org/elasticsearch/snapshots/SearchableSnapshotsSettings.java
@@ -29,6 +29,7 @@ public final class SearchableSnapshotsSettings {
     );
     public static final String SEARCHABLE_SNAPSHOTS_REPOSITORY_NAME_SETTING_KEY = "index.store.snapshot.repository_name";
     public static final String SEARCHABLE_SNAPSHOTS_REPOSITORY_UUID_SETTING_KEY = "index.store.snapshot.repository_uuid";
+    public static final String SEARCHABLE_SNAPSHOTS_SNAPSHOT_NAME_SETTING_KEY = "index.store.snapshot.snapshot_name";
 
     private SearchableSnapshotsSettings() {}
 
diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MountSnapshotStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MountSnapshotStep.java
@@ -70,22 +70,30 @@ void performDuringNoSnapshot(IndexMetadata indexMetadata, ClusterState currentCl
         LifecycleExecutionState lifecycleState = fromIndexMetadata(indexMetadata);
 
         String policyName = indexMetadata.getSettings().get(LifecycleSettings.LIFECYCLE_NAME);
-        final String snapshotRepository = lifecycleState.getSnapshotRepository();
+        String snapshotRepository = lifecycleState.getSnapshotRepository();
+        SearchableSnapshotAction.SearchableSnapshotMetadata searchableSnapshotMetadata = SearchableSnapshotAction
+            .extractSearchableSnapshotFromSettings(indexMetadata);
         if (Strings.hasText(snapshotRepository) == false) {
-            listener.onFailure(
-                new IllegalStateException(
-                    "snapshot repository is not present for policy [" + policyName + "] and index [" + indexName + "]"
-                )
-            );
-            return;
+            if (searchableSnapshotMetadata == null) {
+                listener.onFailure(
+                    new IllegalStateException(
+                        "snapshot repository is not present for policy [" + policyName + "] and index [" + indexName + "]"
+                    )
+                );
+                return;
+            } else {
+                snapshotRepository = searchableSnapshotMetadata.repositoryName();
+            }
         }
 
-        final String snapshotName = lifecycleState.getSnapshotName();
-        if (Strings.hasText(snapshotName) == false) {
+        String snapshotName = lifecycleState.getSnapshotName();
+        if (Strings.hasText(snapshotName) == false && searchableSnapshotMetadata == null) {
             listener.onFailure(
                 new IllegalStateException("snapshot name was not generated for policy [" + policyName + "] and index [" + indexName + "]")
             );
             return;
+        } else if (searchableSnapshotMetadata != null) {
+            snapshotName = searchableSnapshotMetadata.snapshotName();
         }
 
         String mountedIndexName = restoredIndexPrefix + indexName;
@@ -102,16 +110,20 @@ void performDuringNoSnapshot(IndexMetadata indexMetadata, ClusterState currentCl
 
         final String snapshotIndexName = lifecycleState.getSnapshotIndexName();
         if (snapshotIndexName == null) {
-            // This index had its searchable snapshot created prior to a version where we captured
-            // the original index name, so make our best guess at the name
-            indexName = bestEffortIndexNameResolution(indexName);
-            logger.debug(
-                "index [{}] using policy [{}] does not have a stored snapshot index name, "
-                    + "using our best effort guess of [{}] for the original snapshotted index name",
-                indexMetadata.getIndex().getName(),
-                policyName,
-                indexName
-            );
+            if (searchableSnapshotMetadata == null) {
+                // This index had its searchable snapshot created prior to a version where we captured
+                // the original index name, so make our best guess at the name
+                indexName = bestEffortIndexNameResolution(indexName);
+                logger.debug(
+                    "index [{}] using policy [{}] does not have a stored snapshot index name, "
+                        + "using our best effort guess of [{}] for the original snapshotted index name",
+                    indexMetadata.getIndex().getName(),
+                    policyName,
+                    indexName
+                );
+            } else {
+                indexName = searchableSnapshotMetadata.sourceIndex();
+            }
         } else {
             // Use the name of the snapshot as specified in the metadata, because the current index
             // name not might not reflect the name of the index actually in the snapshot
diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SearchableSnapshotAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SearchableSnapshotAction.java
@@ -16,6 +16,7 @@
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.core.Nullable;
 import org.elasticsearch.license.LicenseUtils;
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.xcontent.ConstructingObjectParser;
@@ -31,6 +32,7 @@
 import java.util.Objects;
 
 import static org.elasticsearch.snapshots.SearchableSnapshotsSettings.SEARCHABLE_SNAPSHOTS_REPOSITORY_NAME_SETTING_KEY;
+import static org.elasticsearch.snapshots.SearchableSnapshotsSettings.SEARCHABLE_SNAPSHOTS_SNAPSHOT_NAME_SETTING_KEY;
 import static org.elasticsearch.snapshots.SearchableSnapshotsSettings.SEARCHABLE_SNAPSHOT_PARTIAL_SETTING_KEY;
 import static org.elasticsearch.xpack.core.searchablesnapshots.SearchableSnapshotsConstants.SEARCHABLE_SNAPSHOT_FEATURE;
 
@@ -143,10 +145,12 @@ public List<Step> toSteps(Client client, String phase, StepKey nextStepKey, XPac
                 IndexMetadata indexMetadata = clusterState.getMetadata().index(index);
                 assert indexMetadata != null : "index " + index.getName() + " must exist in the cluster state";
                 String policyName = LifecycleSettings.LIFECYCLE_NAME_SETTING.get(indexMetadata.getSettings());
-                if (indexMetadata.getSettings().get(LifecycleSettings.SNAPSHOT_INDEX_NAME) != null) {
+                SearchableSnapshotMetadata searchableSnapshotMetadata = extractSearchableSnapshotFromSettings(indexMetadata);
+                if (searchableSnapshotMetadata != null) {
+                    // TODO: allow this behavior instead of returning false, in this case the index is already a searchable a snapshot
+                    // so the most graceful way of recovery might be to use this repo
                     // The index is already a searchable snapshot, let's see if the repository matches
-                    String repo = indexMetadata.getSettings().get(SEARCHABLE_SNAPSHOTS_REPOSITORY_NAME_SETTING_KEY);
-                    if (this.snapshotRepository.equals(repo) == false) {
+                    if (this.snapshotRepository.equals(searchableSnapshotMetadata.repositoryName) == false) {
                         // Okay, different repo, we need to go ahead with the searchable snapshot
                         logger.debug(
                             "[{}] action is configured for index [{}] in policy [{}] which is already mounted as a searchable "
@@ -155,15 +159,14 @@ public List<Step> toSteps(Client client, String phase, StepKey nextStepKey, XPac
                             SearchableSnapshotAction.NAME,
                             index.getName(),
                             policyName,
-                            repo,
+                            searchableSnapshotMetadata.repositoryName,
                             this.snapshotRepository
                         );
                         return false;
                     }
 
                     // Check to the storage type to see if we need to convert between full <-> partial
-                    final boolean partial = indexMetadata.getSettings().getAsBoolean(SEARCHABLE_SNAPSHOT_PARTIAL_SETTING_KEY, false);
-                    MountSearchableSnapshotRequest.Storage existingType = partial
+                    MountSearchableSnapshotRequest.Storage existingType = searchableSnapshotMetadata.partial
                         ? MountSearchableSnapshotRequest.Storage.SHARED_CACHE
                         : MountSearchableSnapshotRequest.Storage.FULL_COPY;
                     MountSearchableSnapshotRequest.Storage type = getConcreteStorageType(preActionBranchingKey);
@@ -174,7 +177,7 @@ public List<Step> toSteps(Client client, String phase, StepKey nextStepKey, XPac
                             SearchableSnapshotAction.NAME,
                             index.getName(),
                             policyName,
-                            repo,
+                            searchableSnapshotMetadata.repositoryName,
                             type
                         );
                         return true;
@@ -211,7 +214,7 @@ public List<Step> toSteps(Client client, String phase, StepKey nextStepKey, XPac
         // When generating a snapshot, we either jump to the force merge step, or we skip the
         // forcemerge and go straight to steps for creating the snapshot
         StepKey keyForSnapshotGeneration = forceMergeIndex ? forceMergeStepKey : generateSnapshotNameKey;
-        // Branch, deciding whether there is an existing searchable snapshot snapshot that can be used for mounting the index
+        // Branch, deciding whether there is an existing searchable snapshot that can be used for mounting the index
         // (in which case, skip generating a new name and the snapshot cleanup), or if we need to generate a new snapshot
         BranchingStep skipGeneratingSnapshotStep = new BranchingStep(
             skipGeneratingSnapshotKey,
@@ -221,7 +224,8 @@ public List<Step> toSteps(Client client, String phase, StepKey nextStepKey, XPac
                 IndexMetadata indexMetadata = clusterState.getMetadata().index(index);
                 String policyName = LifecycleSettings.LIFECYCLE_NAME_SETTING.get(indexMetadata.getSettings());
                 LifecycleExecutionState lifecycleExecutionState = LifecycleExecutionState.fromIndexMetadata(indexMetadata);
-                if (lifecycleExecutionState.getSnapshotName() == null) {
+                SearchableSnapshotMetadata searchableSnapshotMetadata = extractSearchableSnapshotFromSettings(indexMetadata);
+                if (lifecycleExecutionState.getSnapshotName() == null && searchableSnapshotMetadata == null) {
                     // No name exists, so it must be generated
                     logger.trace(
                         "no snapshot name for index [{}] in policy [{}] exists, so one will be generated",
@@ -230,8 +234,20 @@ public List<Step> toSteps(Client client, String phase, StepKey nextStepKey, XPac
                     );
                     return false;
                 }
+                String snapshotIndexName;
+                String snapshotName;
+                String repoName;
+                if (lifecycleExecutionState.getSnapshotName() != null) {
+                    snapshotIndexName = lifecycleExecutionState.getSnapshotIndexName();
+                    snapshotName = lifecycleExecutionState.getSnapshotName();
+                    repoName = lifecycleExecutionState.getSnapshotRepository();
+                } else {
+                    snapshotIndexName = searchableSnapshotMetadata.sourceIndex;
+                    snapshotName = searchableSnapshotMetadata.snapshotName;
+                    repoName = searchableSnapshotMetadata.repositoryName;
+                }
 
-                if (this.snapshotRepository.equals(lifecycleExecutionState.getSnapshotRepository()) == false) {
+                if (this.snapshotRepository.equals(repoName) == false) {
                     // A different repository is being used
                     // TODO: allow this behavior instead of throwing an exception
                     throw new IllegalArgumentException("searchable snapshot indices may be converted only within the same repository");
@@ -240,12 +256,14 @@ public List<Step> toSteps(Client client, String phase, StepKey nextStepKey, XPac
                 // We can skip the generate, initial cleanup, and snapshot taking for this index, as we already have a generated snapshot.
                 // This will jump ahead directly to the "mount snapshot" step
                 logger.debug(
-                    "an existing snapshot [{}] in repository [{}] (index name: [{}]) "
-                        + "will be used for mounting [{}] as a searchable snapshot",
-                    lifecycleExecutionState.getSnapshotName(),
-                    lifecycleExecutionState.getSnapshotRepository(),
-                    lifecycleExecutionState.getSnapshotIndexName(),
-                    index.getName()
+                    "Policy [{}] will use an existing snapshot [{}] in repository [{}] (index name: [{}]) "
+                        + "to mount [{}] as a searchable snapshot. This snapshot was found in the {}.",
+                    policyName,
+                    snapshotName,
+                    snapshotRepository,
+                    snapshotIndexName,
+                    index.getName(),
+                    lifecycleExecutionState.getSnapshotName() != null ? "lifecycle execution state" : "metadata of " + index.getName()
                 );
                 return true;
             }
@@ -401,11 +419,53 @@ public boolean equals(Object o) {
             return false;
         }
         SearchableSnapshotAction that = (SearchableSnapshotAction) o;
-        return Objects.equals(snapshotRepository, that.snapshotRepository);
+        return Objects.equals(snapshotRepository, that.snapshotRepository) && Objects.equals(forceMergeIndex, that.forceMergeIndex);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(snapshotRepository);
+        return Objects.hash(snapshotRepository, forceMergeIndex);
+    }
+
+    @Nullable
+    static SearchableSnapshotMetadata extractSearchableSnapshotFromSettings(IndexMetadata indexMetadata) {
+        String indexName = indexMetadata.getSettings().get(LifecycleSettings.SNAPSHOT_INDEX_NAME);
+        if (indexName == null) {
+            return null;
+        }
+        String snapshotName = indexMetadata.getSettings().get(SEARCHABLE_SNAPSHOTS_SNAPSHOT_NAME_SETTING_KEY);
+        String repo = indexMetadata.getSettings().get(SEARCHABLE_SNAPSHOTS_REPOSITORY_NAME_SETTING_KEY);
+        final boolean partial = indexMetadata.getSettings().getAsBoolean(SEARCHABLE_SNAPSHOT_PARTIAL_SETTING_KEY, false);
+        return new SearchableSnapshotMetadata(indexName, repo, snapshotName, partial);
     }
+
+    static class SearchableSnapshotMetadata {
+        private final String sourceIndex;
+        private final String repositoryName;
+        private final String snapshotName;
+        private final boolean partial;
+
+        SearchableSnapshotMetadata(String sourceIndex, String repositoryName, String snapshotName, boolean partial) {
+            this.sourceIndex = sourceIndex;
+            this.repositoryName = repositoryName;
+            this.snapshotName = snapshotName;
+            this.partial = partial;
+        }
+
+        public String sourceIndex() {
+            return sourceIndex;
+        }
+
+        public String repositoryName() {
+            return repositoryName;
+        }
+
+        public String snapshotName() {
+            return snapshotName;
+        }
+
+        public boolean partial() {
+            return partial;
+        }
+    };
 }
diff --git a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java
diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java
