Split RolloverStep into Wait and Action steps#35524
Merged
AthenaEryma merged 12 commits intoelastic:masterfrom Nov 16, 2018
Merged
Split RolloverStep into Wait and Action steps#35524AthenaEryma merged 12 commits intoelastic:masterfrom
AthenaEryma merged 12 commits intoelastic:masterfrom
Conversation
RolloverAction will now periodically check the rollover conditions using the Rollover API with the dry_run option as an AsyncWaitStep, then run the rollover itself by calling the Rollover API with no conditions, which will always roll over, as an AsyncActionStep. This will resolve race condition issues in policies using RolloverAction.
Collaborator
|
Pinging @elastic/es-core-infra |
colings86
previously requested changes
Nov 14, 2018
Contributor
colings86
left a comment
There was a problem hiding this comment.
Can we re-enable the ChangePolicyforIndexIT#testChangePolicyForIndex test as part of this PR so we ca be sure that this fixes the original issue?
...ck/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/RolloverAction.java
Outdated
Show resolved
Hide resolved
Contributor
Author
|
Thanks for the reminder @colings86, I've reinstated the test which was failing ( |
dakrone
approved these changes
Nov 14, 2018
Member
dakrone
left a comment
There was a problem hiding this comment.
LGTM too, I left one minor comment
...core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/WaitForRolloverReadyStep.java
Outdated
Show resolved
Hide resolved
left a LGTM contingent on my LGTM
AthenaEryma
added a commit
to AthenaEryma/elasticsearch
that referenced
this pull request
Nov 16, 2018
RolloverAction will now periodically check the rollover conditions using the Rollover API with the dry_run option as an AsyncWaitStep, then run the rollover itself by calling the Rollover API with no conditions, which will always roll over, as an AsyncActionStep. This will resolve race condition issues in policies using RolloverAction.
AthenaEryma
added a commit
that referenced
this pull request
Nov 16, 2018
RolloverAction will now periodically check the rollover conditions using the Rollover API with the dry_run option as an AsyncWaitStep, then run the rollover itself by calling the Rollover API with no conditions, which will always roll over, as an AsyncActionStep. This will resolve race condition issues in policies using RolloverAction.
This was referenced Nov 16, 2018
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RolloverAction will now periodically check the rollover conditions using
the Rollover API with the dry_run option as an AsyncWaitStep, then run
the rollover itself by calling the Rollover API with no conditions,
which will always roll over, as an AsyncActionStep. This will resolve
race condition issues in policies using RolloverAction.
Replaces #35391 as this approach is more streamlined.
Relates to #35313
Also relates to #35244
Background for this PR: We agreed that it would be best to be able to separate checking the rollover conditions from executing the rollover. In the middle of refactoring the API to accomplish this, I noticed that this is already possible: Calling the Rollover API without providing conditions will unconditionally roll over the index, without using any hacky workarounds such as
max_size: 0b. This is not mentioned in the Rollover API documentation - I will be opening a separate PR to add it to the docs.