From 6fcc3be438962c1b3271536c9558f00b2cb9ec19 Mon Sep 17 00:00:00 2001 From: David Roberts Date: Wed, 16 Aug 2017 16:16:30 +0100 Subject: [PATCH] [ML] Preserve _meta on results index mapping update (elastic/x-pack-elasticsearch#2274) When mappings are updated for an index are updated most settings are merged, but not _meta. This change ensures that _meta is set when we add per-job term mappings to our results index mappings. In order to keep the logic for updating mappings after upgrade working, we now have to put ALL the mappings for our results along with the latest _meta section when updating per-job term mappings. relates elastic/x-pack-elasticsearch#2265 Original commit: elastic/x-pack-elasticsearch@f58c11a13e9f6728a876634002b46b0487f79bdd --- .../persistence/ElasticsearchMappings.java | 7 +++ .../xpack/ml/job/persistence/JobProvider.java | 7 ++- .../test/ml/ml_anomalies_default_mappings.yml | 51 +++++++++++++++++++ 3 files changed, 64 insertions(+), 1 deletion(-) diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/persistence/ElasticsearchMappings.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/persistence/ElasticsearchMappings.java index 2b5dfb48ca3..517b7f77393 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/persistence/ElasticsearchMappings.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/persistence/ElasticsearchMappings.java @@ -26,6 +26,7 @@ import org.elasticsearch.xpack.ml.notifications.AuditMessage; import java.io.IOException; import java.util.Collection; +import java.util.Collections; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; @@ -126,6 +127,10 @@ public class ElasticsearchMappings { } public static XContentBuilder docMapping() throws IOException { + return docMapping(Collections.emptyList()); + } + + public static XContentBuilder docMapping(Collection extraTermFields) throws IOException { XContentBuilder builder = jsonBuilder(); builder.startObject(); builder.startObject(DOC_TYPE); @@ -153,6 +158,8 @@ public class ElasticsearchMappings { addDataCountsMapping(builder); addModelSnapshotMapping(builder); + addTermFields(builder, extraTermFields); + // end properties builder.endObject(); // end mapping diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobProvider.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobProvider.java index 79e035d9df9..f0197b7bf39 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobProvider.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobProvider.java @@ -234,6 +234,8 @@ public class JobProvider { if (!state.getMetaData().hasIndex(indexName)) { LOGGER.trace("ES API CALL: create index {}", indexName); CreateIndexRequest createIndexRequest = new CreateIndexRequest(indexName); + // This assumes the requested mapping will be merged with mappings from the template, + // and may need to be revisited if template merging is ever refactored try (XContentBuilder termFieldsMapping = ElasticsearchMappings.termFieldsMapping(ElasticsearchMappings.DOC_TYPE, termFields)) { createIndexRequest.mapping(ElasticsearchMappings.DOC_TYPE, termFieldsMapping); } @@ -298,7 +300,8 @@ public class JobProvider { } private void updateIndexMappingWithTermFields(String indexName, Collection termFields, ActionListener listener) { - try (XContentBuilder termFieldsMapping = ElasticsearchMappings.termFieldsMapping(null, termFields)) { + // Put the whole "doc" mapping, not just the term fields, otherwise we'll wipe the _meta section of the mapping + try (XContentBuilder termFieldsMapping = ElasticsearchMappings.docMapping(termFields)) { client.admin().indices().preparePutMapping(indexName).setType(ElasticsearchMappings.DOC_TYPE) .setSource(termFieldsMapping) .execute(new ActionListener() { @@ -312,6 +315,8 @@ public class JobProvider { listener.onFailure(e); } }); + } catch (IOException e) { + listener.onFailure(e); } } diff --git a/plugin/src/test/resources/rest-api-spec/test/ml/ml_anomalies_default_mappings.yml b/plugin/src/test/resources/rest-api-spec/test/ml/ml_anomalies_default_mappings.yml index 468f0229d08..024a9954be0 100644 --- a/plugin/src/test/resources/rest-api-spec/test/ml/ml_anomalies_default_mappings.yml +++ b/plugin/src/test/resources/rest-api-spec/test/ml/ml_anomalies_default_mappings.yml @@ -36,3 +36,54 @@ type: doc fields: new_field - match: {\.ml-anomalies-shared.mappings.doc.new_field.mapping.new_field.type: keyword} + +--- +"Test _meta exists when two jobs share an index": + + - do: + xpack.ml.put_job: + job_id: ml-anomalies-shared-mappings-job1 + body: > + { + "analysis_config" : { + "bucket_span": "1h", + "detectors" :[{"function":"count","by_field_name":"foo"}] + }, + "data_description" : { + "time_field":"time" + } + } + - match: { job_id: "ml-anomalies-shared-mappings-job1" } + + - do: + indices.refresh: + index: .ml-anomalies-shared + + - do: + indices.get_mapping: + index: .ml-anomalies-shared + - is_true: \.ml-anomalies-shared.mappings.doc._meta.version + + - do: + xpack.ml.put_job: + job_id: ml-anomalies-shared-mappings-job2 + body: > + { + "analysis_config" : { + "bucket_span": "1h", + "detectors" :[{"function":"count","by_field_name":"bar"}] + }, + "data_description" : { + "time_field":"time" + } + } + - match: { job_id: "ml-anomalies-shared-mappings-job2" } + + - do: + indices.refresh: + index: .ml-anomalies-shared + + - do: + indices.get_mapping: + index: .ml-anomalies-shared + - is_true: \.ml-anomalies-shared.mappings.doc._meta.version