Skip to content

Commit

Permalink
[ML] Need to tolerate .ml-config being an alias (elastic#80025) (elas…
Browse files Browse the repository at this point in the history 
…tic#80036)

The .ml-config index was introduced in 6.x, and hence may
be reindexed by the system index upgrade code in 7.16.
This in turn means that all our use of the name .ml-config
must tolerate it either being a concrete index or an alias
to some other index that's been created as a replacement
for the original 6.x index.

We were mostly compliant with this need. The only place
where we weren't was in the code for config migration from
cluster state. It's likely that any cluster running 7.16
would already have had configs migrated out of cluster
state. The only way this couldn't be the case is if the
cluster was upgraded from 6.x to 7.16 in a full cluster
restart. Any rolling upgrade would have had to go via
6.8 where config migration could have taken place. But
still, it's best that the patterns in the code reflect
best practice for the future, as .ml-config will need to
be reindexed again as the years go by.
  • Loading branch information
@droberts195
droberts195 authored Oct 28, 2021
1 parent 28d0446 commit f8aec2e
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 13 deletions.
4 changes: 3 additions & 1 deletion ...c/internalClusterTest/java/org/elasticsearch/xpack/ml/integration/MlConfigMigratorIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -424,15 +424,17 @@ public void assertSnapshot(MlMetadata expectedMlMetadata) throws IOException {
}

private void addMlConfigIndex(Metadata.Builder metadata, RoutingTable.Builder routingTable) {
final String uuid = "_uuid";
IndexMetadata.Builder indexMetadata = IndexMetadata.builder(MlConfigIndex.indexName());
indexMetadata.settings(
Settings.builder()
.put(IndexMetadata.SETTING_INDEX_UUID, uuid)
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
);
metadata.put(indexMetadata);
Index index = new Index(MlConfigIndex.indexName(), "_uuid");
Index index = new Index(MlConfigIndex.indexName(), uuid);
ShardId shardId = new ShardId(index, 0);
ShardRouting shardRouting = ShardRouting.newUnassigned(
shardId,
Expand Down
11 changes: 5 additions & 6 deletions ...plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlConfigMigrationEligibilityCheck.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
package org.elasticsearch.xpack.ml;

import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.metadata.IndexAbstraction;
import org.elasticsearch.cluster.routing.IndexRoutingTable;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.settings.Setting;
Expand Down Expand Up @@ -57,15 +58,13 @@ public boolean canStartMigration(ClusterState clusterState) {
}

static boolean mlConfigIndexIsAllocated(ClusterState clusterState) {
if (clusterState.metadata().hasIndex(MlConfigIndex.indexName()) == false) {
IndexAbstraction configIndexOrAlias = clusterState.metadata().getIndicesLookup().get(MlConfigIndex.indexName());
if (configIndexOrAlias == null) {
return false;
}

IndexRoutingTable routingTable = clusterState.getRoutingTable().index(MlConfigIndex.indexName());
if (routingTable == null || routingTable.allPrimaryShardsActive() == false) {
return false;
}
return true;
IndexRoutingTable routingTable = clusterState.getRoutingTable().index(configIndexOrAlias.getWriteIndex());
return routingTable != null && routingTable.allPrimaryShardsActive();
}

/**
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlConfigMigrator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ public void migrateConfigs(ClusterState clusterState, ActionListener<Boolean> li
return;
}

if (clusterState.metadata().hasIndex(MlConfigIndex.indexName()) == false) {
if (clusterState.metadata().hasIndexAbstraction(MlConfigIndex.indexName()) == false) {
createConfigIndex(
ActionListener.wrap(
response -> { unMarkMigrationInProgress.onResponse(Boolean.FALSE); },
Expand Down
38 changes: 35 additions & 3 deletions ...n/ml/src/test/java/org/elasticsearch/xpack/ml/MlConfigMigrationEligibilityCheckTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import org.elasticsearch.Version;
import org.elasticsearch.cluster.ClusterName;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.metadata.AliasMetadata;
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.cluster.metadata.Metadata;
import org.elasticsearch.cluster.routing.IndexRoutingTable;
Expand Down Expand Up @@ -77,6 +78,24 @@ public void testCanStartMigration_givenMissingIndex() {
assertFalse(check.canStartMigration(clusterState));
}

public void testCanStartMigration_givenMlConfigIsAlias() {
Settings settings = newSettings(true);
givenClusterSettings(settings);

// index has been replaced by an alias
String reindexedName = ".reindexed_ml_config";
Metadata.Builder metadata = Metadata.builder();
RoutingTable.Builder routingTable = RoutingTable.builder();
addMlConfigIndex(reindexedName, MlConfigIndex.indexName(), metadata, routingTable);
ClusterState clusterState = ClusterState.builder(new ClusterName("migratortests"))
.metadata(metadata)
.routingTable(routingTable.build())
.build();

MlConfigMigrationEligibilityCheck check = new MlConfigMigrationEligibilityCheck(settings, clusterService);
assertTrue(check.canStartMigration(clusterState));
}

public void testCanStartMigration_givenInactiveShards() {
Settings settings = newSettings(true);
givenClusterSettings(settings);
Expand All @@ -85,22 +104,35 @@ public void testCanStartMigration_givenInactiveShards() {
Metadata.Builder metadata = Metadata.builder();
RoutingTable.Builder routingTable = RoutingTable.builder();
addMlConfigIndex(metadata, routingTable);
ClusterState clusterState = ClusterState.builder(new ClusterName("migratortests")).metadata(metadata).build();
ClusterState clusterState = ClusterState.builder(new ClusterName("migratortests"))
.metadata(metadata)
// the difference here is that the routing table that's been created is
// _not_ added to the cluster state, simulating no route to the index
.build();

MlConfigMigrationEligibilityCheck check = new MlConfigMigrationEligibilityCheck(settings, clusterService);
assertFalse(check.canStartMigration(clusterState));
}

private void addMlConfigIndex(Metadata.Builder metadata, RoutingTable.Builder routingTable) {
IndexMetadata.Builder indexMetadata = IndexMetadata.builder(MlConfigIndex.indexName());
addMlConfigIndex(MlConfigIndex.indexName(), null, metadata, routingTable);
}

private void addMlConfigIndex(String indexName, String aliasName, Metadata.Builder metadata, RoutingTable.Builder routingTable) {
final String uuid = "_uuid";
IndexMetadata.Builder indexMetadata = IndexMetadata.builder(indexName);
indexMetadata.settings(
Settings.builder()
.put(IndexMetadata.SETTING_INDEX_UUID, uuid)
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
);
if (aliasName != null) {
indexMetadata.putAlias(AliasMetadata.builder(aliasName));
}
metadata.put(indexMetadata);
Index index = new Index(MlConfigIndex.indexName(), "_uuid");
Index index = new Index(indexName, uuid);
ShardId shardId = new ShardId(index, 0);
ShardRouting shardRouting = ShardRouting.newUnassigned(
shardId,
Expand Down
6 changes: 4 additions & 2 deletions x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/JobManagerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -470,15 +470,17 @@ public void testUpdateJob_notAllowedPreMigration() {
Metadata.Builder metadata = Metadata.builder();
RoutingTable.Builder routingTable = RoutingTable.builder();

final String uuid = "_uuid";
IndexMetadata.Builder indexMetadata = IndexMetadata.builder(MlConfigIndex.indexName());
indexMetadata.settings(
Settings.builder()
.put(IndexMetadata.SETTING_INDEX_UUID, uuid)
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
);
metadata.put(indexMetadata);
Index index = new Index(MlConfigIndex.indexName(), "_uuid");
Index index = new Index(MlConfigIndex.indexName(), uuid);
ShardId shardId = new ShardId(index, 0);
ShardRouting shardRouting = ShardRouting.newUnassigned(
shardId,
Expand All @@ -503,7 +505,7 @@ public void testUpdateJob_notAllowedPreMigration() {
new UpdateJobAction.Request("closed-job-not-migrated", null),
ActionListener.wrap(
response -> fail("response not expected: " + response),
exception -> { assertThat(exception, instanceOf(ElasticsearchStatusException.class)); }
exception -> assertThat(exception, instanceOf(ElasticsearchStatusException.class))
)
);

Expand Down

0 comments on commit f8aec2e

Please sign in to comment.