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

Don't allow indices containing too-old segments to be opened #11072

Merged
merged 1 commit into from May 20, 2015

Conversation

@imotov
Copy link
Member

commented May 8, 2015

When an index is introduced into the cluster via cluster upgrade, restore or as a dangled index the MetaDataIndexUpgradeService checks if this index can be upgraded to the current version. If upgrade is not possible, the newly upgraded cluster startup and restore process are aborted, the dangled index is imported as a closed index that cannot be open. This change doesn't actually checks the segment versions. Instead, it indiscriminately bans all old indices created with Elasticsearch older than v0.90. In 1.6 we will add an additional logic to upgrade API that will mark upgraded indices as "upgraded" using special setting and we will modify this process to accept such indices (see #11095).

Closes #10215

@rjernst

This comment has been minimized.

Copy link
Member

commented May 9, 2015

Maybe I'm misunderstanding this, but it looks like this check is only on the ES version the index was created with, and not the lucene index? While this will certainly block upgrades that are not possible, it would also fail indexes that were already upgraded (ie don't have any lucene 3.x segments)? As far as I understand, the created version is never modified (certainly not by merging or the upgrade api)?

@imotov

This comment has been minimized.

Copy link
Member Author

commented May 11, 2015

@rjernst great observation! Sorry, I forgot to mention this in the description. I have updated the description.

