Skip to content

[ML] Use a new annotations index for future annotations #79151

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

Merged
merged 2 commits into from
Oct 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
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;
Expand All @@ -30,6 +31,7 @@
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;
Expand All @@ -41,8 +43,15 @@ 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 INDEX_NAME = ".ml-annotations-6";

// 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");

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

Expand Down Expand Up @@ -85,12 +94,18 @@ public static void createAnnotationsIndexIfNecessary(Client client, ClusterState
finalListener::onFailure);

final ActionListener<Boolean> createAliasListener = ActionListener.wrap(success -> {
final IndicesAliasesRequest request =
final IndicesAliasesRequestBuilder requestBuilder =
client.admin().indices().prepareAliases()
.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,
.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(),
ActionListener.<AcknowledgedResponse>wrap(
r -> checkMappingsListener.onResponse(r.isAcknowledged()), finalListener::onFailure),
client.admin().indices()::aliases);
Expand All @@ -104,18 +119,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(INDEX_NAME) == false) {
if (mlLookup.containsKey(LATEST_INDEX_NAME) == false) {
logger.debug(
() -> new ParameterizedMessage(
"Creating [{}] because [{}] exists; trace {}",
INDEX_NAME,
LATEST_INDEX_NAME,
mlLookup.firstKey(),
org.elasticsearch.ExceptionsHelper.formatStackTrace(Thread.currentThread().getStackTrace())
)
);

CreateIndexRequest createIndexRequest =
new CreateIndexRequest(INDEX_NAME)
new CreateIndexRequest(LATEST_INDEX_NAME)
.mapping(annotationsMapping())
.settings(Settings.builder()
.put(IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS, "0-1")
Expand All @@ -140,7 +155,14 @@ public static void createAnnotationsIndexIfNecessary(Client client, ClusterState
}

// Recreate the aliases if they've gone even though the index still exists.
if (mlLookup.containsKey(READ_ALIAS_NAME) == false || mlLookup.containsKey(WRITE_ALIAS_NAME) == false) {
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) {
createAliasListener.onResponse(true);
return;
}
Expand All @@ -154,7 +176,7 @@ public static void createAnnotationsIndexIfNecessary(Client client, ClusterState
finalListener.onResponse(false);
}

private static String annotationsMapping() {
public static String annotationsMapping() {
return TemplateUtils.loadTemplate(
"/org/elasticsearch/xpack/core/ml/annotations_index_mappings.json", Version.CURRENT.toString(), MAPPINGS_VERSION_VARIABLE);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"_meta" : {
"version" : "${xpack.ml.version}"
},
"dynamic" : false,
"properties" : {
"annotation" : {
"type" : "text"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1479,7 +1479,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.INDEX_NAME);
assertReadWriteDocsButNotDeleteIndexAllowed(role, AnnotationIndex.LATEST_INDEX_NAME);

assertNoAccessAllowed(role, RestrictedIndicesNames.RESTRICTED_NAMES);
assertNoAccessAllowed(role, XPackPlugin.ASYNC_RESULTS_INDEX + randomAlphaOfLengthBetween(0, 2));
Expand Down Expand Up @@ -1633,7 +1633,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.INDEX_NAME);
assertReadWriteDocsButNotDeleteIndexAllowed(role, AnnotationIndex.LATEST_INDEX_NAME);

assertNoAccessAllowed(role, RestrictedIndicesNames.RESTRICTED_NAMES);
assertNoAccessAllowed(role, XPackPlugin.ASYNC_RESULTS_INDEX + randomAlphaOfLengthBetween(0, 2));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.INDEX_NAME)
client().admin().indices().prepareRefresh(AnnotationIndex.LATEST_INDEX_NAME)
.setIndicesOptions(IndicesOptions.LENIENT_EXPAND_OPEN_CLOSED_HIDDEN)
.execute()
.actionGet();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,18 @@
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;
Expand All @@ -26,11 +31,12 @@
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 {
Expand All @@ -44,11 +50,6 @@ 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
Expand All @@ -71,6 +72,52 @@ 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();
Expand Down Expand Up @@ -123,7 +170,7 @@ public void testNotCreatedWhenAfterOtherMlIndexAndResetInProgress() throws Excep
}

private boolean annotationsIndexExists() {
return ESIntegTestCase.indexExists(AnnotationIndex.INDEX_NAME, client());
return ESIntegTestCase.indexExists(AnnotationIndex.LATEST_INDEX_NAME, client());
}

private int numberOfAnnotationsAliases() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.INDEX_NAME)
client().admin().indices().prepareRefresh(AnnotationIndex.LATEST_INDEX_NAME)
.setIndicesOptions(IndicesOptions.STRICT_EXPAND_OPEN_HIDDEN_FORBID_CLOSED)
.execute()
.actionGet();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@
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;
Expand Down Expand Up @@ -184,21 +185,23 @@ 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()).build())
.putAlias(AliasMetadata.builder(AnomalyDetectorsIndex.jobStateIndexWriteAlias()).isHidden(true).build())
.build())
.fPut(
AnnotationIndex.INDEX_NAME,
IndexMetadata.builder(AnnotationIndex.INDEX_NAME)
AnnotationIndex.LATEST_INDEX_NAME,
IndexMetadata.builder(AnnotationIndex.LATEST_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).build())
.putAlias(AliasMetadata.builder(AnnotationIndex.WRITE_ALIAS_NAME).build())
.putAlias(AliasMetadata.builder(AnnotationIndex.READ_ALIAS_NAME).isHidden(true).build())
.putAlias(AliasMetadata.builder(AnnotationIndex.WRITE_ALIAS_NAME).isHidden(true).build())
.build())
.build())
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,6 @@ setup:
indices.get_mapping:
index: .ml-annotations-write

- 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" }
- 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" }