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 metadata of deleted indices at start #48918

Merged

Conversation

@DaveCTurner
Copy link
Contributor

DaveCTurner commented Nov 9, 2019

Today in 6.x it is possible to add an index tombstone to the graveyard without
deleting the corresponding index metadata, because the deletion is slightly
deferred. If you shut down the node and upgrade to 7.x when in this state then
the node will fail to apply any cluster states, reporting

java.lang.IllegalStateException: Cannot delete index [...], it is still part of the cluster state.

This commit addresses this situation by skipping over any index metadata with a
corresponding tombstone, allowing this metadata to be cleaned up by the 7.x
node.

Today in 6.x it is possible to add an index tombstone to the graveyard without
deleting the corresponding index metadata, because the deletion is slightly
deferred. If you shut down the node and upgrade to 7.x when in this state then
the node will fail to apply any cluster states, reporting

    java.lang.IllegalStateException: Cannot delete index [...], it is still part of the cluster state.

This commit addresses this situation by skipping over any index metadata with a
corresponding tombstone, allowing this metadata to be cleaned up by the 7.x
node.
@elasticmachine

This comment has been minimized.

Copy link
Collaborator

elasticmachine commented Nov 9, 2019

Pinging @elastic/es-distributed (:Distributed/Cluster Coordination)

@DaveCTurner

This comment has been minimized.

Copy link
Contributor Author

DaveCTurner commented Nov 10, 2019

This needs more work because it doesn't address the case where you're in this state in a rolling upgrade.

@DaveCTurner

This comment has been minimized.

Copy link
Contributor Author

DaveCTurner commented Nov 11, 2019

This needs more work because it doesn't address the case where you're in this state in a rolling upgrade.

Discussed this and decided it's ok for a rolling upgrade to fall back on a full cluster restart if it happens to be in this state, and this PR will allow the full cluster restart to proceed.

Copy link
Contributor

andrershov left a comment

Looks good, I've left one request to rename the function name


final MetaData metaData = internalCluster().getInstance(ClusterService.class).state().metaData();
final Path[] paths = internalCluster().getInstance(NodeEnvironment.class).nodeDataPaths();
writeBrokenMeta(metaStateService -> {

This comment has been minimized.

Copy link
@andrershov

andrershov Nov 11, 2019

Contributor

I know this is not your change, but the name "writeBrokenMeta" looks invalid for two reasons - in this test we write well-formed metadata, this method performs full-cluster restart and I think this should be reflected in the method name.

This comment has been minimized.

Copy link
@DaveCTurner

DaveCTurner Nov 11, 2019

Author Contributor

A more substantial change to this method (including fixing its name) is incoming in https://github.com/elastic/elasticsearch/pull/48733/files#diff-a53ee618ca95b1bde55d7f5508a03d6aR511.

This comment has been minimized.

Copy link
@DaveCTurner

DaveCTurner Nov 11, 2019

Author Contributor

However the metadata written here is well-formed but still broken - it contains a tombstone for an index that is not properly deleted.

@DaveCTurner DaveCTurner requested a review from andrershov Nov 11, 2019
Copy link
Contributor

andrershov left a comment

LGTM

@jimczi jimczi removed the v7.5.0 label Nov 12, 2019
@DaveCTurner DaveCTurner merged commit 13170a7 into elastic:master Nov 12, 2019
8 checks passed
8 checks passed
CLA All commits in pull request signed
Details
elasticsearch-ci/1 Build finished.
Details
elasticsearch-ci/2 Build finished.
Details
elasticsearch-ci/bwc Build finished.
Details
elasticsearch-ci/default-distro Build finished.
Details
elasticsearch-ci/docs Build finished.
Details
elasticsearch-ci/oss-distro-docs Build finished.
Details
elasticsearch-ci/packaging-sample-matrix Build finished.
Details
@DaveCTurner DaveCTurner deleted the DaveCTurner:2019-11-08-half-deleted-index-import branch Nov 12, 2019
DaveCTurner added a commit that referenced this pull request Nov 12, 2019
Today in 6.x it is possible to add an index tombstone to the graveyard without
deleting the corresponding index metadata, because the deletion is slightly
deferred. If you shut down the node and upgrade to 7.x when in this state then
the node will fail to apply any cluster states, reporting

    java.lang.IllegalStateException: Cannot delete index [...], it is still part of the cluster state.

This commit addresses this situation by skipping over any index metadata with a
corresponding tombstone, allowing this metadata to be cleaned up by the 7.x
node.
DaveCTurner added a commit that referenced this pull request Nov 12, 2019
Today in 6.x it is possible to add an index tombstone to the graveyard without
deleting the corresponding index metadata, because the deletion is slightly
deferred. If you shut down the node and upgrade to 7.x when in this state then
the node will fail to apply any cluster states, reporting

    java.lang.IllegalStateException: Cannot delete index [...], it is still part of the cluster state.

This commit addresses this situation by skipping over any index metadata with a
corresponding tombstone, allowing this metadata to be cleaned up by the 7.x
node.
@DaveCTurner DaveCTurner added v7.5.0 and removed backport pending labels Nov 12, 2019
debadair added a commit to debadair/elasticsearch that referenced this pull request Nov 13, 2019
Today in 6.x it is possible to add an index tombstone to the graveyard without
deleting the corresponding index metadata, because the deletion is slightly
deferred. If you shut down the node and upgrade to 7.x when in this state then
the node will fail to apply any cluster states, reporting

    java.lang.IllegalStateException: Cannot delete index [...], it is still part of the cluster state.

This commit addresses this situation by skipping over any index metadata with a
corresponding tombstone, allowing this metadata to be cleaned up by the 7.x
node.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.