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.

Sign up for GitHub

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

Jump to bottom

Fix alias and template setup order #12149

Closed
wants to merge 4 commits into from
Closed

Fix alias and template setup order #12149

wants to merge 4 commits into from

Conversation

urso
Copy link

@urso urso commented May 9, 2019

There is the chance an index is not correctly managed by ILM, due to
setup order. This changes fixes the order.

urso added 2 commits May 9, 2019 23:06
Fix alias and template setup order
72b7187 
There is the chance an index is not correctly managed by ILM, due to
setup order. This changes fixes the order.
Add unit test
14b62e6 
Update mockClientHandler to implement the complete interface and record
the order of 'create/load' operations. This is used to validate the
index management unit tests for always checking the correct order of
index management operations in the index manager.
@urso urso requested a review from a team as a code owner May 9, 2019 21:39
@urso urso requested review from simitt and ph May 9, 2019 21:40
simitt
simitt reviewed May 10, 2019
View reviewed changes
Copy link
Contributor

@simitt simitt left a comment

Choose a reason for hiding this comment

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

So just to see if I get this wright. The problem is that a beat might already write to an alias, although the template hasn't been created yet, leading to the template not being applied to the index afterwards? Especially when users do not run dedicated setup cmd but setup is done during startup?

@ruflin ruflin added the needs_backport PR is waiting to be backported to other branches. label May 10, 2019
@ruflin
Copy link
Member

ruflin commented May 10, 2019

@urso You mention there is a chance that it is not correctly managed. But based on the code it seems that was always the case as the alias / index was always created before the template when a Beat was directly started?

I remember we had the same bug in the initial code of ILM but it got fixed before it was shipped in 6.x. So my assumption is the above affects all 7.x releases? I added a needs_backport label.

@simitt That is also my understanding.

@simitt
Copy link
Contributor

simitt commented May 10, 2019

@ruflin it looks I introduced this bug just recently https://github.com/elastic/beats/pull/11777/files#diff-a7b20900159263d87105489854d4d628R216, and I think this is only in master so far.

@simitt
Copy link
Contributor

simitt commented May 10, 2019

I remember we had the same bug in the initial code of ILM but it got fixed before it was shipped in 6.x.

@ruflin could you by any chance point to that fix? I'd like to follow along with those changes.

@urso
Copy link
Author

urso commented May 10, 2019 •
edited
Loading

Code in 7.0 handles this correctly. This PR fixes a code regression and adds improves checks. Unfortunately python tests fail now, as index is setup correctly.

Need to do some more investigation in 7.0.

@ruflin
Copy link
Member

ruflin commented May 10, 2019

@simitt Good news that it's only in master. Here is the PR for 6.6 but don't think it's relevant now that big part of the code changed: #9617 More important is that we should have some tests to validate that it works correctly.

@urso urso added bug libbeat and removed needs_backport PR is waiting to be backported to other branches. labels May 10, 2019
@simitt simitt mentioned this pull request May 10, 2019
Closed
@simitt
Copy link
Contributor

simitt commented May 10, 2019

jenkins, test this

@urso
Copy link
Author

urso commented May 10, 2019

There are a many changes in how we do setup and test in flight the recent days.
Closing this in favor of #12132, that will include the fixe and also fixes a many related integration tests.

@urso urso closed this May 10, 2019
simitt added a commit to simitt/beats that referenced this pull request May 10, 2019
@simitt
Fix order of template and write alias creation.
b5948f2 
According to elastic#12149
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simitt simitt simitt approved these changes

@ph ph Awaiting requested review from ph

Assignees
No one assigned
Labels
bug libbeat
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@urso @ruflin @simitt