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

Ignore obsolete dangling indices #37824

Conversation

henningandersen
Copy link
Contributor

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

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
@henningandersen henningandersen added >bug :Distributed Coordination/Allocation All issues relating to the decision making around placing a shard (both master logic & on the nodes) v6.7.0 labels Jan 24, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@henningandersen henningandersen self-assigned this Jan 24, 2019
Copy link
Contributor

@ywelsch ywelsch left a 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).
@henningandersen
Copy link
Contributor Author

henningandersen commented Jan 31, 2019

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

Copy link
Contributor

@ywelsch ywelsch left a 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.");
Copy link
Contributor

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

Copy link
Contributor

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.
@henningandersen henningandersen merged commit c23049a into elastic:6.x Feb 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Distributed Coordination/Allocation All issues relating to the decision making around placing a shard (both master logic & on the nodes) v6.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants