Skip to content

Adds checks to ensure index metadata exists when we try to use it#33455

Merged
colings86 merged 2 commits intoelastic:index-lifecyclefrom
colings86:ilm/fix-no-index
Sep 7, 2018
Merged

Adds checks to ensure index metadata exists when we try to use it#33455
colings86 merged 2 commits intoelastic:index-lifecyclefrom
colings86:ilm/fix-no-index

Conversation

@colings86
Copy link
Copy Markdown
Contributor

IF the index no longer exists when we try to retrieve the index metadata it's been delete since the policy was triggered and there is nothing for us to do. Currently in a few places we throw a NPE if this happens which, although it does not affect the execution result is ugly and logs a needless stack trace which suggests something might be wrong even though its not. This change makes sure we check for when this happens, logs a debug message and then ignores execution of that index for whatever operation is in progress.

@colings86 colings86 added review :Data Management/ILM+SLM DO NOT USE. Use ":StorageEngine/ILM" or ":Distributed Coordination/SLM" instead. labels Sep 6, 2018
@colings86 colings86 self-assigned this Sep 6, 2018
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-core-infra

Copy link
Copy Markdown
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 optional comment. I've run into this issue a few times locally so it's good to fix it in as many places as possible

}

private void runPolicy(IndexMetaData indexMetaData, ClusterState currentState) {
if (indexMetaData == null) {
Copy link
Copy Markdown
Member

@dakrone dakrone Sep 6, 2018

Choose a reason for hiding this comment

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

I think my preference would be to handle this in the caller of runPolicy, what do you think? (I'm okay either way)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

moveToStep is the only caller of this method, so I am also more-or-less indifferent to where this goes

Copy link
Copy Markdown
Contributor

@talevy talevy left a comment

Choose a reason for hiding this comment

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

LGTM once CI is happy

@colings86 colings86 merged commit f836413 into elastic:index-lifecycle Sep 7, 2018
@colings86 colings86 deleted the ilm/fix-no-index branch September 7, 2018 12:06
AthenaEryma added a commit that referenced this pull request Oct 23, 2018
Adds a null check for IndexMetaData similar to those in #33455.
AthenaEryma added a commit that referenced this pull request Oct 23, 2018
Adds a null check for IndexMetaData similar to those in #33455.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Data Management/ILM+SLM DO NOT USE. Use ":StorageEngine/ILM" or ":Distributed Coordination/SLM" instead.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants