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

Add validation for supported index version on node join, restore, upgrade & open index #21830

Merged
merged 12 commits into from
Dec 1, 2016
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.elasticsearch.cluster.metadata;

import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.Version;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.admin.indices.close.CloseIndexClusterStateUpdateRequest;
import org.elasticsearch.action.admin.indices.open.OpenIndexClusterStateUpdateRequest;
Expand Down Expand Up @@ -160,12 +161,14 @@ public ClusterState execute(ClusterState currentState) {
MetaData.Builder mdBuilder = MetaData.builder(currentState.metaData());
ClusterBlocks.Builder blocksBuilder = ClusterBlocks.builder()
.blocks(currentState.blocks());
Version minIndexCompatibilityVersion = currentState.getNodes().getSmallestNonClientNodeVersion()
Copy link
Contributor

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) ?

Copy link
Contributor

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

Copy link
Contributor Author

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

Copy link
Contributor Author

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yikes. Of course.

.minimumIndexCompatibilityVersion();
for (IndexMetaData closedMetaData : indicesToOpen) {
final String indexName = closedMetaData.getIndex().getName();
IndexMetaData indexMetaData = IndexMetaData.builder(closedMetaData).state(IndexMetaData.State.OPEN).build();
// The index might be closed because we couldn't import it due to old incompatible version
// We need to check that this index can be upgraded to the current version
indexMetaData = metaDataIndexUpgradeService.upgradeIndexMetaData(indexMetaData);
indexMetaData = metaDataIndexUpgradeService.upgradeIndexMetaData(indexMetaData, minIndexCompatibilityVersion);
try {
indicesService.verifyIndexMetadata(indexMetaData, indexMetaData);
} catch (Exception e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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."
Copy link
Contributor

Choose a reason for hiding this comment

The 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);
}

