Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add setting to bypass Rollover action #36235

Merged
merged 13 commits into from
Dec 11, 2018

Conversation

gwbrown
Copy link
Contributor

@gwbrown gwbrown commented Dec 4, 2018

Adds a setting that indicates that an index is done indexing, set by ILM
when the Rollover action completes. This indicates that the Rollover
action should be skipped in any future invocations, as long as the index
is no longer the write index for its alias.

This enables 1) an index with a policy that involves the Rollover action
to have the policy removed and switched to another one without use of
the move-to-step API, and 2) integrations with Beats and CCR.

Relates to #35944 and #34648

Adds a setting that indicates that an index is done indexing, set by ILM
when the Rollover action completes. This indicates that the Rollover
action should be skipped in any future invocations, as long as the index
is no longer the write index for its alias.

This enables 1) an index with a policy that involves the Rollover action
to have the policy removed and switched to another one without use of
the move-to-step API, and 2) integrations with Beats and CCR.
@gwbrown gwbrown added >enhancement WIP :Data Management/ILM+SLM Index and Snapshot lifecycle management labels Dec 4, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features

@gwbrown gwbrown removed the WIP label Dec 6, 2018
@gwbrown gwbrown requested review from colings86, talevy and martijnvg and removed request for colings86 December 6, 2018 00:48
@gwbrown
Copy link
Contributor Author

gwbrown commented Dec 6, 2018

Open question: Is this something we want to document? It's main use is internal, and it may be more confusing than useful to most users.

Copy link
Contributor

@colings86 colings86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a comment. Also this setting definitely needs to be documented as e.g. Beats will need to get users to set it when they upgrade their beats so the last index on the old version can be told to progress past the rollover action. One thing we probably want to do is add a warning box to the documentation for this which warns the user that setting this manually will mean the index will not rollover so will not create a new index and move the write alias. This is by design but it would be good to warn users so its clear.

UpdateRolloverLifecycleDateStep updateDateStep = new UpdateRolloverLifecycleDateStep(updateDateStepKey, setIndexingCompleteStepKey,
System::currentTimeMillis);
UpdateSettingsStep setIndexingCompleteStep = new UpdateSettingsStep(setIndexingCompleteStepKey, nextStepKey,
client, indexingComplete);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this actually need to be a different step to the lifecycle date step? I think we could do both things in one step?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put it in a different step originally because I felt it was better to keep separate actions in separate steps. I've rolled it into UpdateRolloverLifecycleDateStep, how does that look?

Also, I've added some documentation. I'm not sure it's in the best place though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After discussing this synchronously, I'm going to revert the change and keep this in a separate step, as it's good to go through the Settings infrastructure if we can, rather than bypassing it in a ClusterStateActionStep.

Copy link
Contributor

@colings86 colings86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM apart from the one typo comment

completely.

IMPORTANT: If `index.lifecycle.indexing_complete` is set to `true` on an index,
it will not be rolled over by {ilm}, but {ilm} will very that this index is no
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it will not be rolled over by {ilm}, but {ilm} will very that this index is no
it will not be rolled over by {ilm}, but {ilm} will verify that this index is no

typo?

@gwbrown gwbrown merged commit 6481f2e into elastic:master Dec 11, 2018
gwbrown added a commit that referenced this pull request Dec 11, 2018
Adds a setting that indicates that an index is done indexing, set by ILM
when the Rollover action completes. This indicates that the Rollover
action should be skipped in any future invocations, as long as the index
is no longer the write index for its alias.

This enables 1) an index with a policy that involves the Rollover action
to have the policy removed and switched to another one without use of
the move-to-step API, and 2) integrations with Beats and CCR.
jasontedor added a commit to dnhatn/elasticsearch that referenced this pull request Dec 11, 2018
* elastic/master: (36 commits)
  Add check for minimum required Docker version (elastic#36497)
  Minor search controller changes (elastic#36479)
  Add default methods to DocValueFormat (elastic#36480)
  Fix the mixed cluster REST test explain/11_basic_with_types.
  Modify `BigArrays` to take name of circuit breaker (elastic#36461)
  Move LoggedExec to minimumRuntime source set (elastic#36453)
  added 6.5.4 version
  Add test logging for elastic#35644
  Tests- added helper methods to ESRestTestCase for checking warnings (elastic#36443)
  SQL: move requests' parameters to requests JSON body (elastic#36149)
  [Zen2] Respect the no_master_block setting (elastic#36478)
  Require soft-deletes when access changes snapshot (elastic#36446)
  Switch more tests to zen2 (elastic#36367)
  [Painless] Add extensive tests for def to primitive casts (elastic#36455)
  Add setting to bypass Rollover action (elastic#36235)
  Try running CI against Zulu (elastic#36391)
  [DOCS] Reworked the shard allocation filtering info.  (elastic#36456)
  Log [initial_master_nodes] on formation failure (elastic#36466)
  converting ForbiddenPatternsTask to .java (elastic#36194)
  fixed typo
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker :Data Management/ILM+SLM Index and Snapshot lifecycle management >enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants