-
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
Add validation for supported index version on node join, restore, upgrade & open index #21830
Changes from 6 commits
5965ab3
d9aed75
5c9a61f
f63bb98
7231d45
c709e34
544d70e
df43c5e
0d1be73
fbf4dbc
d835ac4
31aee20
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -67,13 +67,13 @@ public MetaDataIndexUpgradeService(Settings settings, MapperRegistry mapperRegis | |
* If the index does not need upgrade it returns the index metadata unchanged, otherwise it returns a modified index metadata. If index | ||
* cannot be updated the method throws an exception. | ||
*/ | ||
public IndexMetaData upgradeIndexMetaData(IndexMetaData indexMetaData) { | ||
public IndexMetaData upgradeIndexMetaData(IndexMetaData indexMetaData, Version minimumIndexCompatibilityVersion) { | ||
// Throws an exception if there are too-old segments: | ||
if (isUpgraded(indexMetaData)) { | ||
assert indexMetaData == archiveBrokenIndexSettings(indexMetaData) : "all settings must have been upgraded before"; | ||
return indexMetaData; | ||
} | ||
checkSupportedVersion(indexMetaData); | ||
checkSupportedVersion(indexMetaData, minimumIndexCompatibilityVersion); | ||
IndexMetaData newMetaData = indexMetaData; | ||
// we have to run this first otherwise in we try to create IndexSettings | ||
// with broken settings and fail in checkMappingsCompatibility | ||
|
@@ -92,21 +92,22 @@ boolean isUpgraded(IndexMetaData indexMetaData) { | |
} | ||
|
||
/** | ||
* Elasticsearch 5.0 no longer supports indices with pre Lucene v5.0 (Elasticsearch v2.0.0.beta1) segments. All indices | ||
* that were created before Elasticsearch v2.0.0.beta1 should be reindexed in Elasticsearch 2.x | ||
* Elasticsearch 6.0 no longer supports indices with pre Lucene v5.0 (Elasticsearch v5.0.0.beta1) segments. All indices | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: we lead with ES and make lucene second? it's a bit weird now as ES v5 uses lucene 6.0. How about "ES 6.0 no longer supports indices created by an ES version earlier than 5.0" and leave it at that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seriously? |
||
* that were created before Elasticsearch v5.0.0.beta1 should be reindexed in Elasticsearch 5.x | ||
* before they can be opened by this version of elasticsearch. */ | ||
private void checkSupportedVersion(IndexMetaData indexMetaData) { | ||
if (indexMetaData.getState() == IndexMetaData.State.OPEN && isSupportedVersion(indexMetaData) == false) { | ||
throw new IllegalStateException("The index [" + indexMetaData.getIndex() + "] was created before v2.0.0.beta1." | ||
+ " It should be reindexed in Elasticsearch 2.x before upgrading to " + Version.CURRENT + "."); | ||
private void checkSupportedVersion(IndexMetaData indexMetaData, Version minimumIndexCompatibilityVersion) { | ||
if (indexMetaData.getState() == IndexMetaData.State.OPEN && isSupportedVersion(indexMetaData, | ||
minimumIndexCompatibilityVersion) == false) { | ||
throw new IllegalStateException("The index [" + indexMetaData.getIndex() + "] was created before v5.0.0.beta1." | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any way we could bake the minimumIndexCompatibilityVersion into the message ? I'm afraid we'll forget in the future and the error message will be wrong. |
||
+ " It should be reindexed in Elasticsearch 5.x before upgrading to " + Version.CURRENT + "."); | ||
} | ||
} | ||
|
||
/* | ||
* Returns true if this index can be supported by the current version of elasticsearch | ||
*/ | ||
private static boolean isSupportedVersion(IndexMetaData indexMetaData) { | ||
return indexMetaData.getCreationVersion().onOrAfter(Version.V_2_0_0_beta1); | ||
private static boolean isSupportedVersion(IndexMetaData indexMetaData, Version minimumIndexCompatibilityVersion) { | ||
return indexMetaData.getCreationVersion().onOrAfter(minimumIndexCompatibilityVersion); | ||
} | ||
|
||
/** | ||
|
@@ -173,4 +174,4 @@ IndexMetaData archiveBrokenIndexSettings(IndexMetaData indexMetaData) { | |
return indexMetaData; | ||
} | ||
} | ||
} | ||
} |
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.
hmm... I'm wondering if we should exclude the non-data non-master nodes here. They do all the coordination and might rely on some metadata they can't digest? We're still have to keep the transport client around, but I don't think we should exclude ClientNode (now called coordinating nodes) ?
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.
This holds for other places in the code as well...
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 don't think coordinating nodes do rely on any metadata they can't digest. They don't parse mappings afaik (I am convinced they should not) which is the main thing here so I thin we are good
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.
the actual question is why do we use the smallest, we have to use the largest node in the cluster since that is the one relevant for the index version to be opened.
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.
yikes. Of course.