/**
Expand Down Expand Up @@ -173,4 +174,4 @@ IndexMetaData archiveBrokenIndexSettings(IndexMetaData indexMetaData) {
return indexMetaData;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,8 @@ static MetaData upgradeMetaData(MetaData metaData,
boolean changed = false;
final MetaData.Builder upgradedMetaData = MetaData.builder(metaData);
for (IndexMetaData indexMetaData : metaData) {
IndexMetaData newMetaData = metaDataIndexUpgradeService.upgradeIndexMetaData(indexMetaData);
IndexMetaData newMetaData = metaDataIndexUpgradeService.upgradeIndexMetaData(indexMetaData,
Version.CURRENT.minimumIndexCompatibilityVersion());
changed |= indexMetaData != newMetaData;
upgradedMetaData.put(newMetaData, false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,8 @@ public ClusterState execute(ClusterState currentState) {
try {
// The dangled index might be from an older version, we need to make sure it's compatible
// with the current version and upgrade it if needed.
upgradedIndexMetaData = metaDataIndexUpgradeService.upgradeIndexMetaData(indexMetaData);
upgradedIndexMetaData = metaDataIndexUpgradeService.upgradeIndexMetaData(indexMetaData,
minIndexCompatibilityVersion);
} catch (Exception ex) {
// upgrade failed - adding index as closed
logger.warn((Supplier<?>) () -> new ParameterizedMessage("found dangled index [{}] on node [{}]. This index cannot be upgraded to the latest version, adding as closed", indexMetaData.getIndex(), request.fromNode), ex);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
import java.util.Set;
import java.util.stream.Collectors;

import static java.util.Collections.min;
import static java.util.Collections.unmodifiableSet;
import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_AUTO_EXPAND_REPLICAS;
import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_CREATION_DATE;
Expand Down Expand Up @@ -225,6 +226,8 @@ public ClusterState execute(ClusterState currentState) {
if (!renamedIndices.isEmpty()) {
// We have some indices to restore
ImmutableOpenMap.Builder<ShardId, RestoreInProgress.ShardRestoreStatus> shardsBuilder = ImmutableOpenMap.builder();
Version minIndexCompatibilityVersion = currentState.getNodes().getSmallestNonClientNodeVersion()
.minimumIndexCompatibilityVersion();
for (Map.Entry<String, String> indexEntry : renamedIndices.entrySet()) {
String index = indexEntry.getValue();
boolean partial = checkPartial(index);
Expand All @@ -233,7 +236,8 @@ public ClusterState execute(ClusterState currentState) {
IndexMetaData snapshotIndexMetaData = metaData.index(index);
snapshotIndexMetaData = updateIndexSettings(snapshotIndexMetaData, request.indexSettings, request.ignoreIndexSettings);
try {
snapshotIndexMetaData = metaDataIndexUpgradeService.upgradeIndexMetaData(snapshotIndexMetaData);
snapshotIndexMetaData = metaDataIndexUpgradeService.upgradeIndexMetaData(snapshotIndexMetaData,
minIndexCompatibilityVersion);
} catch (Exception ex) {
throw new SnapshotRestoreException(snapshot, "cannot restore index [" + index + "] because it cannot be upgraded", ex);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public void testRestoreOldSnapshots() throws Exception {
for (Version v : VersionUtils.allReleasedVersions()) {
if (VersionUtils.isSnapshot(v)) continue; // snapshots are unreleased, so there is no backcompat yet
if (v.isRelease() == false) continue; // no guarantees for prereleases
if (v.onOrBefore(Version.V_2_0_0_beta1)) continue; // we can only test back one major lucene version
if (v.onOrBefore(Version.V_5_0_0_beta1)) continue; // we can only test back one major lucene version
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering - we didn't you go with Version.CURRENT.minimumIndexCompatibilityVersion() here?

if (v.equals(Version.CURRENT)) continue; // the current version is always compatible with itself
expectedVersions.add(v.toString());
}
Expand Down Expand Up @@ -128,44 +128,6 @@ public void testRestoreUnsupportedSnapshots() throws Exception {
}
}

public void testRestoreSnapshotWithMissingChecksum() throws Exception {
final String repo = "test_repo";
final String snapshot = "test_1";
final String indexName = "index-2.3.4";
final String repoFileId = "missing-checksum-repo-2.3.4";
Path repoFile = getBwcIndicesPath().resolve(repoFileId + ".zip");
URI repoFileUri = repoFile.toUri();
URI repoJarUri = new URI("jar:" + repoFileUri.toString() + "!/repo/");
logger.info("--> creating repository [{}] for repo file [{}]", repo, repoFileId);
assertAcked(client().admin().cluster().preparePutRepository(repo)
.setType("url")
.setSettings(Settings.builder().put("url", repoJarUri.toString())));

logger.info("--> get snapshot and check its indices");
GetSnapshotsResponse getSnapshotsResponse = client().admin().cluster().prepareGetSnapshots(repo).setSnapshots(snapshot).get();
assertThat(getSnapshotsResponse.getSnapshots().size(), equalTo(1));
SnapshotInfo snapshotInfo = getSnapshotsResponse.getSnapshots().get(0);
assertThat(snapshotInfo.indices(), equalTo(Arrays.asList(indexName)));

logger.info("--> restoring snapshot");
RestoreSnapshotResponse response = client().admin().cluster().prepareRestoreSnapshot(repo, snapshot).setRestoreGlobalState(true).setWaitForCompletion(true).get();
assertThat(response.status(), equalTo(RestStatus.OK));
RestoreInfo restoreInfo = response.getRestoreInfo();
assertThat(restoreInfo.successfulShards(), greaterThan(0));
assertThat(restoreInfo.successfulShards(), equalTo(restoreInfo.totalShards()));
assertThat(restoreInfo.failedShards(), equalTo(0));
String index = restoreInfo.indices().get(0);
assertThat(index, equalTo(indexName));

logger.info("--> check search");
SearchResponse searchResponse = client().prepareSearch(index).get();
assertThat(searchResponse.getHits().totalHits(), greaterThan(0L));

logger.info("--> cleanup");
cluster().wipeIndices(restoreInfo.indices().toArray(new String[restoreInfo.indices().size()]));
cluster().wipeTemplates();
}

private List<String> repoVersions() throws Exception {
return listRepoVersions("repo");
}
Expand Down Expand Up @@ -245,7 +207,7 @@ private void assertUnsupportedIndexFailsToRestore(String repo, String snapshot)
logger.info("--> restoring unsupported snapshot");
try {
client().admin().cluster().prepareRestoreSnapshot(repo, snapshot).setRestoreGlobalState(true).setWaitForCompletion(true).get();
fail("should have failed to restore");
fail("should have failed to restore - " + repo);
} catch (SnapshotRestoreException ex) {
assertThat(ex.getMessage(), containsString("cannot restore index"));
assertThat(ex.getMessage(), containsString("because it cannot be upgraded"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,11 @@ public void testUpgrade() {
Collections.emptyMap()), IndexScopedSettings.DEFAULT_SCOPED_SETTINGS);
IndexMetaData src = newIndexMeta("foo", Settings.builder().put("index.refresh_interval", "-200").build());
assertFalse(service.isUpgraded(src));
src = service.upgradeIndexMetaData(src);
src = service.upgradeIndexMetaData(src, Version.CURRENT.minimumIndexCompatibilityVersion());
assertTrue(service.isUpgraded(src));
assertEquals("-200", src.getSettings().get("archived.index.refresh_interval"));
assertNull(src.getSettings().get("index.refresh_interval"));
assertSame(src, service.upgradeIndexMetaData(src)); // no double upgrade
assertSame(src, service.upgradeIndexMetaData(src, Version.CURRENT.minimumIndexCompatibilityVersion())); // no double upgrade
}

public void testIsUpgraded() {
Expand All @@ -87,7 +87,8 @@ public void testFailUpgrade() {
.put(IndexMetaData.SETTING_VERSION_CREATED, Version.fromString("1.7.0"))
.put(IndexMetaData.SETTING_VERSION_MINIMUM_COMPATIBLE,
Version.CURRENT.luceneVersion.toString()).build());
String message = expectThrows(IllegalStateException.class, () -> service.upgradeIndexMetaData(metaData)).getMessage();
String message = expectThrows(IllegalStateException.class, () -> service.upgradeIndexMetaData(metaData,
Version.V_5_0_0.minimumIndexCompatibilityVersion())).getMessage();
assertEquals(message, "The index [[foo/BOOM]] was created before v2.0.0.beta1. It should be reindexed in Elasticsearch 2.x " +
"before upgrading to " + Version.CURRENT.toString() + ".");

Expand All @@ -96,7 +97,7 @@ public void testFailUpgrade() {
.put(IndexMetaData.SETTING_VERSION_CREATED, Version.fromString("2.1.0"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Should we bump these to realistic current versions? i.e., 5.0.0 for index created and Version.V_6_0_0. minimumIndexCompatibilityVersion

.put(IndexMetaData.SETTING_VERSION_MINIMUM_COMPATIBLE,
Version.CURRENT.luceneVersion.toString()).build());
service.upgradeIndexMetaData(goodMeta);
service.upgradeIndexMetaData(goodMeta, Version.V_5_0_0.minimumIndexCompatibilityVersion());
}

public static IndexMetaData newIndexMeta(String name, Settings indexSettings) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ public MockMetaDataIndexUpgradeService(boolean upgrade) {
this.upgrade = upgrade;
}
@Override
public IndexMetaData upgradeIndexMetaData(IndexMetaData indexMetaData) {
public IndexMetaData upgradeIndexMetaData(IndexMetaData indexMetaData, Version minimumIndexCompatibilityVersion) {
return upgrade ? IndexMetaData.builder(indexMetaData).build() : indexMetaData;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.elasticsearch.indices.cluster;

import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.Version;
import org.elasticsearch.action.ActionResponse;
import org.elasticsearch.action.admin.cluster.reroute.ClusterRerouteRequest;
import org.elasticsearch.action.admin.cluster.reroute.TransportClusterRerouteAction;
Expand Down Expand Up @@ -158,7 +159,7 @@ public ClusterStateChanges() {
MetaDataIndexUpgradeService metaDataIndexUpgradeService = new MetaDataIndexUpgradeService(settings, null, null) {
// metaData upgrader should do nothing
@Override
public IndexMetaData upgradeIndexMetaData(IndexMetaData indexMetaData) {
public IndexMetaData upgradeIndexMetaData(IndexMetaData indexMetaData, Version minimumIndexCompatibilityVersion) {
return indexMetaData;
}
};
Expand Down
Binary file not shown.