try {
indexMetaData = metaDataIndexUpgradeService.upgradeIndexMetaData(indexMetaData);
} catch (Exception ex) {
throw new IndexException(new Index(index), "unpgradable index", ex);

This comment has been minimized.

Copy link
@kimchy

kimchy May 14, 2015

Member

can we have a better message here?

This comment has been minimized.

Copy link
@imotov

imotov May 15, 2015

Author Member

@kimchy I will change it to "cannot open the index due to upgrade failure" unless you have better suggestions. The reason why this index cannot be upgraded and possible remedies will be in the inner exception that will be returned as part of the response, so there is probably no reason to repeat them in this exception as well.

@kimchy

This comment has been minimized.

Copy link
Member

commented May 14, 2015

@imotov the code looks ok, but just so I understand how we will work in 2.0. Effectively, any version that has created version before 0.90 will require going through 1.6 for an upgrade, right?

@imotov

This comment has been minimized.

Copy link
Member Author

commented May 14, 2015

@kimchy correct, if a cluster contains at least one index that was created before 0.90 such cluster will have to be upgraded to v1.6 before it can be upgraded to 2.0.

if (indexMetaData.getState() == IndexMetaData.State.OPEN && isSupportedVersion(indexMetaData) == false) {
throw new IllegalStateException("The index [" + indexMetaData.getIndex() + "] was created before v0.90.0 and wasn't upgraded."
+ " This index should be open using a version before " + Version.CURRENT.minimumCompatibilityVersion()
+ " and upgraded using upgrade API.");

This comment has been minimized.

Copy link
@rjernst

rjernst May 15, 2015

Member

upgrade API -> the upgrade API

* Returns true if this index can be supported by the current version of elasticsearch
*/
private static boolean isSupportedVersion(IndexMetaData indexMetaData) {
return indexMetaData.creationVersion() != null && indexMetaData.creationVersion().onOrAfter(Version.V_0_90_0_Beta1);

This comment has been minimized.

Copy link
@rjernst

rjernst May 15, 2015

Member

Maybe add minimumLuceneVersion() to index metadata as a placeholder for #11095? Then the impl can just be createdVersion().luceneVersion in this PR? It would just make it more clear here that the lucene version is the driving factor.

* occurs during cluster upgrade, when dangling indices are imported into the cluster or indices
* are restored from a repository.
*/
public class MetaDataIndexUpgradeService extends AbstractComponent {

This comment has been minimized.

Copy link
@rjernst

rjernst May 15, 2015

Member

I think this name should be IndexMetaDataUpgradeService? Otherwise it sounds like there is a "metadata index".

This comment has been minimized.

Copy link
@imotov

imotov May 15, 2015

Author Member

I was just trying to stay consistent with MetaDataIndexAliasesService, MetaDataIndexStateService, and MetaDataIndexTemplateService.

This comment has been minimized.

Copy link
@rjernst

rjernst May 15, 2015

Member

Ok, didn't know about those...I guess keep for consistency...

@@ -212,58 +213,27 @@ private void ensureNoPre019State() throws Exception {
}

/**
* Elasticsearch 2.0 deprecated custom routing hash functions. So what we do here is that for old indices, we
* move this old & deprecated node setting to an index setting so that we can keep things backward compatible.
* Elasticsearch 2.0 removed several deprecated features and removed support for Lucene 3.0. This method calls

This comment has been minimized.

Copy link
@rjernst

rjernst May 15, 2015

Member

Lucene 3.0 -> Lucene 3.x?

logger.warn("Settings [{}] and [{}] are deprecated. Index settings from your old indices have been updated to record the fact that they "
+ "used some custom routing logic, you can now remove these settings from your `elasticsearch.yml` file", DEPRECATED_SETTING_ROUTING_HASH_FUNCTION, DEPRECATED_SETTING_ROUTING_USE_TYPE);
// We successfully checked all indices for backward compatibility and found no non-upgradable indices, which
// means the upgrade can continue. Now it's save to overwrite index metadata with the new version.

This comment has been minimized.

Copy link
@rjernst

rjernst May 15, 2015

Member

save -> safe

client().admin().indices().prepareOpen(indexName).get();
fail("Shouldn't be able to open an old index");
} catch (IndexException ex) {
assertThat(ex.getMessage(), containsString("unpgradable index"));

This comment has been minimized.

Copy link
@rjernst

rjernst May 15, 2015

Member

Can we not use hamcrest? I've been trying to eliminate it everywhere I see it...

assertTrue(ex.getMessage(), ex.getMessage().contains("unupgradable index");

Also, I agree with Shay's previous comment we should have a better exception message. And I'm not even sure unupgradeable is a word...certainly not unpgradable. :)

@rjernst

This comment has been minimized.

Copy link
Member

commented May 15, 2015

This looks pretty good, I left a few comments.

@imotov

This comment has been minimized.

Copy link
Member Author

commented May 15, 2015

@kimchy, @rjernst I pushed changes addressing your comments.

@rjernst

This comment has been minimized.

Copy link
Member

commented May 15, 2015

LGTM

@clintongormley

This comment has been minimized.

Copy link
Member

commented May 15, 2015

@kimchy correct, if a cluster contains at least one index that was created before 0.90 such cluster will have to be upgraded to v1.6 before it can be upgraded to 2.0.

Or the index can be closed, and have the setting updated manually, without upgrading to 1.6. The migration plugin https://github.com/elastic/elasticsearch-migration/ checks for minimum segment version and checks for the presense of this setting. We could add a link to instructions about how to set it without upgrading

When index is introduced into the cluster via cluster upgrade, restore or as a dangled index the MetaDataIndexUpgradeService checks if this index can be upgraded to the current version. If upgrade is not possible, the newly upgraded cluster startup and restore process are aborted, the dangled index is imported as a closed index that cannot be open.

Closes #10215
@imotov imotov force-pushed the imotov:issue-10215-dont-allow-old-indices branch from dce9a38 to 21ed6bb May 20, 2015
@imotov imotov removed the review label May 20, 2015
@imotov imotov merged commit 21ed6bb into elastic:master May 20, 2015
1 check passed
1 check passed
CLA Commit author is a member of Elasticsearch
Details
imotov added a commit to imotov/elasticsearch that referenced this pull request May 25, 2015
…ble version that the index was upgraded to

In elastic#11072 we are adding a check that will prevent opening of old indices. However, this check doesn't take into consideration the fact that indices can be made compatible with the current version through upgrade API. In order to make compatibility check aware of the upgrade, the upgrade API should write a new setting `index.version.minimum_compatible` that will indicate the minimum compatible version of lucene this index is compatible with and `index.version.upgraded` that will indicate the version of elasticsearch that performed the upgrade.

Closes elastic#11095
imotov added a commit to imotov/elasticsearch that referenced this pull request May 28, 2015
…ble version that the index was upgraded to

In elastic#11072 we are adding a check that will prevent opening of old indices. However, this check doesn't take into consideration the fact that indices can be made compatible with the current version through upgrade API. In order to make compatibility check aware of the upgrade, the upgrade API should write a new setting `index.version.minimum_compatible` that will indicate the minimum compatible version of lucene this index is compatible with and `index.version.upgraded` that will indicate the version of elasticsearch that performed the upgrade.

Closes elastic#11095
imotov added a commit that referenced this pull request May 28, 2015
…ble version that the index was upgraded to

In #11072 we are adding a check that will prevent opening of old indices. However, this check doesn't take into consideration the fact that indices can be made compatible with the current version through upgrade API. In order to make compatibility check aware of the upgrade, the upgrade API should write a new setting `index.version.minimum_compatible` that will indicate the minimum compatible version of lucene this index is compatible with and `index.version.upgraded` that will indicate the version of elasticsearch that performed the upgrade.

Closes #11095
@clintongormley clintongormley changed the title Core: Don't allow indices containing too-old segments to be opened Don't allow indices containing too-old segments to be opened Jun 6, 2015
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.