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

Projects
None yet
3 participants
@henningandersen
Copy link
Contributor

commented Jan 24, 2019

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

Ignore obsolete dangling indices
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
@elasticmachine

This comment has been minimized.

Copy link
Collaborator

commented Jan 24, 2019

@henningandersen henningandersen self-assigned this Jan 24, 2019

@henningandersen henningandersen requested a review from ywelsch Jan 24, 2019

@ywelsch
Copy link
Contributor

left a comment

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.

henningandersen added some commits Jan 31, 2019

Warn about left-behind data or metadata
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).
Warn about left-behind data or metadata
Fix checkstyle issues.
Warn about left-behind data or metadata
Improved messaging.
@henningandersen

This comment has been minimized.

Copy link
Contributor Author

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

henningandersen added some commits Jan 31, 2019

Warn about left-behind data or metadata
Fix checkstyle issue.

@henningandersen henningandersen requested a review from ywelsch Jan 31, 2019

@ywelsch
Copy link
Contributor

left a comment

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.");

This comment has been minimized.

Copy link
@ywelsch

ywelsch Jan 31, 2019

Contributor

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

This comment has been minimized.

Copy link
@ywelsch

ywelsch Jan 31, 2019

Contributor

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

Warn about left-behind data or metadata
Improved messaging and log to the deprecation logger.

@henningandersen henningandersen requested a review from ywelsch Feb 1, 2019

@ywelsch

ywelsch approved these changes Feb 1, 2019

@henningandersen henningandersen merged commit c23049a into elastic:6.x Feb 2, 2019

7 checks passed

CLA Commit author is a member of Elasticsearch
Details
elasticsearch-ci/1 Build finished.
Details
elasticsearch-ci/2 Build finished.
Details
elasticsearch-ci/default-distro Build finished.
Details
elasticsearch-ci/docbldesx Build finished.
Details
elasticsearch-ci/oss-distro-docs Build finished.
Details
elasticsearch-ci/packaging-sample Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.