Skip to content

Commit d5249ce

Browse files
droberts195droberts195
authored
[ML] Use a new annotations index for future annotations (#79460)
Due to historical bugs a non-negligible proportion of ML users have an annotations index with incorrect mappings. We have published instructions on how to correct the mappings, but the procedure is complicated, and some users prefer to tolerate the lack of annotations functionality rather than attempt the operations necessary to fix the mappings of the existing index. The best way to solve the problem is to create a new annotations index with the correct mappings, and use this for all annotations created from that moment on. This PR implements that solution. The annotations read alias will span the old and new indices in the case where both exist. The annotations write index is adjusted to point only at the new index. Adds back #79006, which was reverted because Kibana was not ready.
1 parent f13912c commit d5249ce

File tree

8 files changed

+105
-31
lines changed

8 files changed

+105
-31
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/annotations/AnnotationIndex.java

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import org.elasticsearch.action.admin.cluster.health.ClusterHealthRequest;
1616
import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse;
1717
import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest;
18+
import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequestBuilder;
1819
import org.elasticsearch.action.admin.indices.create.CreateIndexRequest;
1920
import org.elasticsearch.action.admin.indices.create.CreateIndexResponse;
2021
import org.elasticsearch.action.support.master.AcknowledgedResponse;
@@ -25,11 +26,13 @@
2526
import org.elasticsearch.cluster.metadata.IndexMetadata;
2627
import org.elasticsearch.common.settings.Settings;
2728
import org.elasticsearch.core.TimeValue;
29+
import org.elasticsearch.index.Index;
2830
import org.elasticsearch.xpack.core.ml.MlMetadata;
2931
import org.elasticsearch.xpack.core.ml.job.persistence.ElasticsearchMappings;
3032
import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper;
3133
import org.elasticsearch.xpack.core.template.TemplateUtils;
3234

35+
import java.util.List;
3336
import java.util.SortedMap;
3437

3538
import static org.elasticsearch.xpack.core.ClientHelper.ML_ORIGIN;
@@ -41,8 +44,15 @@ public class AnnotationIndex {
4144

4245
public static final String READ_ALIAS_NAME = ".ml-annotations-read";
4346
public static final String WRITE_ALIAS_NAME = ".ml-annotations-write";
44-
// Exposed for testing, but always use the aliases in non-test code
45-
public static final String INDEX_NAME = ".ml-annotations-6";
47+
48+
// Exposed for testing, but always use the aliases in non-test code.
49+
public static final String LATEST_INDEX_NAME = ".ml-annotations-000001";
50+
// Due to historical bugs this index may not have the correct mappings
51+
// in some production clusters. Therefore new annotations should be
52+
// written to the latest index. If we ever switch to another new annotations
53+
// index then this list should be adjusted to include the previous latest
54+
// index.
55+
public static final List<String> OLD_INDEX_NAMES = List.of(".ml-annotations-6");
4656

4757
private static final String MAPPINGS_VERSION_VARIABLE = "xpack.ml.version";
4858

@@ -85,12 +95,18 @@ public static void createAnnotationsIndexIfNecessary(Client client, ClusterState
8595
finalListener::onFailure);
8696

8797
final ActionListener<Boolean> createAliasListener = ActionListener.wrap(success -> {
88-
final IndicesAliasesRequest request =
98+
final IndicesAliasesRequestBuilder requestBuilder =
8999
client.admin().indices().prepareAliases()
90-
.addAliasAction(IndicesAliasesRequest.AliasActions.add().index(INDEX_NAME).alias(READ_ALIAS_NAME).isHidden(true))
91-
.addAliasAction(IndicesAliasesRequest.AliasActions.add().index(INDEX_NAME).alias(WRITE_ALIAS_NAME).isHidden(true))
92-
.request();
93-
executeAsyncWithOrigin(client.threadPool().getThreadContext(), ML_ORIGIN, request,
100+
.addAliasAction(IndicesAliasesRequest.AliasActions.add()
101+
.index(LATEST_INDEX_NAME).alias(READ_ALIAS_NAME).isHidden(true))
102+
.addAliasAction(IndicesAliasesRequest.AliasActions.add()
103+
.index(LATEST_INDEX_NAME).alias(WRITE_ALIAS_NAME).isHidden(true));
104+
for (String oldIndexName : OLD_INDEX_NAMES) {
105+
if (state.getMetadata().getIndicesLookup().containsKey(oldIndexName)) {
106+
requestBuilder.removeAlias(oldIndexName, WRITE_ALIAS_NAME);
107+
}
108+
}
109+
executeAsyncWithOrigin(client.threadPool().getThreadContext(), ML_ORIGIN, requestBuilder.request(),
94110
ActionListener.<AcknowledgedResponse>wrap(
95111
r -> checkMappingsListener.onResponse(r.isAcknowledged()), finalListener::onFailure),
96112
client.admin().indices()::aliases);
@@ -104,18 +120,18 @@ public static void createAnnotationsIndexIfNecessary(Client client, ClusterState
104120
mlLookup.isEmpty() == false && mlLookup.firstKey().startsWith(".ml")) {
105121

106122
// Create the annotations index if it doesn't exist already.
107-
if (mlLookup.containsKey(INDEX_NAME) == false) {
123+
if (mlLookup.containsKey(LATEST_INDEX_NAME) == false) {
108124
logger.debug(
109125
() -> new ParameterizedMessage(
110126
"Creating [{}] because [{}] exists; trace {}",
111-
INDEX_NAME,
127+
LATEST_INDEX_NAME,
112128
mlLookup.firstKey(),
113129
org.elasticsearch.ExceptionsHelper.formatStackTrace(Thread.currentThread().getStackTrace())
114130
)
115131
);
116132

117133
CreateIndexRequest createIndexRequest =
118-
new CreateIndexRequest(INDEX_NAME)
134+
new CreateIndexRequest(LATEST_INDEX_NAME)
119135
.mapping(annotationsMapping())
120136
.settings(Settings.builder()
121137
.put(IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS, "0-1")
@@ -140,7 +156,14 @@ public static void createAnnotationsIndexIfNecessary(Client client, ClusterState
140156
}
141157

142158
// Recreate the aliases if they've gone even though the index still exists.
143-
if (mlLookup.containsKey(READ_ALIAS_NAME) == false || mlLookup.containsKey(WRITE_ALIAS_NAME) == false) {
159+
IndexAbstraction writeAliasDefinition = mlLookup.get(WRITE_ALIAS_NAME);
160+
if (mlLookup.containsKey(READ_ALIAS_NAME) == false || writeAliasDefinition == null) {
161+
createAliasListener.onResponse(true);
162+
return;
163+
}
164+
165+
List<Index> writeAliasIndices = writeAliasDefinition.getIndices();
166+
if (writeAliasIndices.size() != 1 || LATEST_INDEX_NAME.equals(writeAliasIndices.get(0).getName()) == false) {
144167
createAliasListener.onResponse(true);
145168
return;
146169
}
@@ -154,7 +177,7 @@ public static void createAnnotationsIndexIfNecessary(Client client, ClusterState
154177
finalListener.onResponse(false);
155178
}
156179

157-
private static String annotationsMapping() {
180+
public static String annotationsMapping() {
158181
return TemplateUtils.loadTemplate(
159182
"/org/elasticsearch/xpack/core/ml/annotations_index_mappings.json", Version.CURRENT.toString(), MAPPINGS_VERSION_VARIABLE);
160183
}

‎x-pack/plugin/core/src/main/resources/org/elasticsearch/xpack/core/ml/annotations_index_mappings.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
"_meta" : {
44
"version" : "${xpack.ml.version}"
55
},
6+
"dynamic" : false,
67
"properties" : {
78
"annotation" : {
89
"type" : "text"

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1478,7 +1478,7 @@ public void testMachineLearningAdminRole() {
14781478
assertOnlyReadAllowed(role, AnomalyDetectorsIndexFields.STATE_INDEX_PREFIX);
14791479
assertOnlyReadAllowed(role, AnomalyDetectorsIndexFields.RESULTS_INDEX_PREFIX + AnomalyDetectorsIndexFields.RESULTS_INDEX_DEFAULT);
14801480
assertOnlyReadAllowed(role, NotificationsIndex.NOTIFICATIONS_INDEX);
1481-
assertReadWriteDocsButNotDeleteIndexAllowed(role, AnnotationIndex.INDEX_NAME);
1481+
assertReadWriteDocsButNotDeleteIndexAllowed(role, AnnotationIndex.LATEST_INDEX_NAME);
14821482

14831483
assertNoAccessAllowed(role, RestrictedIndicesNames.RESTRICTED_NAMES);
14841484
assertNoAccessAllowed(role, XPackPlugin.ASYNC_RESULTS_INDEX + randomAlphaOfLengthBetween(0, 2));
@@ -1632,7 +1632,7 @@ public void testMachineLearningUserRole() {
16321632
assertNoAccessAllowed(role, AnomalyDetectorsIndexFields.STATE_INDEX_PREFIX);
16331633
assertOnlyReadAllowed(role, AnomalyDetectorsIndexFields.RESULTS_INDEX_PREFIX + AnomalyDetectorsIndexFields.RESULTS_INDEX_DEFAULT);
16341634
assertOnlyReadAllowed(role, NotificationsIndex.NOTIFICATIONS_INDEX);
1635-
assertReadWriteDocsButNotDeleteIndexAllowed(role, AnnotationIndex.INDEX_NAME);
1635+
assertReadWriteDocsButNotDeleteIndexAllowed(role, AnnotationIndex.LATEST_INDEX_NAME);
16361636

16371637
assertNoAccessAllowed(role, RestrictedIndicesNames.RESTRICTED_NAMES);
16381638
assertNoAccessAllowed(role, XPackPlugin.ASYNC_RESULTS_INDEX + randomAlphaOfLengthBetween(0, 2));

‎x-pack/plugin/ml/qa/native-multi-node-tests/src/javaRestTest/java/org/elasticsearch/xpack/ml/integration/MlNativeAutodetectIntegTestCase.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ protected void waitForecastStatus(int maxWaitTimeSeconds,
264264

265265
protected void assertThatNumberOfAnnotationsIsEqualTo(int expectedNumberOfAnnotations) throws IOException {
266266
// Refresh the annotations index so that recently indexed annotation docs are visible.
267-
client().admin().indices().prepareRefresh(AnnotationIndex.INDEX_NAME)
267+
client().admin().indices().prepareRefresh(AnnotationIndex.LATEST_INDEX_NAME)
268268
.setIndicesOptions(IndicesOptions.LENIENT_EXPAND_OPEN_CLOSED_HIDDEN)
269269
.execute()
270270
.actionGet();

‎x-pack/plugin/ml/src/internalClusterTest/java/org/elasticsearch/xpack/ml/integration/AnnotationIndexIT.java

Lines changed: 54 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,18 @@
77
package org.elasticsearch.xpack.ml.integration;
88

99
import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
10+
11+
import org.elasticsearch.action.admin.indices.alias.Alias;
12+
import org.elasticsearch.action.admin.indices.create.CreateIndexAction;
13+
import org.elasticsearch.action.admin.indices.create.CreateIndexRequest;
1014
import org.elasticsearch.action.index.IndexRequest;
1115
import org.elasticsearch.action.index.IndexResponse;
1216
import org.elasticsearch.action.search.SearchPhaseExecutionException;
1317
import org.elasticsearch.action.search.SearchRequest;
1418
import org.elasticsearch.action.search.SearchResponse;
1519
import org.elasticsearch.action.support.IndicesOptions;
1620
import org.elasticsearch.cluster.metadata.AliasMetadata;
21+
import org.elasticsearch.cluster.metadata.IndexMetadata;
1722
import org.elasticsearch.cluster.service.ClusterService;
1823
import org.elasticsearch.common.collect.ImmutableOpenMap;
1924
import org.elasticsearch.common.settings.Settings;
@@ -26,11 +31,12 @@
2631
import org.elasticsearch.xpack.core.ml.annotations.AnnotationIndex;
2732
import org.elasticsearch.xpack.ml.MlSingleNodeTestCase;
2833
import org.elasticsearch.xpack.ml.notifications.AnomalyDetectionAuditor;
29-
import org.junit.Before;
3034

35+
import java.util.ArrayList;
3136
import java.util.Collections;
3237
import java.util.List;
3338

39+
import static org.hamcrest.Matchers.containsInAnyOrder;
3440
import static org.hamcrest.Matchers.is;
3541

3642
public class AnnotationIndexIT extends MlSingleNodeTestCase {
@@ -44,11 +50,6 @@ protected Settings nodeSettings() {
4450
return newSettings.build();
4551
}
4652

47-
@Before
48-
public void createComponents() throws Exception {
49-
waitForMlTemplates();
50-
}
51-
5253
public void testNotCreatedWhenNoOtherMlIndices() {
5354

5455
// Ask a few times to increase the chance of failure if the .ml-annotations index is created when no other ML index exists
@@ -71,6 +72,52 @@ public void testCreatedWhenAfterOtherMlIndex() throws Exception {
7172
});
7273
}
7374

75+
public void testAliasesMovedFromOldToNew() throws Exception {
76+
77+
// Create an old annotations index with both read and write aliases pointing at it.
78+
String oldIndex = randomFrom(AnnotationIndex.OLD_INDEX_NAMES);
79+
CreateIndexRequest createIndexRequest =
80+
new CreateIndexRequest(oldIndex)
81+
.mapping(AnnotationIndex.annotationsMapping())
82+
.settings(Settings.builder()
83+
.put(IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS, "0-1")
84+
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, "1")
85+
.put(IndexMetadata.SETTING_INDEX_HIDDEN, true))
86+
.alias(new Alias(AnnotationIndex.READ_ALIAS_NAME).isHidden(true))
87+
.alias(new Alias(AnnotationIndex.WRITE_ALIAS_NAME).isHidden(true));
88+
client().execute(CreateIndexAction.INSTANCE, createIndexRequest).actionGet();
89+
90+
// Because the old annotations index name began with .ml, it will trigger the new annotations index to be created.
91+
// When this happens the read alias should be changed to cover both indices, and the write alias should be
92+
// switched to only point at the new index.
93+
assertBusy(() -> {
94+
assertTrue(annotationsIndexExists());
95+
ImmutableOpenMap<String, List<AliasMetadata>> aliases = client().admin().indices()
96+
.prepareGetAliases(AnnotationIndex.READ_ALIAS_NAME, AnnotationIndex.WRITE_ALIAS_NAME)
97+
.setIndicesOptions(IndicesOptions.LENIENT_EXPAND_OPEN_CLOSED_HIDDEN)
98+
.get()
99+
.getAliases();
100+
assertNotNull(aliases);
101+
List<String> indicesWithReadAlias = new ArrayList<>();
102+
for (ObjectObjectCursor<String, List<AliasMetadata>> entry : aliases) {
103+
for (AliasMetadata aliasMetadata : entry.value) {
104+
switch (aliasMetadata.getAlias()) {
105+
case AnnotationIndex.WRITE_ALIAS_NAME:
106+
assertThat(entry.key, is(AnnotationIndex.LATEST_INDEX_NAME));
107+
break;
108+
case AnnotationIndex.READ_ALIAS_NAME:
109+
indicesWithReadAlias.add(entry.key);
110+
break;
111+
default:
112+
fail("Found unexpected alias " + aliasMetadata.getAlias() + " on index " + entry.key);
113+
break;
114+
}
115+
}
116+
}
117+
assertThat(indicesWithReadAlias, containsInAnyOrder(oldIndex, AnnotationIndex.LATEST_INDEX_NAME));
118+
});
119+
}
120+
74121
public void testNotCreatedWhenAfterOtherMlIndexAndUpgradeInProgress() throws Exception {
75122

76123
client().execute(SetUpgradeModeAction.INSTANCE, new SetUpgradeModeAction.Request(true)).actionGet();
@@ -123,7 +170,7 @@ public void testNotCreatedWhenAfterOtherMlIndexAndResetInProgress() throws Excep
123170
}
124171

125172
private boolean annotationsIndexExists() {
126-
return ESIntegTestCase.indexExists(AnnotationIndex.INDEX_NAME, client());
173+
return ESIntegTestCase.indexExists(AnnotationIndex.LATEST_INDEX_NAME, client());
127174
}
128175

129176
private int numberOfAnnotationsAliases() {

‎x-pack/plugin/ml/src/internalClusterTest/java/org/elasticsearch/xpack/ml/integration/AutodetectResultProcessorIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -731,7 +731,7 @@ private QueryPage<ModelSnapshot> getModelSnapshots() throws Exception {
731731

732732
private List<Annotation> getAnnotations() throws Exception {
733733
// Refresh the annotations index so that recently indexed annotation docs are visible.
734-
client().admin().indices().prepareRefresh(AnnotationIndex.INDEX_NAME)
734+
client().admin().indices().prepareRefresh(AnnotationIndex.LATEST_INDEX_NAME)
735735
.setIndicesOptions(IndicesOptions.STRICT_EXPAND_OPEN_HIDDEN_FORBID_CLOSED)
736736
.execute()
737737
.actionGet();

‎x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectProcessManagerTests.java

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@
9292
import java.util.function.Consumer;
9393

9494
import static org.elasticsearch.action.support.master.MasterNodeRequest.DEFAULT_MASTER_NODE_TIMEOUT;
95+
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_INDEX_HIDDEN;
9596
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
9697
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
9798
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_VERSION_CREATED;
@@ -184,21 +185,23 @@ public void setup() throws Exception {
184185
Settings.builder()
185186
.put(SETTING_NUMBER_OF_SHARDS, 1)
186187
.put(SETTING_NUMBER_OF_REPLICAS, 0)
188+
.put(SETTING_INDEX_HIDDEN, true)
187189
.put(SETTING_VERSION_CREATED, Version.CURRENT)
188190
.build())
189-
.putAlias(AliasMetadata.builder(AnomalyDetectorsIndex.jobStateIndexWriteAlias()).build())
191+
.putAlias(AliasMetadata.builder(AnomalyDetectorsIndex.jobStateIndexWriteAlias()).isHidden(true).build())
190192
.build())
191193
.fPut(
192-
AnnotationIndex.INDEX_NAME,
193-
IndexMetadata.builder(AnnotationIndex.INDEX_NAME)
194+
AnnotationIndex.LATEST_INDEX_NAME,
195+
IndexMetadata.builder(AnnotationIndex.LATEST_INDEX_NAME)
194196
.settings(
195197
Settings.builder()
196198
.put(SETTING_NUMBER_OF_SHARDS, 1)
197199
.put(SETTING_NUMBER_OF_REPLICAS, 0)
200+
.put(SETTING_INDEX_HIDDEN, true)
198201
.put(SETTING_VERSION_CREATED, Version.CURRENT)
199202
.build())
200-
.putAlias(AliasMetadata.builder(AnnotationIndex.READ_ALIAS_NAME).build())
201-
.putAlias(AliasMetadata.builder(AnnotationIndex.WRITE_ALIAS_NAME).build())
203+
.putAlias(AliasMetadata.builder(AnnotationIndex.READ_ALIAS_NAME).isHidden(true).build())
204+
.putAlias(AliasMetadata.builder(AnnotationIndex.WRITE_ALIAS_NAME).isHidden(true).build())
202205
.build())
203206
.build())
204207
.build();

‎x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/30_ml_jobs_crud.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,6 @@ setup:
157157
indices.get_mapping:
158158
index: .ml-annotations-write
159159

160-
- match: { \.ml-annotations-6.mappings.properties.type.type: "keyword" }
161-
- match: { \.ml-annotations-6.mappings.properties.event.type: "keyword" }
162-
- match: { \.ml-annotations-6.mappings.properties.detector_index.type: "integer" }
160+
- match: { \.ml-annotations-000001.mappings.properties.type.type: "keyword" }
161+
- match: { \.ml-annotations-000001.mappings.properties.event.type: "keyword" }
162+
- match: { \.ml-annotations-000001.mappings.properties.detector_index.type: "integer" }

0 commit comments

Comments
 (0)