diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/annotations/AnnotationIndex.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/annotations/AnnotationIndex.java index 2d2d65ee4bde2..f0b9320a34274 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/annotations/AnnotationIndex.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/annotations/AnnotationIndex.java @@ -107,15 +107,21 @@ public static void createAnnotationsIndexIfNecessary( finalListener::onFailure ); - final ActionListener createAliasListener = ActionListener.wrap(success -> { + final ActionListener createAliasListener = ActionListener.wrap(currentIndexName -> { final IndicesAliasesRequestBuilder requestBuilder = client.admin() .indices() .prepareAliases() - .addAliasAction(IndicesAliasesRequest.AliasActions.add().index(LATEST_INDEX_NAME).alias(READ_ALIAS_NAME).isHidden(true)) - .addAliasAction(IndicesAliasesRequest.AliasActions.add().index(LATEST_INDEX_NAME).alias(WRITE_ALIAS_NAME).isHidden(true)); + .addAliasAction(IndicesAliasesRequest.AliasActions.add().index(currentIndexName).alias(READ_ALIAS_NAME).isHidden(true)) + .addAliasAction(IndicesAliasesRequest.AliasActions.add().index(currentIndexName).alias(WRITE_ALIAS_NAME).isHidden(true)); + SortedMap lookup = state.getMetadata().getIndicesLookup(); for (String oldIndexName : OLD_INDEX_NAMES) { - if (state.getMetadata().hasIndexAbstraction(oldIndexName)) { - requestBuilder.removeAlias(oldIndexName, WRITE_ALIAS_NAME); + IndexAbstraction oldIndexAbstraction = lookup.get(oldIndexName); + if (oldIndexAbstraction != null) { + // The old index might no longer be an index - that index could have been reindexed + // with the old index name now being an alias to that reindexed index. + for (Index oldIndex : oldIndexAbstraction.getIndices()) { + requestBuilder.removeAlias(oldIndex.getName(), WRITE_ALIAS_NAME); + } } } executeAsyncWithOrigin( @@ -140,7 +146,8 @@ public static void createAnnotationsIndexIfNecessary( && mlLookup.firstKey().startsWith(".ml")) { // Create the annotations index if it doesn't exist already. - if (mlLookup.containsKey(LATEST_INDEX_NAME) == false) { + IndexAbstraction currentIndexAbstraction = mlLookup.get(LATEST_INDEX_NAME); + if (currentIndexAbstraction == null) { logger.debug( () -> new ParameterizedMessage( "Creating [{}] because [{}] exists; trace {}", @@ -162,12 +169,12 @@ public static void createAnnotationsIndexIfNecessary( client.threadPool().getThreadContext(), ML_ORIGIN, createIndexRequest, - ActionListener.wrap(r -> createAliasListener.onResponse(r.isAcknowledged()), e -> { + ActionListener.wrap(r -> createAliasListener.onResponse(LATEST_INDEX_NAME), e -> { // Possible that the index was created while the request was executing, // so we need to handle that possibility if (ExceptionsHelper.unwrapCause(e) instanceof ResourceAlreadyExistsException) { // Create the alias - createAliasListener.onResponse(true); + createAliasListener.onResponse(LATEST_INDEX_NAME); } else { finalListener.onFailure(e); } @@ -177,16 +184,20 @@ public static void createAnnotationsIndexIfNecessary( return; } + // Account for the possibility that the latest index has been reindexed + // into a new index with the latest index name as an alias. + String currentIndexName = currentIndexAbstraction.getIndices().get(0).getName(); + // Recreate the aliases if they've gone even though the index still exists. - IndexAbstraction writeAliasDefinition = mlLookup.get(WRITE_ALIAS_NAME); - if (mlLookup.containsKey(READ_ALIAS_NAME) == false || writeAliasDefinition == null) { - createAliasListener.onResponse(true); + IndexAbstraction writeAliasAbstraction = mlLookup.get(WRITE_ALIAS_NAME); + if (mlLookup.containsKey(READ_ALIAS_NAME) == false || writeAliasAbstraction == null) { + createAliasListener.onResponse(currentIndexName); return; } - List writeAliasIndices = writeAliasDefinition.getIndices(); - if (writeAliasIndices.size() != 1 || LATEST_INDEX_NAME.equals(writeAliasIndices.get(0).getName()) == false) { - createAliasListener.onResponse(true); + List writeAliasIndices = writeAliasAbstraction.getIndices(); + if (writeAliasIndices.size() != 1 || currentIndexName.equals(writeAliasIndices.get(0).getName()) == false) { + createAliasListener.onResponse(currentIndexName); return; } diff --git a/x-pack/plugin/ml/src/internalClusterTest/java/org/elasticsearch/xpack/ml/integration/AnnotationIndexIT.java b/x-pack/plugin/ml/src/internalClusterTest/java/org/elasticsearch/xpack/ml/integration/AnnotationIndexIT.java index 2574fac8b8e9d..5609ecd6151b9 100644 --- a/x-pack/plugin/ml/src/internalClusterTest/java/org/elasticsearch/xpack/ml/integration/AnnotationIndexIT.java +++ b/x-pack/plugin/ml/src/internalClusterTest/java/org/elasticsearch/xpack/ml/integration/AnnotationIndexIT.java @@ -9,8 +9,11 @@ import com.carrotsearch.hppc.cursors.ObjectObjectCursor; import org.elasticsearch.action.admin.indices.alias.Alias; +import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest; +import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequestBuilder; import org.elasticsearch.action.admin.indices.create.CreateIndexAction; import org.elasticsearch.action.admin.indices.create.CreateIndexRequest; +import org.elasticsearch.action.admin.indices.get.GetIndexResponse; import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.action.index.IndexResponse; import org.elasticsearch.action.search.SearchPhaseExecutionException; @@ -23,7 +26,6 @@ import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.rest.RestStatus; -import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.xpack.core.XPackSettings; import org.elasticsearch.xpack.core.action.SetResetModeActionRequest; import org.elasticsearch.xpack.core.ml.action.SetResetModeAction; @@ -33,6 +35,7 @@ import org.elasticsearch.xpack.ml.notifications.AnomalyDetectionAuditor; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -54,22 +57,99 @@ public void testNotCreatedWhenNoOtherMlIndices() { // Ask a few times to increase the chance of failure if the .ml-annotations index is created when no other ML index exists for (int i = 0; i < 10; ++i) { - assertFalse(annotationsIndexExists()); + assertFalse(annotationsIndexExists(AnnotationIndex.LATEST_INDEX_NAME)); assertEquals(0, numberOfAnnotationsAliases()); } } public void testCreatedWhenAfterOtherMlIndex() throws Exception { - AnomalyDetectionAuditor auditor = new AnomalyDetectionAuditor(client(), getInstanceFromNode(ClusterService.class)); - auditor.info("whatever", "blah"); + // Creating a document in the .ml-notifications-000002 index should cause .ml-annotations + // to be created, as it should get created as soon as any other ML index exists + createAnnotation(); + + assertBusy(() -> { + assertTrue(annotationsIndexExists(AnnotationIndex.LATEST_INDEX_NAME)); + assertEquals(2, numberOfAnnotationsAliases()); + }); + } + public void testReindexing() throws Exception { // Creating a document in the .ml-notifications-000002 index should cause .ml-annotations // to be created, as it should get created as soon as any other ML index exists + createAnnotation(); assertBusy(() -> { - assertTrue(annotationsIndexExists()); + assertTrue(annotationsIndexExists(AnnotationIndex.LATEST_INDEX_NAME)); assertEquals(2, numberOfAnnotationsAliases()); }); + + client().execute(SetUpgradeModeAction.INSTANCE, new SetUpgradeModeAction.Request(true)).actionGet(); + + String reindexedIndexName = ".reindexed-v7-ml-annotations-6"; + createReindexedIndex(reindexedIndexName); + + IndicesAliasesRequestBuilder indicesAliasesRequestBuilder = client().admin() + .indices() + .prepareAliases() + .addAliasAction( + IndicesAliasesRequest.AliasActions.add().index(reindexedIndexName).alias(AnnotationIndex.READ_ALIAS_NAME).isHidden(true) + ) + .addAliasAction( + IndicesAliasesRequest.AliasActions.add().index(reindexedIndexName).alias(AnnotationIndex.WRITE_ALIAS_NAME).isHidden(true) + ) + .addAliasAction(IndicesAliasesRequest.AliasActions.removeIndex().index(AnnotationIndex.LATEST_INDEX_NAME)) + .addAliasAction( + IndicesAliasesRequest.AliasActions.add().index(reindexedIndexName).alias(AnnotationIndex.LATEST_INDEX_NAME).isHidden(true) + ); + + client().admin().indices().aliases(indicesAliasesRequestBuilder.request()).actionGet(); + + client().execute(SetUpgradeModeAction.INSTANCE, new SetUpgradeModeAction.Request(false)).actionGet(); + + // Ask a few times to increase the chance of failure if the .ml-annotations index is created when no other ML index exists + for (int i = 0; i < 10; ++i) { + assertFalse(annotationsIndexExists(AnnotationIndex.LATEST_INDEX_NAME)); + assertTrue(annotationsIndexExists(reindexedIndexName)); + // Aliases should be read, write and original name + assertEquals(3, numberOfAnnotationsAliases()); + } + } + + public void testReindexingWithLostAliases() throws Exception { + // Creating a document in the .ml-notifications-000002 index should cause .ml-annotations + // to be created, as it should get created as soon as any other ML index exists + createAnnotation(); + + assertBusy(() -> { + assertTrue(annotationsIndexExists(AnnotationIndex.LATEST_INDEX_NAME)); + assertEquals(2, numberOfAnnotationsAliases()); + }); + + client().execute(SetUpgradeModeAction.INSTANCE, new SetUpgradeModeAction.Request(true)).actionGet(); + + String reindexedIndexName = ".reindexed-v7-ml-annotations-6"; + createReindexedIndex(reindexedIndexName); + + IndicesAliasesRequestBuilder indicesAliasesRequestBuilder = client().admin() + .indices() + .prepareAliases() + // The difference compared to the standard reindexing test is that the read and write aliases are not correctly set up. + // The annotations index maintenance code should add them back. + .addAliasAction(IndicesAliasesRequest.AliasActions.removeIndex().index(AnnotationIndex.LATEST_INDEX_NAME)) + .addAliasAction( + IndicesAliasesRequest.AliasActions.add().index(reindexedIndexName).alias(AnnotationIndex.LATEST_INDEX_NAME).isHidden(true) + ); + + client().admin().indices().aliases(indicesAliasesRequestBuilder.request()).actionGet(); + + client().execute(SetUpgradeModeAction.INSTANCE, new SetUpgradeModeAction.Request(false)).actionGet(); + + assertBusy(() -> { + assertFalse(annotationsIndexExists(AnnotationIndex.LATEST_INDEX_NAME)); + assertTrue(annotationsIndexExists(reindexedIndexName)); + // Aliases should be read, write and original name + assertEquals(3, numberOfAnnotationsAliases()); + }); } public void testAliasesMovedFromOldToNew() throws Exception { @@ -91,7 +171,7 @@ public void testAliasesMovedFromOldToNew() throws Exception { // When this happens the read alias should be changed to cover both indices, and the write alias should be // switched to only point at the new index. assertBusy(() -> { - assertTrue(annotationsIndexExists()); + assertTrue(annotationsIndexExists(AnnotationIndex.LATEST_INDEX_NAME)); ImmutableOpenMap> aliases = client().admin() .indices() .prepareGetAliases(AnnotationIndex.READ_ALIAS_NAME, AnnotationIndex.WRITE_ALIAS_NAME) @@ -124,11 +204,9 @@ public void testNotCreatedWhenAfterOtherMlIndexAndUpgradeInProgress() throws Exc client().execute(SetUpgradeModeAction.INSTANCE, new SetUpgradeModeAction.Request(true)).actionGet(); try { - AnomalyDetectionAuditor auditor = new AnomalyDetectionAuditor(client(), getInstanceFromNode(ClusterService.class)); - auditor.info("whatever", "blah"); - // Creating a document in the .ml-notifications-000002 index would normally cause .ml-annotations // to be created, but in this case it shouldn't as we're doing an upgrade + createAnnotation(); assertBusy(() -> { try { @@ -137,7 +215,7 @@ public void testNotCreatedWhenAfterOtherMlIndexAndUpgradeInProgress() throws Exc } catch (SearchPhaseExecutionException e) { throw new AssertionError("Notifications index exists but shards not yet ready - continuing busy wait", e); } - assertFalse(annotationsIndexExists()); + assertFalse(annotationsIndexExists(AnnotationIndex.LATEST_INDEX_NAME)); assertEquals(0, numberOfAnnotationsAliases()); }); } finally { @@ -162,7 +240,7 @@ public void testNotCreatedWhenAfterOtherMlIndexAndResetInProgress() throws Excep assertBusy(() -> { SearchResponse response = client().search(new SearchRequest(".ml-state")).actionGet(); assertEquals(1, response.getHits().getHits().length); - assertFalse(annotationsIndexExists()); + assertFalse(annotationsIndexExists(AnnotationIndex.LATEST_INDEX_NAME)); assertEquals(0, numberOfAnnotationsAliases()); }); } finally { @@ -170,15 +248,22 @@ public void testNotCreatedWhenAfterOtherMlIndexAndResetInProgress() throws Excep } } - private boolean annotationsIndexExists() { - return ESIntegTestCase.indexExists(AnnotationIndex.LATEST_INDEX_NAME, client()); + private boolean annotationsIndexExists(String expectedName) { + GetIndexResponse getIndexResponse = client().admin() + .indices() + .prepareGetIndex() + .setIndices(AnnotationIndex.LATEST_INDEX_NAME) + .setIndicesOptions(IndicesOptions.LENIENT_EXPAND_OPEN) + .execute() + .actionGet(); + return Arrays.asList(getIndexResponse.getIndices()).contains(expectedName); } private int numberOfAnnotationsAliases() { int count = 0; ImmutableOpenMap> aliases = client().admin() .indices() - .prepareGetAliases(AnnotationIndex.READ_ALIAS_NAME, AnnotationIndex.WRITE_ALIAS_NAME) + .prepareGetAliases(AnnotationIndex.READ_ALIAS_NAME, AnnotationIndex.WRITE_ALIAS_NAME, AnnotationIndex.LATEST_INDEX_NAME) .setIndicesOptions(IndicesOptions.LENIENT_EXPAND_OPEN_CLOSED_HIDDEN) .get() .getAliases(); @@ -192,4 +277,24 @@ private int numberOfAnnotationsAliases() { } return count; } + + private void createReindexedIndex(String reindexedIndexName) { + CreateIndexRequest createIndexRequest = new CreateIndexRequest(reindexedIndexName).mapping(AnnotationIndex.annotationsMapping()) + .settings( + Settings.builder() + .put(IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS, "0-1") + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, "1") + .put(IndexMetadata.SETTING_INDEX_HIDDEN, true) + ); + + client().admin().indices().create(createIndexRequest).actionGet(); + + // At this point the upgrade assistant would reindex the old index into the new index but there's + // no point in this test as there's nothing in the old index. + } + + private void createAnnotation() { + AnomalyDetectionAuditor auditor = new AnomalyDetectionAuditor(client(), getInstanceFromNode(ClusterService.class)); + auditor.info("whatever", "blah"); + } } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MlSingleNodeTestCase.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MlSingleNodeTestCase.java index 2228e9670f8b6..273db19bf3e8b 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MlSingleNodeTestCase.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MlSingleNodeTestCase.java @@ -13,6 +13,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.core.TimeValue; +import org.elasticsearch.index.mapper.extras.MapperExtrasPlugin; import org.elasticsearch.ingest.common.IngestCommonPlugin; import org.elasticsearch.license.LicenseService; import org.elasticsearch.plugins.Plugin; @@ -96,7 +97,9 @@ protected Collection> getPlugins() { IngestCommonPlugin.class, MockPainlessScriptEngine.TestPlugin.class, // ILM is required for .ml-state template index settings - IndexLifecycle.class + IndexLifecycle.class, + // Needed for scaled_float + MapperExtrasPlugin.class ); }