Skip to content

Commit

Permalink
[ML] Ensure annotations index mappings are up to date
Browse files Browse the repository at this point in the history
When the ML annotations index was first added, only the
ML UI wrote to it, so the code to create it was designed
with this in mind.  Now the ML backend also creates
annotations, and those mappings can change between
versions.

In this change:

1. The code that runs on the master node to create the
   annotations index if it doesn't exist but another ML
   index does also now ensures the mappings are up-to-date.
   This is good enough for the ML UI's use of the
   annotations index, because the upgrade order rules say
   that the whole Elasticsearch cluster must be upgraded
   prior to Kibana, so the master node should be on the
   newer version before Kibana tries to write an
   annotation with the new fields.
2. We now also check whether the annotations index exists
   with the correct mappings before starting an autodetect
   process on a node.  This is necessary because ML nodes
   can be upgraded before the master node, so could write
   an annotation with the new fields before the master node
   knows about the new fields.

Relates elastic/kibana#74935
  • Loading branch information
droberts195 committed Aug 13, 2020
1 parent 3250ea4 commit 90155af
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.elasticsearch.cluster.metadata.IndexAbstraction;
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.xpack.core.ml.job.persistence.ElasticsearchMappings;
import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper;
import org.elasticsearch.xpack.core.template.TemplateUtils;

Expand All @@ -40,14 +41,24 @@ public class AnnotationIndex {
*/
public static void createAnnotationsIndexIfNecessary(Client client, ClusterState state, final ActionListener<Boolean> finalListener) {

final ActionListener<Boolean> checkMappingsListener = ActionListener.wrap(success -> {
ElasticsearchMappings.addDocMappingIfMissing(
WRITE_ALIAS_NAME,
AnnotationIndex::annotationsMapping,
client,
state,
finalListener);
}, finalListener::onFailure);

final ActionListener<Boolean> createAliasListener = ActionListener.wrap(success -> {
final IndicesAliasesRequest request =
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,
ActionListener.<AcknowledgedResponse>wrap(r -> finalListener.onResponse(r.isAcknowledged()), finalListener::onFailure),
ActionListener.<AcknowledgedResponse>wrap(
r -> checkMappingsListener.onResponse(r.isAcknowledged()), finalListener::onFailure),
client.admin().indices()::aliases);
}, finalListener::onFailure);

Expand Down Expand Up @@ -88,6 +99,10 @@ public static void createAnnotationsIndexIfNecessary(Client client, ClusterState
createAliasListener.onResponse(true);
return;
}

// Check the mappings
checkMappingsListener.onResponse(false);
return;
}

// Nothing to do, but respond to the listener
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.elasticsearch.xpack.core.action.util.QueryPage;
import org.elasticsearch.xpack.core.ml.MlMetadata;
import org.elasticsearch.xpack.core.ml.action.GetFiltersAction;
import org.elasticsearch.xpack.core.ml.annotations.AnnotationIndex;
import org.elasticsearch.xpack.core.ml.calendars.ScheduledEvent;
import org.elasticsearch.xpack.core.ml.job.config.Job;
import org.elasticsearch.xpack.core.ml.job.config.JobState;
Expand Down Expand Up @@ -448,8 +449,20 @@ protected void doRun() {
);

// Try adding the results doc mapping - this updates to the latest version if an old mapping is present
ElasticsearchMappings.addDocMappingIfMissing(AnomalyDetectorsIndex.jobResultsAliasedName(jobId),
AnomalyDetectorsIndex::resultsMapping, client, clusterState, resultsMappingUpdateHandler);
ActionListener<Boolean> annotationsIndexUpdateHandler = ActionListener.wrap(
ack -> ElasticsearchMappings.addDocMappingIfMissing(AnomalyDetectorsIndex.jobResultsAliasedName(jobId),
AnomalyDetectorsIndex::resultsMapping, client, clusterState, resultsMappingUpdateHandler),
e -> {
// Due to a bug in 7.9.0 it's possible that the annotations index already has incorrect mappings
// and it would cause more harm than good to block jobs from opening in subsequent releases
logger.warn(new ParameterizedMessage("[{}] ML annotations index could not be updated with latest mappings", jobId), e);
ElasticsearchMappings.addDocMappingIfMissing(AnomalyDetectorsIndex.jobResultsAliasedName(jobId),
AnomalyDetectorsIndex::resultsMapping, client, clusterState, resultsMappingUpdateHandler);
}
);

// Create the annotations index if necessary - this also updates the mappings if an old mapping is present
AnnotationIndex.createAnnotationsIndexIfNecessary(client, clusterState, annotationsIndexUpdateHandler);
}

private boolean createProcessAndSetRunning(ProcessContext processContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,3 +149,14 @@ setup:
job_id: old-cluster-function-shortcut-expansion
- match: { count: 1 }
- match: { jobs.0.analysis_config.detectors.0.function: "non_zero_count" }

---
"Test annotation index mappings":

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

0 comments on commit 90155af

Please sign in to comment.