Restrict ILM frozen phase to searchable snapshot actions only by dakrone · Pull Request #70158 · elastic/elasticsearch

dakrone · 2021-03-09T15:45:22Z

This commit changes the frozen phase within ILM in the following ways:

The searchable_snapshot action now no longer takes a storage parameter. The storage type is
determined by the phase within which it is invoked (shared cache for frozen and full copy for
everything else).
The frozen phase in ILM now no longer allows any actions other than searchable_snapshot
If a frozen phase is provided, it must include a searchable_snapshot action.

These changes may seem breaking, but since they are intended to go back to 7.12 which has not been
released yet, they are not truly breaking changes.

This commit changes the frozen phase within ILM in the following ways: - The `searchable_snapshot` action now no longer takes a `storage` parameter. The storage type is determined by the phase within which it is invoked (shared cache for frozen and full copy for everything else). - The frozen phase in ILM now no longer allows *any* actions other than `searchable_snapshot` - If a frozen phase is provided, it *must* include a `searchable_snapshot` action. These changes may seem breaking, but since they are intended to go back to 7.12 which has not been released yet, they are not truly breaking changes.

elasticmachine · 2021-03-09T15:45:26Z

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

dakrone · 2021-03-09T15:47:31Z

build.gradle

+boolean bwc_tests_enabled = false
+String bwc_tests_disabled_issue = "https://github.com/elastic/elasticsearch/pull/70158" /* place a PR link here when committing bwc changes */


This is required because I am removing the serialization for 7.12 in a weird way (we don't usually remove things like this), once this has been backported all the way back to 7.12 I will re-enable BWC

andreidan

LGTM, thanks for implementing this Lee

andreidan · 2021-03-09T17:11:43Z

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

            }
            phases.put(phase, new Phase(phase, after, actions));
        }
+        // Add a frozen phase if neither the hot nor cold phase contains a searchable snapshot action


What is the connection between the hot/cold actions and the frozen phase? We're still able to have searchable_snapshot actions in all phases as far as I understand

It's mostly because we'd have to use a consistent repository name (or else validation would fail), we have enough validation that this gets more and more complex, and this doesn't have to generate all possible valid lifecycles, just a random one.

andreidan · 2021-03-09T17:17:47Z

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

    }

