Revert "[ML] Use a new annotations index for future annotations (#79151…

…)" This reverts commit 7d59540.
elastic · Oct 19, 2021 · 5402f4c · 5402f4c
1 parent 7d59540
commit 5402f4c
Show file tree

Hide file tree

Showing 8 changed files with 31 additions and 104 deletions.
diff --git a/...lugin/core/src/main/java/org/elasticsearch/xpack/core/ml/annotations/AnnotationIndex.java b/...lugin/core/src/main/java/org/elasticsearch/xpack/core/ml/annotations/AnnotationIndex.java
@@ -15,7 +15,6 @@
 import org.elasticsearch.action.admin.cluster.health.ClusterHealthRequest;
 import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse;
 import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest;
-import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequestBuilder;
 import org.elasticsearch.action.admin.indices.create.CreateIndexRequest;
 import org.elasticsearch.action.admin.indices.create.CreateIndexResponse;
 import org.elasticsearch.action.support.master.AcknowledgedResponse;
@@ -31,7 +30,6 @@
 import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper;
 import org.elasticsearch.xpack.core.template.TemplateUtils;
 
-import java.util.List;
 import java.util.SortedMap;
 
 import static org.elasticsearch.xpack.core.ClientHelper.ML_ORIGIN;
@@ -43,15 +41,8 @@ public class AnnotationIndex {
 
     public static final String READ_ALIAS_NAME = ".ml-annotations-read";
     public static final String WRITE_ALIAS_NAME = ".ml-annotations-write";
-
-    // Exposed for testing, but always use the aliases in non-test code.
-    public static final String LATEST_INDEX_NAME = ".ml-annotations-000001";
-    // Due to historical bugs this index may not have the correct mappings
-    // in some production clusters. Therefore new annotations should be
-    // written to the latest index. If we ever switch to another new annotations
-    // index then this list should be adjusted to include the previous latest
-    // index.
-    public static final List<String> OLD_INDEX_NAMES = List.of(".ml-annotations-6");
+    // Exposed for testing, but always use the aliases in non-test code
+    public static final String INDEX_NAME = ".ml-annotations-6";
 
     private static final String MAPPINGS_VERSION_VARIABLE = "xpack.ml.version";
 
@@ -94,18 +85,12 @@ public static void createAnnotationsIndexIfNecessary(Client client, ClusterState
             finalListener::onFailure);
 
         final ActionListener<Boolean> createAliasListener = ActionListener.wrap(success -> {
-            final IndicesAliasesRequestBuilder requestBuilder =
+            final IndicesAliasesRequest request =
                 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));
-            for (String oldIndexName : OLD_INDEX_NAMES) {
-                if (state.getMetadata().getIndicesLookup().containsKey(oldIndexName)) {
-                    requestBuilder.removeAlias(oldIndexName, WRITE_ALIAS_NAME);
-                }
-            }
-            executeAsyncWithOrigin(client.threadPool().getThreadContext(), ML_ORIGIN, requestBuilder.request(),
+                    .addAliasAction(IndicesAliasesRequest.AliasActions.add().index(INDEX_NAME).alias(READ_ALIAS_NAME).isHidden(true))
+                    .addAliasAction(IndicesAliasesRequest.AliasActions.add().index(INDEX_NAME).alias(WRITE_ALIAS_NAME).isHidden(true))
+                    .request();
+            executeAsyncWithOrigin(client.threadPool().getThreadContext(), ML_ORIGIN, request,
                 ActionListener.<AcknowledgedResponse>wrap(
                     r -> checkMappingsListener.onResponse(r.isAcknowledged()), finalListener::onFailure),
                 client.admin().indices()::aliases);
@@ -119,18 +104,18 @@ public static void createAnnotationsIndexIfNecessary(Client client, ClusterState
             mlLookup.isEmpty() == false && mlLookup.firstKey().startsWith(".ml")) {
 
             // Create the annotations index if it doesn't exist already.
-            if (mlLookup.containsKey(LATEST_INDEX_NAME) == false) {
+            if (mlLookup.containsKey(INDEX_NAME) == false) {
                 logger.debug(
                     () -> new ParameterizedMessage(
                         "Creating [{}] because [{}] exists; trace {}",
-                        LATEST_INDEX_NAME,
+                        INDEX_NAME,
                         mlLookup.firstKey(),
                         org.elasticsearch.ExceptionsHelper.formatStackTrace(Thread.currentThread().getStackTrace())
                     )
                 );
 
                 CreateIndexRequest createIndexRequest =
-                    new CreateIndexRequest(LATEST_INDEX_NAME)
+                    new CreateIndexRequest(INDEX_NAME)
                         .mapping(annotationsMapping())
                         .settings(Settings.builder()
                             .put(IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS, "0-1")
@@ -155,14 +140,7 @@ public static void createAnnotationsIndexIfNecessary(Client client, ClusterState
             }
 
             // 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);
-                return;
-            }
-
-            List<IndexMetadata> writeAliasMetadata = writeAliasDefinition.getIndices();
-            if (writeAliasMetadata.size() != 1 || LATEST_INDEX_NAME.equals(writeAliasMetadata.get(0).getIndex().getName()) == false) {
+            if (mlLookup.containsKey(READ_ALIAS_NAME) == false || mlLookup.containsKey(WRITE_ALIAS_NAME) == false) {
                 createAliasListener.onResponse(true);
                 return;
             }
@@ -176,7 +154,7 @@ public static void createAnnotationsIndexIfNecessary(Client client, ClusterState
         finalListener.onResponse(false);
     }
 
-    public static String annotationsMapping() {
+    private static String annotationsMapping() {
         return TemplateUtils.loadTemplate(
             "/org/elasticsearch/xpack/core/ml/annotations_index_mappings.json", Version.CURRENT.toString(), MAPPINGS_VERSION_VARIABLE);
     }

diff --git a/...n/core/src/main/resources/org/elasticsearch/xpack/core/ml/annotations_index_mappings.json b/...n/core/src/main/resources/org/elasticsearch/xpack/core/ml/annotations_index_mappings.json
@@ -3,7 +3,6 @@
     "_meta" : {
       "version" : "${xpack.ml.version}"
     },
-    "dynamic" : false,
     "properties" : {
       "annotation" : {
         "type" : "text"

diff --git a/.../test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java b/.../test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java
@@ -1478,7 +1478,7 @@ public void testMachineLearningAdminRole() {
         assertOnlyReadAllowed(role, AnomalyDetectorsIndexFields.STATE_INDEX_PREFIX);
         assertOnlyReadAllowed(role, AnomalyDetectorsIndexFields.RESULTS_INDEX_PREFIX + AnomalyDetectorsIndexFields.RESULTS_INDEX_DEFAULT);
         assertOnlyReadAllowed(role, NotificationsIndex.NOTIFICATIONS_INDEX);
-        assertReadWriteDocsButNotDeleteIndexAllowed(role, AnnotationIndex.LATEST_INDEX_NAME);
+        assertReadWriteDocsButNotDeleteIndexAllowed(role, AnnotationIndex.INDEX_NAME);
 
         assertNoAccessAllowed(role, RestrictedIndicesNames.RESTRICTED_NAMES);
         assertNoAccessAllowed(role, XPackPlugin.ASYNC_RESULTS_INDEX + randomAlphaOfLengthBetween(0, 2));
@@ -1632,7 +1632,7 @@ public void testMachineLearningUserRole() {
         assertNoAccessAllowed(role, AnomalyDetectorsIndexFields.STATE_INDEX_PREFIX);
         assertOnlyReadAllowed(role, AnomalyDetectorsIndexFields.RESULTS_INDEX_PREFIX + AnomalyDetectorsIndexFields.RESULTS_INDEX_DEFAULT);
         assertOnlyReadAllowed(role, NotificationsIndex.NOTIFICATIONS_INDEX);
-        assertReadWriteDocsButNotDeleteIndexAllowed(role, AnnotationIndex.LATEST_INDEX_NAME);
+        assertReadWriteDocsButNotDeleteIndexAllowed(role, AnnotationIndex.INDEX_NAME);
 
         assertNoAccessAllowed(role, RestrictedIndicesNames.RESTRICTED_NAMES);
         assertNoAccessAllowed(role, XPackPlugin.ASYNC_RESULTS_INDEX + randomAlphaOfLengthBetween(0, 2));

diff --git a/...RestTest/java/org/elasticsearch/xpack/ml/integration/MlNativeAutodetectIntegTestCase.java b/...RestTest/java/org/elasticsearch/xpack/ml/integration/MlNativeAutodetectIntegTestCase.java
@@ -264,7 +264,7 @@ protected void waitForecastStatus(int maxWaitTimeSeconds,
 
     protected void assertThatNumberOfAnnotationsIsEqualTo(int expectedNumberOfAnnotations) throws IOException {
         // Refresh the annotations index so that recently indexed annotation docs are visible.
-        client().admin().indices().prepareRefresh(AnnotationIndex.LATEST_INDEX_NAME)
+        client().admin().indices().prepareRefresh(AnnotationIndex.INDEX_NAME)
             .setIndicesOptions(IndicesOptions.LENIENT_EXPAND_OPEN_CLOSED_HIDDEN)
             .execute()
             .actionGet();

diff --git a/...rc/internalClusterTest/java/org/elasticsearch/xpack/ml/integration/AnnotationIndexIT.java b/...rc/internalClusterTest/java/org/elasticsearch/xpack/ml/integration/AnnotationIndexIT.java
@@ -7,18 +7,13 @@
 package org.elasticsearch.xpack.ml.integration;
 
 import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
-
-import org.elasticsearch.action.admin.indices.alias.Alias;
-import org.elasticsearch.action.admin.indices.create.CreateIndexAction;
-import org.elasticsearch.action.admin.indices.create.CreateIndexRequest;
 import org.elasticsearch.action.index.IndexRequest;
 import org.elasticsearch.action.index.IndexResponse;
 import org.elasticsearch.action.search.SearchPhaseExecutionException;
 import org.elasticsearch.action.search.SearchRequest;
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.action.support.IndicesOptions;
 import org.elasticsearch.cluster.metadata.AliasMetadata;
-import org.elasticsearch.cluster.metadata.IndexMetadata;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.collect.ImmutableOpenMap;
 import org.elasticsearch.common.settings.Settings;
@@ -31,12 +26,11 @@
 import org.elasticsearch.xpack.core.ml.annotations.AnnotationIndex;
 import org.elasticsearch.xpack.ml.MlSingleNodeTestCase;
 import org.elasticsearch.xpack.ml.notifications.AnomalyDetectionAuditor;
+import org.junit.Before;
 
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 
-import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.is;
 
 public class AnnotationIndexIT extends MlSingleNodeTestCase {
@@ -50,6 +44,11 @@ protected Settings nodeSettings()  {
         return newSettings.build();
     }
 
+    @Before
+    public void createComponents() throws Exception {
+        waitForMlTemplates();
+    }
+
     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
@@ -72,52 +71,6 @@ public void testCreatedWhenAfterOtherMlIndex() throws Exception {
         });
     }
 
-    public void testAliasesMovedFromOldToNew() throws Exception {
-
-        // Create an old annotations index with both read and write aliases pointing at it.
-        String oldIndex = randomFrom(AnnotationIndex.OLD_INDEX_NAMES);
-        CreateIndexRequest createIndexRequest =
-            new CreateIndexRequest(oldIndex)
-                .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))
-                .alias(new Alias(AnnotationIndex.READ_ALIAS_NAME).isHidden(true))
-                .alias(new Alias(AnnotationIndex.WRITE_ALIAS_NAME).isHidden(true));
-        client().execute(CreateIndexAction.INSTANCE, createIndexRequest).actionGet();
-
-        // Because the old annotations index name began with .ml, it will trigger the new annotations index to be created.
-        // 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());
-            ImmutableOpenMap<String, List<AliasMetadata>> aliases = client().admin().indices()
-                .prepareGetAliases(AnnotationIndex.READ_ALIAS_NAME, AnnotationIndex.WRITE_ALIAS_NAME)
-                .setIndicesOptions(IndicesOptions.LENIENT_EXPAND_OPEN_CLOSED_HIDDEN)
-                .get()
-                .getAliases();
-            assertNotNull(aliases);
-            List<String> indicesWithReadAlias = new ArrayList<>();
-            for (ObjectObjectCursor<String, List<AliasMetadata>> entry : aliases) {
-                for (AliasMetadata aliasMetadata : entry.value) {
-                    switch (aliasMetadata.getAlias()) {
-                        case AnnotationIndex.WRITE_ALIAS_NAME:
-                            assertThat(entry.key, is(AnnotationIndex.LATEST_INDEX_NAME));
-                            break;
-                        case AnnotationIndex.READ_ALIAS_NAME:
-                            indicesWithReadAlias.add(entry.key);
-                            break;
-                        default:
-                            fail("Found unexpected alias " + aliasMetadata.getAlias() + " on index " + entry.key);
-                            break;
-                    }
-                }
-            }
-            assertThat(indicesWithReadAlias, containsInAnyOrder(oldIndex, AnnotationIndex.LATEST_INDEX_NAME));
-        });
-    }
-
     public void testNotCreatedWhenAfterOtherMlIndexAndUpgradeInProgress() throws Exception {
 
         client().execute(SetUpgradeModeAction.INSTANCE, new SetUpgradeModeAction.Request(true)).actionGet();
@@ -170,7 +123,7 @@ public void testNotCreatedWhenAfterOtherMlIndexAndResetInProgress() throws Excep
     }
 
     private boolean annotationsIndexExists() {
-        return ESIntegTestCase.indexExists(AnnotationIndex.LATEST_INDEX_NAME, client());
+        return ESIntegTestCase.indexExists(AnnotationIndex.INDEX_NAME, client());
     }
 
     private int numberOfAnnotationsAliases() {

diff --git a/...lClusterTest/java/org/elasticsearch/xpack/ml/integration/AutodetectResultProcessorIT.java b/...lClusterTest/java/org/elasticsearch/xpack/ml/integration/AutodetectResultProcessorIT.java
@@ -731,7 +731,7 @@ private QueryPage<ModelSnapshot> getModelSnapshots() throws Exception {
 
     private List<Annotation> getAnnotations() throws Exception {
         // Refresh the annotations index so that recently indexed annotation docs are visible.
-        client().admin().indices().prepareRefresh(AnnotationIndex.LATEST_INDEX_NAME)
+        client().admin().indices().prepareRefresh(AnnotationIndex.INDEX_NAME)
             .setIndicesOptions(IndicesOptions.STRICT_EXPAND_OPEN_HIDDEN_FORBID_CLOSED)
             .execute()
             .actionGet();

diff --git a/...java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectProcessManagerTests.java b/...java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectProcessManagerTests.java
@@ -92,7 +92,6 @@
 import java.util.function.Consumer;
 
 import static org.elasticsearch.action.support.master.MasterNodeRequest.DEFAULT_MASTER_NODE_TIMEOUT;
-import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_INDEX_HIDDEN;
 import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
 import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
 import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_VERSION_CREATED;
@@ -185,23 +184,21 @@ public void setup() throws Exception {
                             Settings.builder()
                                 .put(SETTING_NUMBER_OF_SHARDS, 1)
                                 .put(SETTING_NUMBER_OF_REPLICAS, 0)
-                                .put(SETTING_INDEX_HIDDEN, true)
                                 .put(SETTING_VERSION_CREATED, Version.CURRENT)
                                 .build())
-                        .putAlias(AliasMetadata.builder(AnomalyDetectorsIndex.jobStateIndexWriteAlias()).isHidden(true).build())
+                        .putAlias(AliasMetadata.builder(AnomalyDetectorsIndex.jobStateIndexWriteAlias()).build())
                         .build())
                 .fPut(
-                    AnnotationIndex.LATEST_INDEX_NAME,
-                    IndexMetadata.builder(AnnotationIndex.LATEST_INDEX_NAME)
+                    AnnotationIndex.INDEX_NAME,
+                    IndexMetadata.builder(AnnotationIndex.INDEX_NAME)
                         .settings(
                             Settings.builder()
                                 .put(SETTING_NUMBER_OF_SHARDS, 1)
                                 .put(SETTING_NUMBER_OF_REPLICAS, 0)
-                                .put(SETTING_INDEX_HIDDEN, true)
                                 .put(SETTING_VERSION_CREATED, Version.CURRENT)
                                 .build())
-                        .putAlias(AliasMetadata.builder(AnnotationIndex.READ_ALIAS_NAME).isHidden(true).build())
-                        .putAlias(AliasMetadata.builder(AnnotationIndex.WRITE_ALIAS_NAME).isHidden(true).build())
+                        .putAlias(AliasMetadata.builder(AnnotationIndex.READ_ALIAS_NAME).build())
+                        .putAlias(AliasMetadata.builder(AnnotationIndex.WRITE_ALIAS_NAME).build())
                         .build())
                 .build())
             .build();

diff --git a/...olling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/30_ml_jobs_crud.yml b/...olling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/30_ml_jobs_crud.yml
@@ -157,6 +157,6 @@ setup:
       indices.get_mapping:
         index: .ml-annotations-write
 
-  - match: { \.ml-annotations-000001.mappings.properties.type.type: "keyword" }
-  - match: { \.ml-annotations-000001.mappings.properties.event.type: "keyword" }
-  - match: { \.ml-annotations-000001.mappings.properties.detector_index.type: "integer" }
+  - match: { \.ml-annotations-6.mappings.properties.type.type: "keyword" }
+  - match: { \.ml-annotations-6.mappings.properties.event.type: "keyword" }
+  - match: { \.ml-annotations-6.mappings.properties.detector_index.type: "integer" }