Autoscale frozen tier into existence #73435

henningandersen · 2021-05-26T18:52:58Z

This commit adds two related changes:

ILM WaitForDataTierStep
Autoscaling frozen_existence decider

The first part ensures that we wait mounting an index until a node that
can hold the index is available, avoiding a failed restore and red
cluster state. This is in particular important for the frozen phase, but
is done generically in the searchable snapshot action.

The second part triggers on indices in the ILM frozen phase to scale the
tier into existence by requiring a minimal amount of memory and storage.

Closes #72771

elasticmachine · 2021-05-26T18:53:01Z

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

elasticmachine · 2021-05-26T18:53:02Z

Pinging @elastic/es-distributed (Team:Distributed)

This commit adds two related changes: * ILM WaitForDataTierStep * Autoscaling frozen_existence decider The first part ensures that we wait mounting an index until a node that can hold the index is available, avoiding a failed restore and red cluster state. This is in particular important for the frozen phase, but is done generically in the searchable snapshot action. The second part triggers on indices in the ILM frozen phase to scale the tier into existence by requiring a minimal amount of memory and storage. Closes elastic#72771

…_tier

dakrone

I left some initial comments about the ILM-specific stuff

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

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

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

dakrone · 2021-05-27T17:13:54Z

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

 // here before generating snapshots that can't be used if the user doesn't have the right license level.
 BranchingStep conditionalSkipActionStep = new BranchingStep(preActionBranchingKey, checkNoWriteIndex, nextStepKey,
 (index, clusterState) -> {
- XPackLicenseState licenseState = XPackPlugin.getSharedLicenseState();


Can you explain why we need to move the license state back up into the top level instead of getting it here? It added quite a few lines injecting it into every ILM action

The problem is that XPackPlugin.getSharedLicenseState() is backed by a static SetOnce field making it unsuitable for use in tests. This pattern of passing in the licenseState was taken from ML. In CCR and Autoscaling, the licenceState is wrapped in a checker that is passed around instead. For ILM, the ML approach felt most natural since it allows any license check in the future.

I think there is no internal cluster tests today that trigger the SearchableSnapshotAction, however, I needed to add one in FrozenExistenceDeciderIT, necessitating this change.

If there is a different and/or better pattern for doing this I should be more than happy to adapt this.

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

Now wait for hot tier in hot phase rather than any of cold,warm,hot.

…_tier

henningandersen · 2021-05-28T06:52:47Z

@elasticmachine run elasticsearch-ci/packaging-tests-windows-sample

…_tier

By adding a default method to LifecycleAction we can reduce the impact on every step in ILM.

…_tier

dakrone

ILM parts LGTM, thanks for iterating on this Henning

DaveCTurner

LGTM I think, sorry it's taken a while to get my head around this. I left only trivial and optional comments.

DaveCTurner · 2021-06-18T16:16:46Z

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


 @Override
 public List<Step> toSteps(Client client, String phase, StepKey nextStepKey) {
+ throw new UnsupportedOperationException();


nit: assert false here too? For the record I'd rather just add the parameter to toSteps() everywhere rather than have this uncalled overload, i.e, the opposite of what we did in d4b2e32.

See 63816a0. I left the overload as is.

DaveCTurner · 2021-06-18T16:17:08Z

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

+ * performing a full mount the preference is cold - warm - hot. When
+ * performing a partial mount the preference is only frozen
+ */
+ public static String getDataTiersPreference(Storage type) {


nit/qq: why not an instance method on Storage?

Doh, thanks, fixed in 32371ac

DaveCTurner · 2021-06-18T16:17:42Z

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

 *
 * @param client The Elasticsearch Client to use during execution of {@link AsyncActionStep}
 * and {@link AsyncWaitStep} steps.
+ * @param licenseState


nit: undocumented, suggest removal?

Suggested change

* @param licenseState

I added a bit of doc in aec84ec

DaveCTurner · 2021-06-18T16:18:46Z

...lugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/SearchableSnapshotActionTests.java

 assertThat(steps.get(12).getKey(), is(expectedSteps.get(12)));
 assertThat(steps.get(13).getKey(), is(expectedSteps.get(13)));
 assertThat(steps.get(14).getKey(), is(expectedSteps.get(14)));
+ assertThat(steps.get(15).getKey(), is(expectedSteps.get(15)));


nit: what's the deal with all this unrolling?

I mainly followed the same methodology. I will leave it to @dakrone to comment on the idea behind this?

There's no reason these have to be unrolled, as long as we have the expected steps comparing them all at once is fine. I don't know why they were initially unrolled.

…_tier

This commit adds two related changes: * ILM WaitForDataTierStep * Autoscaling frozen_existence decider The first part ensures that we wait mounting an index until a node that can hold the index is available, avoiding a failed restore and red cluster state. This is in particular important for the frozen phase, but is done generically in the searchable snapshot action. The second part triggers on indices in the ILM frozen phase to scale the tier into existence by requiring a minimal amount of memory and storage. Closes elastic#72771

This commit adds two related changes: * ILM WaitForDataTierStep * Autoscaling frozen_existence decider The first part ensures that we wait mounting an index until a node that can hold the index is available, avoiding a failed restore and red cluster state. This is in particular important for the frozen phase, but is done generically in the searchable snapshot action. The second part triggers on indices in the ILM frozen phase to scale the tier into existence by requiring a minimal amount of memory and storage. Closes #72771

This commit adds two related changes: * ILM WaitForDataTierStep * Autoscaling frozen_existence decider The first part ensures that we wait mounting an index until a node that can hold the index is available, avoiding a failed restore and red cluster state. This is in particular important for the frozen phase, but is done generically in the searchable snapshot action. The second part triggers on indices in the ILM frozen phase to scale the tier into existence by requiring a minimal amount of memory and storage. Closes elastic#72771

This commit adds two related changes: * ILM WaitForDataTierStep * Autoscaling frozen_existence decider The first part ensures that we wait mounting an index until a node that can hold the index is available, avoiding a failed restore and red cluster state. This is in particular important for the frozen phase, but is done generically in the searchable snapshot action. The second part triggers on indices in the ILM frozen phase to scale the tier into existence by requiring a minimal amount of memory and storage. Closes #72771

henningandersen added >enhancement :Data Management/ILM+SLM Index and Snapshot lifecycle management v8.0.0 :Distributed Coordination/Autoscaling v7.14.0 v7.13.1 labels May 26, 2021

elasticmachine added Team:Data Management Meta label for data/management team Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. labels May 26, 2021

henningandersen force-pushed the fix_wait_for_frozen_tier branch from 7642989 to 275b81a Compare May 26, 2021 18:59

henningandersen force-pushed the fix_wait_for_frozen_tier branch from 275b81a to b4a2df5 Compare May 26, 2021 19:04

henningandersen added 2 commits May 26, 2021 21:04

Merge remote-tracking branch 'origin/master' into fix_wait_for_frozen…

5df7e9e

…_tier

Fix docs

557d457

dakrone self-requested a review May 26, 2021 20:39

henningandersen requested a review from DaveCTurner May 27, 2021 08:49

fix nit

418ca90

dakrone reviewed May 27, 2021

View reviewed changes

henningandersen added 2 commits May 28, 2021 08:12

Wait for hot tier too

3d23644

Now wait for hot tier in hot phase rather than any of cold,warm,hot.

Merge remote-tracking branch 'origin/master' into fix_wait_for_frozen…

7754278

…_tier

henningandersen mentioned this pull request May 28, 2021

[CI] destructiveDistroTest fails because it cannot remove the data directory #65047

Closed

henningandersen added 3 commits May 29, 2021 07:02

Add comment to avoid de-tuning.

bfc5fa4

Use SingleMessageFieldInfo

84b99ef

Merge remote-tracking branch 'origin/master' into fix_wait_for_frozen…

70a24f1

…_tier

henningandersen requested a review from dakrone May 29, 2021 06:16

csoulios added v7.13.2 and removed v7.13.1 labels Jun 2, 2021

Smaller impact change

d4b2e32

By adding a default method to LifecycleAction we can reduce the impact on every step in ILM.

henningandersen added 5 commits June 2, 2021 18:12

Rollback a few more nit changes.

61f7109

Merge remote-tracking branch 'origin/master' into fix_wait_for_frozen…

1152652

…_tier

Cleanup

3f4ca5c

cs

77c13bf

Merge remote-tracking branch 'origin/master' into fix_wait_for_frozen…

24e2d29

…_tier

dakrone approved these changes Jun 3, 2021

View reviewed changes

danhermann added v7.13.3 and removed v7.13.2 labels Jun 14, 2021

DaveCTurner approved these changes Jun 18, 2021

View reviewed changes

henningandersen added 6 commits June 21, 2021 18:09

Merge remote-tracking branch 'origin/master' into fix_wait_for_frozen…

6208035

…_tier

post merge fixes

0c82fbf

Storage.defaultDataTiersPreference

32371ac

Proper javadoc

aec84ec

assert false

63816a0

nits

8e6996e

henningandersen merged commit 7793838 into elastic:master Jun 22, 2021

henningandersen mentioned this pull request Jun 22, 2021

Autoscale frozen tier into existence (#73435) #74412

Merged

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

kingherc mentioned this pull request Jul 4, 2023

Sudden influx of ILM cold shards turn cluster RED before it is autoscaled #97370

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Autoscale frozen tier into existence #73435

Autoscale frozen tier into existence #73435

Uh oh!

henningandersen commented May 26, 2021

elasticmachine commented May 26, 2021

elasticmachine commented May 26, 2021

dakrone left a comment

Uh oh!

Uh oh!

Uh oh!

dakrone May 27, 2021

henningandersen May 27, 2021

Uh oh!

henningandersen commented May 28, 2021

dakrone left a comment

DaveCTurner left a comment

DaveCTurner Jun 18, 2021

henningandersen Jun 21, 2021

DaveCTurner Jun 18, 2021

henningandersen Jun 21, 2021

DaveCTurner Jun 18, 2021

henningandersen Jun 21, 2021

DaveCTurner Jun 18, 2021

henningandersen Jun 21, 2021

dakrone Jun 21, 2021

Labels

7 participants

Autoscale frozen tier into existence #73435

Autoscale frozen tier into existence #73435

Uh oh!

Conversation

henningandersen commented May 26, 2021

elasticmachine commented May 26, 2021

elasticmachine commented May 26, 2021

dakrone left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Uh oh!

henningandersen commented May 28, 2021

dakrone left a comment

Choose a reason for hiding this comment

DaveCTurner 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

Labels

7 participants