+    /**
+     * Require that all "frozen" phases configured in a policy have a searchable snapshot action.


uber nit :) but this implies the possible presence of multiple frozen phases in one policy and it might be confusing

Good point, I changed this to not imply that.

…c#70158) This commit changes the frozen phase within ILM in the following ways: - The `searchable_snapshot` action now no longer takes a `storage` parameter. The storage type is determined by the phase within which it is invoked (shared cache for frozen and full copy for everything else). - The frozen phase in ILM now no longer allows *any* actions other than `searchable_snapshot` - If a frozen phase is provided, it *must* include a `searchable_snapshot` action. These changes may seem breaking, but since they are intended to go back to 7.12 which has not been released yet, they are not truly breaking changes.

With elastic#70158 (master), elastic#70170 (7.x), and elastic#70171 (7.12) merged, we can now re-enable backwards compatibilty tests on master and 7.x.

The action was recently removed from the list of acceptable actions (elastic#70158), the test shouldn't use it any more.

The action was recently removed from the list of acceptable actions (#70158), the test shouldn't use it any more.

The action was recently removed from the list of acceptable actions (elastic#70158), the test shouldn't use it any more.

The action was recently removed from the list of acceptable actions (#70158), the test shouldn't use it any more.

…70158) (#70171) * Restrict ILM frozen phase to searchable snapshot actions only (#70158) This commit changes the frozen phase within ILM in the following ways: - The `searchable_snapshot` action now no longer takes a `storage` parameter. The storage type is determined by the phase within which it is invoked (shared cache for frozen and full copy for everything else). - The frozen phase in ILM now no longer allows *any* actions other than `searchable_snapshot` - If a frozen phase is provided, it *must* include a `searchable_snapshot` action. These changes may seem breaking, but since they are intended to go back to 7.12 which has not been released yet, they are not truly breaking changes. * Fix compilation

…70158) (#70170) Backports the following commits to 7.x: Restrict ILM frozen phase to searchable snapshot actions only (#70158) Remove "frozen" from read only tests (#70173)

With #70158 (master), #70170 (7.x), and #70171 (7.12) merged, we can now re-enable backwards compatibilty tests on master and 7.x.

…ic#70179) With elastic#70158 (master), elastic#70170 (7.x), and elastic#70171 (7.12) merged, we can now re-enable backwards compatibilty tests on master and 7.x.

…70179) (#70188) With #70158 (master), #70170 (7.x), and #70171 (7.12) merged, we can now re-enable backwards compatibilty tests on master and 7.x.

dakrone added blocker :Data Management/ILM+SLM DO NOT USE. Use ":StorageEngine/ILM" or ":Distributed Coordination/SLM" instead. v8.0.0 v7.12.0 v7.13.0 labels Mar 9, 2021

elasticmachine added the Team:Data Management (obsolete) DO NOT USE. This team no longer exists. label Mar 9, 2021

Update issue for BWC disabled

2a21ea1

dakrone commented Mar 9, 2021

View reviewed changes

dakrone requested a review from andreidan March 9, 2021 15:49

andreidan approved these changes Mar 9, 2021

View reviewed changes

Fixup documentation not to imply multiple frozen phases

28c4bc2

dakrone merged commit 67f13bb into elastic:master Mar 9, 2021

dakrone deleted the ilm-frozen-strictness branch March 9, 2021 18:24

dakrone mentioned this pull request Mar 9, 2021

[7.x] Restrict ILM frozen phase to searchable snapshot actions only (#70158) #70170

Merged

dakrone mentioned this pull request Mar 9, 2021

[7.12] Restrict ILM frozen phase to searchable snapshot actions only (#70158) #70171

Merged

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

Remove "frozen" from read only tests

cbd9e16

The action was recently removed from the list of acceptable actions (elastic#70158), the test shouldn't use it any more.

dakrone mentioned this pull request Mar 9, 2021

Remove "frozen" from read only tests #70173

Merged

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

Remove "frozen" from read only tests (#70173)

75ffa41

The action was recently removed from the list of acceptable actions (#70158), the test shouldn't use it any more.

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

Remove "frozen" from read only tests (elastic#70173)

53bf5f4

The action was recently removed from the list of acceptable actions (elastic#70158), the test shouldn't use it any more.

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

Remove "frozen" from read only tests (#70173)

dfac396

The action was recently removed from the list of acceptable actions (#70158), the test shouldn't use it any more.

dakrone mentioned this pull request Mar 9, 2021

Re-enable BWC tests after backport of ILM frozen phase changes #70179

Merged

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

Re-enable BWC tests after backport of ILM frozen phase changes (#70179)

7cd3f6d

With #70158 (master), #70170 (7.x), and #70171 (7.12) merged, we can now re-enable backwards compatibilty tests on master and 7.x.

henningandersen mentioned this pull request Mar 18, 2021

[DOCS] Frozen tier dedicated #70542

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

Restrict ILM frozen phase to searchable snapshot actions only#70158

Restrict ILM frozen phase to searchable snapshot actions only#70158
dakrone merged 3 commits intoelastic:masterfrom
dakrone:ilm-frozen-strictness

dakrone commented Mar 9, 2021

Uh oh!

elasticmachine commented Mar 9, 2021

Uh oh!

dakrone Mar 9, 2021 •

edited

Loading

Uh oh!

andreidan left a comment

Uh oh!

andreidan Mar 9, 2021

Uh oh!

dakrone Mar 9, 2021

Uh oh!

andreidan Mar 9, 2021

Uh oh!

dakrone Mar 9, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		boolean bwc_tests_enabled = false
		String bwc_tests_disabled_issue = "https://github.com/elastic/elasticsearch/pull/70158" /* place a PR link here when committing bwc changes */

Conversation

dakrone commented Mar 9, 2021

Uh oh!

elasticmachine commented Mar 9, 2021

Uh oh!

dakrone Mar 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andreidan left a comment

Choose a reason for hiding this comment

Uh oh!

andreidan Mar 9, 2021

Choose a reason for hiding this comment

Uh oh!

dakrone Mar 9, 2021

Choose a reason for hiding this comment

Uh oh!

andreidan Mar 9, 2021

Choose a reason for hiding this comment

Uh oh!

dakrone Mar 9, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dakrone Mar 9, 2021 •

edited

Loading