Skip to content

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

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

Merged
merged 2 commits into from
Sep 7, 2018
Merged

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

merged 2 commits into from
Sep 7, 2018

Conversation

colings86
Copy link
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 Index and Snapshot lifecycle management labels Sep 6, 2018
@colings86 colings86 self-assigned this Sep 6, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

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 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

@@ -158,6 +158,10 @@ public void onFailure(Exception e) {
}

private void runPolicy(IndexMetaData indexMetaData, ClusterState currentState) {
if (indexMetaData == null) {
Copy link
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
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
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
gwbrown added a commit that referenced this pull request Oct 23, 2018
Adds a null check for IndexMetaData similar to those in #33455.
gwbrown 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 Index and Snapshot lifecycle management
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants