Skip to content

Ignore obsolete dangling indices#37824

Merged
henningandersen merged 7 commits intoelastic:6.xfrom
henningandersen:fix_ignore_obsolete_dangling_indices
Feb 2, 2019
Merged

Ignore obsolete dangling indices#37824
henningandersen merged 7 commits intoelastic:6.xfrom
henningandersen:fix_ignore_obsolete_dangling_indices

Conversation

@henningandersen
Copy link
Copy Markdown
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/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
Copy Markdown
Collaborator

Pinging @elastic/es-distributed

@henningandersen henningandersen self-assigned this Jan 24, 2019
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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/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