-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Ignore obsolete dangling indices #37824
Ignore obsolete dangling indices #37824
Conversation
For a non-data, non-master node we now warn about dangling indices and will otherwise ignore them. This avoids import of old indices with a following inevitable red cluster status. Issue elastic#27073
Pinging @elastic/es-distributed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Should we also log a warning at start-up that the node has metadata / data which should possibly be cleaned up? Another small follow-up would be to completely disable DanglingIndicesState
on non-master non-data nodes on 7.x, by only conditionally calling clusterService.addListener(this);
in the constructor.
Now warn about both left-behind data and metadata for non-data or non-data and non-master nodes. Disable dangling indices check completely for coordinating only nodes (non-data and non-master).
Fix checkstyle issues.
Improved messaging.
Thanks for your comments, @ywelsch, I have added those in this same PR. Seems like a better solution than the original. Since it is more or less a complete rewrite, I am requesting another review. GatewayIndexStateIT did not change since your last review |
Fix checkstyle issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @henningandersen. I like aligning this as close as possible with the 7.0 code. I've left two smaller comments, but looks good o.w.
try { | ||
ensureNoIndexMetaData(nodePaths); | ||
} catch (IllegalStateException e) { | ||
logger.warn(e.getMessage() + ", this should be cleaned up (will refuse to start in 7.0). Beware of data-loss."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should use the deprecation logger here (see DeprecationLogger
class). That one will log to a different log file. This will help users in finding all things to address before upgrading
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about the "Beware of data-loss." part of the message. That's not really actionable. Perhaps it could say something along the lines of "create a backup copy before removing."
Improved messaging and log to the deprecation logger.
For a non-data, non-master node we now warn about dangling indices and
will otherwise ignore them. This avoids import of old indices with a
following inevitable red cluster status.
Issue #27073