Add support for partial searchable snapshots to ILM #68714

dakrone · 2021-02-08T22:24:28Z

This commit adds support for the recently introduced partial searchable snapshot (#68509) to ILM.

Searchable snapshot ILM actions may now be specified with a storage option, specifying either
full_copy or shared_cache (similar to the "mount" API) to mount either a full or partial
searchable snapshot:

PUT _ilm/policy/my_policy
{
  "policy": {
    "phases": {
      "cold": {
        "actions": {
          "searchable_snapshot" : {
            "snapshot_repository" : "backing_repo",
            "storage": "shared_cache"
          }
        }
      }
    }
  }
}

Internally, If more than one searchable snapshot action is specified (for example, a full searchable
snapshot in the "cold" phase and a partial searchable snapshot in the "frozen" phase) ILM will
re-use the existing snapshot when doing the second mount since a second snapshot is not required.

Currently this is allowed for actions that use the same repository, however, multiple
searchable_snapshot actions for the same index that use different repositories is not allowed (the
ERROR state is entered). We plan to allow this in the future in subsequent work.

If the storage option is not specified in the searchable_snapshot action, the mount type
defaults to "shared_cache" in the frozen phase and "full_copy" in all other phases.

Relates to #68605

This commit adds support for the recently introduced partial searchable snapshot (elastic#68509) to ILM. Searchable snapshot ILM actions may now be specified with a `storage` option, specifying either `full_copy` or `shared_cache` (similar to the "mount" API) to mount either a full or partial searchable snapshot: ```json PUT _ilm/policy/my_policy { "policy": { "phases": { "cold": { "actions": { "searchable_snapshot" : { "snapshot_repository" : "backing_repo", "storage": "shared_cache" } } } } } } ``` Internally, If more than one searchable snapshot action is specified (for example, a full searchable snapshot in the "cold" phase and a partial searchable snapshot in the "frozen" phase) ILM will re-use the existing snapshot when doing the second mount since a second snapshot is not required. Currently this is allowed for actions that use the same repository, however, multiple `searchable_snapshot` actions for the same index that use different repositories is not allowed (the ERROR state is entered). We plan to allow this in the future in subsequent work. If the `storage` option is not specified in the `searchable_snapshot` action, the mount type defaults to "shared_cache" in the frozen phase and "full_copy" in all other phases. Relates to elastic#68605

elasticmachine · 2021-02-08T22:24:32Z

Pinging @elastic/es-core-features (Team:Core/Features)

andreidan

This looks great @dakrone! Thanks for adding support for the partial searchable snapshots, this is an awesome feature.

I left only a few minor/cosmetic comments.

andreidan · 2021-02-09T10:41:42Z

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MountSnapshotStep.java

+            // 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, " +


should we log on warn here?

I don't think so, since this will be a pretty common occurrence for anyone upgrading, and it's not anything a user needs to be concerned about (they can't change it anyway)

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SearchableSnapshotAction.java

andreidan · 2021-02-09T11:17:45Z

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SearchableSnapshotAction.java

+    /**
+     * Resolves the prefix to be used for the mounted index depending on the provided key
+     */
+    String getRestoredIndexPrefix(StepKey currentKey) {


this could be static

These make use of the storageType instance variable, while we could passed it in, I think that would make the code unnecessarily verbose where these methods are called. I think I'd prefer to keep it as non-static if that's okay with you.

andreidan · 2021-02-09T11:17:52Z

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SearchableSnapshotAction.java

+    }
+
+    // Resolves the storage type from a Nullable to non-Nullable type
+    MountSearchableSnapshotRequest.Storage getConcreteStorageType(StepKey currentKey) {


this could be static

These make use of the storageType instance variable, while we could passed it in, I think that would make the code unnecessarily verbose where these methods are called. I think I'd prefer to keep it as non-static if that's okay with you.

andreidan · 2021-02-09T11:19:26Z

...in/java/org/elasticsearch/xpack/core/searchablesnapshots/MountSearchableSnapshotRequest.java

@@ -234,6 +234,22 @@ public String toString() {
        FULL_COPY,
        SHARED_CACHE;

+        public static Storage fromString(String type) {
+            if ("full_copy".equals(type)) {


should we equalsIgnoreCase in these branches? Or is case important?

It's not really that important, but since equalsIgnoreCase doesn't take a locale so we can't pass it in, I figure it's safer to keep it as-is and be strict (we can always loosen the restrictions if we decide we need to at a later time)

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/MountSnapshotStepTests.java

andreidan · 2021-02-09T11:34:22Z

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SearchableSnapshotAction.java

+                if (this.snapshotRepository.equals(lifecycleExecutionState.getSnapshotRepository()) == 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");


shall we prevent this at policy validation time for now?

Not yet I think, I'm going to try to get it done this week, and if I don't have time, I'll add the policy validation at the end of the week (trying to avoid adding the policy validation and then immediately removing it)

dakrone · 2021-02-09T15:18:38Z

@elasticmachine update branch

This commit adds support for the recently introduced partial searchable snapshot (elastic#68509) to ILM. Searchable snapshot ILM actions may now be specified with a `storage` option, specifying either `full_copy` or `shared_cache` (similar to the "mount" API) to mount either a full or partial searchable snapshot: ```json PUT _ilm/policy/my_policy { "policy": { "phases": { "cold": { "actions": { "searchable_snapshot" : { "snapshot_repository" : "backing_repo", "storage": "shared_cache" } } } } } } ``` Internally, If more than one searchable snapshot action is specified (for example, a full searchable snapshot in the "cold" phase and a partial searchable snapshot in the "frozen" phase) ILM will re-use the existing snapshot when doing the second mount since a second snapshot is not required. Currently this is allowed for actions that use the same repository, however, multiple `searchable_snapshot` actions for the same index that use different repositories is not allowed (the ERROR state is entered). We plan to allow this in the future in subsequent work. If the `storage` option is not specified in the `searchable_snapshot` action, the mount type defaults to "shared_cache" in the frozen phase and "full_copy" in all other phases. Relates to elastic#68605

Relates to backporting elastic#68714

…68764) Relates to backporting #68714

…68762) This commit adds support for the recently introduced partial searchable snapshot (#68509) to ILM. Searchable snapshot ILM actions may now be specified with a `storage` option, specifying either `full_copy` or `shared_cache` (similar to the "mount" API) to mount either a full or partial searchable snapshot: `json PUT _ilm/policy/my_policy { "policy": { "phases": { "cold": { "actions": { "searchable_snapshot" : { "snapshot_repository" : "backing_repo", "storage": "shared_cache" } } } } } } ` Internally, If more than one searchable snapshot action is specified (for example, a full searchable snapshot in the "cold" phase and a partial searchable snapshot in the "frozen" phase) ILM will re-use the existing snapshot when doing the second mount since a second snapshot is not required. Currently this is allowed for actions that use the same repository, however, multiple `searchable_snapshot` actions for the same index that use different repositories is not allowed (the ERROR state is entered). We plan to allow this in the future in subsequent work. If the `storage` option is not specified in the `searchable_snapshot` action, the mount type defaults to "shared_cache" in the frozen phase and "full_copy" in all other phases. Relates to #68605

This commit adds validation when updating or creating an ILM policy that mulitple searchable snapshot actions all use the same repository. We currently do not have the infrastructure to support switching a searchable snapshot from one repository to another, so until we have that we will disallow this (edge case) when creating a policy. Relates to elastic#68714

…ies (#68856) This commit adds validation when updating or creating an ILM policy that mulitple searchable snapshot actions all use the same repository. We currently do not have the infrastructure to support switching a searchable snapshot from one repository to another, so until we have that we will disallow this (edge case) when creating a policy. Relates to #68714

…ies (elastic#68856) This commit adds validation when updating or creating an ILM policy that mulitple searchable snapshot actions all use the same repository. We currently do not have the infrastructure to support switching a searchable snapshot from one repository to another, so until we have that we will disallow this (edge case) when creating a policy. Relates to elastic#68714

…ositories (#68856) (#68866) This commit adds validation when updating or creating an ILM policy that mulitple searchable snapshot actions all use the same repository. We currently do not have the infrastructure to support switching a searchable snapshot from one repository to another, so until we have that we will disallow this (edge case) when creating a policy. Relates to #68714

dakrone added >feature :Data Management/ILM+SLM Index and Snapshot lifecycle management v8.0.0 v7.12.0 labels Feb 8, 2021

dakrone requested review from andreidan and jasontedor February 8, 2021 22:24

elasticmachine added the Team:Data Management Meta label for data/management team label Feb 8, 2021

Fix switch statement in test that needed to be expanded

be87f43

andreidan approved these changes Feb 9, 2021

View reviewed changes

dakrone added 4 commits February 9, 2021 07:50

Adjust log message to clarify only the storage type differs

b903deb

Remove unneeded 'null' check since BranchingStep already has one

7d76251

Remove @nullable from MountSnapshotStepTests (it's not nullable)

1a42b4e

Add more potato tests

4455669

Merge branch 'master' into ilm-support-partial-searchable-snapshot

a304746

dakrone merged commit e552fd7 into elastic:master Feb 9, 2021

dakrone deleted the ilm-support-partial-searchable-snapshot branch February 9, 2021 16:14

dakrone mentioned this pull request Feb 9, 2021

[7.x] Add support for partial searchable snapshots to ILM (#68714) #68762

Merged

dakrone added a commit to dakrone/elasticsearch that referenced this pull request Feb 9, 2021

Disable BWC for backporting ILM partial searchable snapshot support

bcbac99

Relates to backporting elastic#68714

dakrone mentioned this pull request Feb 9, 2021

Disable BWC for backporting ILM partial searchable snapshot support #68764

Merged

dakrone added a commit that referenced this pull request Feb 9, 2021

Disable BWC for backporting ILM partial searchable snapshot support (#…

20cab22

…68764) Relates to backporting #68714

dakrone mentioned this pull request Feb 10, 2021

Disallow ILM searchable snapshot actions that use different repositories #68856

Merged

cjcenizal mentioned this pull request Feb 11, 2021

Add frozen phase to ILM UI elastic/kibana#91189

Closed

jakelandis added the release highlight label Feb 19, 2021

stevejgordon mentioned this pull request Feb 22, 2021

7.12.0 Meta Ticket elastic/elasticsearch-net#5337

Closed

34 tasks

andreidan mentioned this pull request Mar 1, 2021

[ILM] Allow multiple searchable snapshot actions elastic/kibana#92789

Merged

jrodewig mentioned this pull request Mar 23, 2021

[DOCS] Add 7.12 release highlights #70703

Merged

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for partial searchable snapshots to ILM #68714

Add support for partial searchable snapshots to ILM #68714

dakrone commented Feb 8, 2021

elasticmachine commented Feb 8, 2021

andreidan left a comment

andreidan Feb 9, 2021

dakrone Feb 9, 2021

andreidan Feb 9, 2021

dakrone Feb 9, 2021

andreidan Feb 9, 2021

dakrone Feb 9, 2021

andreidan Feb 9, 2021

dakrone Feb 9, 2021

andreidan Feb 9, 2021

dakrone Feb 9, 2021

dakrone commented Feb 9, 2021

Add support for partial searchable snapshots to ILM #68714

Add support for partial searchable snapshots to ILM #68714

Conversation

dakrone commented Feb 8, 2021

elasticmachine commented Feb 8, 2021

andreidan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dakrone commented Feb 9, 2021