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

Inject Unfollow before Rollover and Shrink #37625

Merged
merged 13 commits into from
Jan 28, 2019

Conversation

gwbrown
Copy link
Contributor

@gwbrown gwbrown commented Jan 19, 2019

We inject an Unfollow action before Shrink because the Shrink action
cannot be safely used on a following index, as it may not be fully
caught up with the leader index before the "original" following index is
deleted and replaced with a non-following Shrunken index. The Unfollow
action will verify that 1) the index is marked as "complete", and 2) all
operations up to this point have been replicated from the leader to the
follower before explicitly disconnecting the follower from the leader.

Injecting an Unfollow action before the Rollover action is done mainly
as a convenience: This allow users to use the same lifecycle policy on
both the leader and follower cluster without having to explictly modify
the policy to unfollow the index, while doing what we expect users to
want in most cases.

Relates to #34648
Follow-on to #36970


This is marked as WIP because it still needs docs, but the code and tests are ready for review.

We inject an Unfollow action before Shrink because the Shrink action
cannot be safely used on a following index, as it may not be fully
caught up with the leader index before the "original" following index is
deleted and replaced with a non-following Shrunken index. The Unfollow
action will verify that 1) the index is marked as "complete", and 2) all
operations up to this point have been replicated from the leader to the
follower before explicitly disconnecting the follower from the leader.

Injecting an Unfollow action before the Rollover action is done mainly
as a convenience: This allow users to use the same lifecycle policy on
both the leader and follower cluster without having to explictly modify
the policy to unfollow the index, while doing what we expect users to
want in most cases.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features

Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

LGTM, I left one comment

@gwbrown gwbrown removed the WIP label Jan 23, 2019
@gwbrown
Copy link
Contributor Author

gwbrown commented Jan 23, 2019

This has docs now, which should be reviewed as well.

I had to fix a couple HLRC tests (21a2548 and 7a42638), which I may not have done in the best way. I ended up setting the poll interval in the client tests, as the tests which assume ILM starts waiting for Rollover immediately break when there's an injected Unfollow action. Without the fast poll interval set, it will often, but not always, wait on WaitForFollowShardTasksStep, which is an AsyncWaitStep. It's much more reliable to just wait until it settles into waiting for the Rollover condition, but if anyone has another idea I'd be happy to switch.

(@talevy the above is the thing I mentioned on slack)

Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

LGTM, left one super minor comment about a typo

docs/reference/ilm/policy-definitions.asciidoc Outdated Show resolved Hide resolved
@gwbrown
Copy link
Contributor Author

gwbrown commented Jan 24, 2019

@elasticmachine run elasticsearch-ci/default-distro

@gwbrown
Copy link
Contributor Author

gwbrown commented Jan 25, 2019

Looks like CI choked.

@elasticmachine run elasticsearch-ci/1
@elasticmachine run elasticsearch-ci/2

@gwbrown gwbrown merged commit 49bd871 into elastic:master Jan 28, 2019
gwbrown added a commit to gwbrown/elasticsearch that referenced this pull request Jan 29, 2019
We inject an Unfollow action before Shrink because the Shrink action
cannot be safely used on a following index, as it may not be fully
caught up with the leader index before the "original" following index is
deleted and replaced with a non-following Shrunken index. The Unfollow
action will verify that 1) the index is marked as "complete", and 2) all
operations up to this point have been replicated from the leader to the
follower before explicitly disconnecting the follower from the leader.

Injecting an Unfollow action before the Rollover action is done mainly
as a convenience: This allow users to use the same lifecycle policy on
both the leader and follower cluster without having to explictly modify
the policy to unfollow the index, while doing what we expect users to
want in most cases.
gwbrown added a commit that referenced this pull request Jan 30, 2019
We inject an Unfollow action before Shrink because the Shrink action
cannot be safely used on a following index, as it may not be fully
caught up with the leader index before the "original" following index is
deleted and replaced with a non-following Shrunken index. The Unfollow
action will verify that 1) the index is marked as "complete", and 2) all
operations up to this point have been replicated from the leader to the
follower before explicitly disconnecting the follower from the leader.

Injecting an Unfollow action before the Rollover action is done mainly
as a convenience: This allow users to use the same lifecycle policy on
both the leader and follower cluster without having to explictly modify
the policy to unfollow the index, while doing what we expect users to
want in most cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants