From 3b827ab2f7324fc1d8de94c9914deaf490b2fc56 Mon Sep 17 00:00:00 2001 From: Tamara Braun Date: Sun, 10 Feb 2019 18:25:06 +0100 Subject: [PATCH 1/5] Enforce Completion Context Limit * Enforcing a maximum number of completion contexts as reqested in #32741 * Closes #32741 --- .../index/mapper/MapperService.java | 16 ++++++++++++++ .../index/mapper/MapperServiceTests.java | 21 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java index 398ce4cdd17ce..19c80c45aba45 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java @@ -108,6 +108,9 @@ public enum MergeReason { Setting.boolSetting("index.mapper.dynamic", INDEX_MAPPER_DYNAMIC_DEFAULT, Property.Dynamic, Property.IndexScope, Property.Deprecated); + // Maximum allowed number of completion contexts in a mapping + public static final int COMPLETION_CONTEXTS_LIMIT = 10; + //TODO this needs to be cleaned up: _timestamp and _ttl are not supported anymore, _field_names, _seq_no, _version and _source are //also missing, not sure if on purpose. See IndicesModule#getMetadataMappers private static final String[] SORTED_META_FIELDS = new String[]{ @@ -497,6 +500,7 @@ private synchronized Map internalMerge(@Nullable Documen ContextMapping.validateContextPaths(indexSettings.getIndexVersionCreated(), fieldMappers, fieldTypes::get); if (reason == MergeReason.MAPPING_UPDATE) { + checkCompletionContextsLimit(fieldMappers); // this check will only be performed on the master node when there is // a call to the update mapping API. For all other cases like // the master node restoring mappings from disk or data nodes @@ -557,6 +561,18 @@ private synchronized Map internalMerge(@Nullable Documen return results; } + private void checkCompletionContextsLimit(List fieldMappers) { + for (FieldMapper fieldMapper : fieldMappers) { + if (CompletionFieldMapper.CONTENT_TYPE.equals(fieldMapper.typeName())) { + CompletionFieldMapper.CompletionFieldType fieldType = ((CompletionFieldMapper) fieldMapper).fieldType(); + if (fieldType.hasContextMappings() && fieldType.getContextMappings().size() > COMPLETION_CONTEXTS_LIMIT) { + throw new IllegalArgumentException("Limit of contexts [" + COMPLETION_CONTEXTS_LIMIT + "] in index [" + + index().getName() + "] has been exceeded"); + } + } + } + } + private boolean assertMappersShareSameFieldType() { if (mapper != null) { List fieldMappers = new ArrayList<>(); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java b/server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java index 2c1a75b40d4c0..cd5e383bc3998 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java @@ -323,4 +323,25 @@ public void testDefaultMappingIsRejectedOn7() throws IOException { + " that indices can have at most one type.", e.getMessage()); } + public void testLimitOfContextMappings() throws Throwable { + final String index = "test"; + XContentBuilder mappingBuilder = XContentFactory.jsonBuilder().startObject().startObject("properties") + .startObject("suggest").field("type", "completion").startArray("contexts"); + for (int i = 0; i < MapperService.COMPLETION_CONTEXTS_LIMIT + 1; i++) { + mappingBuilder.startObject(); + mappingBuilder.field("name", Integer.toString(i)); + mappingBuilder.field("type", "category"); + mappingBuilder.endObject(); + } + + mappingBuilder.endArray().endObject().endObject().endObject(); + String mappings = Strings.toString(mappingBuilder); + + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> { + createIndex(index).mapperService().merge("type1", new CompressedXContent(mappings), MergeReason.MAPPING_UPDATE); + }); + assertTrue(e.getMessage(), + e.getMessage().contains("Limit of contexts [" + MapperService.COMPLETION_CONTEXTS_LIMIT + + "] in index [" + index + "] has been exceeded")); + } } From 7b4a783267df0d275a811b13c676635b5e9df872 Mon Sep 17 00:00:00 2001 From: Tamara Braun Date: Mon, 11 Feb 2019 21:43:32 +0100 Subject: [PATCH 2/5] move functionality to different class --- .../index/mapper/CompletionFieldMapper.java | 25 +++++++++++++++++++ .../index/mapper/MapperService.java | 16 ------------ .../mapper/CompletionFieldMapperTests.java | 22 ++++++++++++++++ .../index/mapper/MapperServiceTests.java | 21 ---------------- 4 files changed, 47 insertions(+), 37 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java index df6a291372f64..d31045758026d 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java @@ -18,6 +18,7 @@ */ package org.elasticsearch.index.mapper; +import org.apache.logging.log4j.LogManager; import org.apache.lucene.codecs.PostingsFormat; import org.apache.lucene.index.IndexableField; import org.apache.lucene.index.Term; @@ -31,8 +32,10 @@ import org.apache.lucene.search.suggest.document.RegexCompletionQuery; import org.apache.lucene.search.suggest.document.SuggestField; import org.elasticsearch.Version; +import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParsingException; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.Fuzziness; import org.elasticsearch.common.util.set.Sets; @@ -85,6 +88,11 @@ public class CompletionFieldMapper extends FieldMapper implements ArrayValueMapperParser { public static final String CONTENT_TYPE = "completion"; + /** + * Maximum allowed number of completion contexts in a mapping. + */ + static final int COMPLETION_CONTEXTS_LIMIT = 10; + public static class Defaults { public static final MappedFieldType FIELD_TYPE = new CompletionFieldType(); static { @@ -354,6 +362,8 @@ public static class Builder extends FieldMapper.Builder COMPLETION_CONTEXTS_LIMIT) { + if (context.indexCreatedVersion().onOrAfter(Version.V_8_0_0)) { + throw new IllegalArgumentException( + "Limit of completion field contexts [" + COMPLETION_CONTEXTS_LIMIT + "] has been exceeded"); + } else { + deprecationLogger.deprecated("You have defined more than [" + COMPLETION_CONTEXTS_LIMIT + "] completion contexts" + + " in the mapping for index [" + context.indexSettings().get(IndexMetaData.SETTING_INDEX_PROVIDED_NAME) + "]. " + + "The maximum allowed number of completion contexts in a mapping will be limited to " + + "[" + COMPLETION_CONTEXTS_LIMIT + "] starting in version [8.0]."); + } + } + } } private int maxInputLength; diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java index 19c80c45aba45..398ce4cdd17ce 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java @@ -108,9 +108,6 @@ public enum MergeReason { Setting.boolSetting("index.mapper.dynamic", INDEX_MAPPER_DYNAMIC_DEFAULT, Property.Dynamic, Property.IndexScope, Property.Deprecated); - // Maximum allowed number of completion contexts in a mapping - public static final int COMPLETION_CONTEXTS_LIMIT = 10; - //TODO this needs to be cleaned up: _timestamp and _ttl are not supported anymore, _field_names, _seq_no, _version and _source are //also missing, not sure if on purpose. See IndicesModule#getMetadataMappers private static final String[] SORTED_META_FIELDS = new String[]{ @@ -500,7 +497,6 @@ private synchronized Map internalMerge(@Nullable Documen ContextMapping.validateContextPaths(indexSettings.getIndexVersionCreated(), fieldMappers, fieldTypes::get); if (reason == MergeReason.MAPPING_UPDATE) { - checkCompletionContextsLimit(fieldMappers); // this check will only be performed on the master node when there is // a call to the update mapping API. For all other cases like // the master node restoring mappings from disk or data nodes @@ -561,18 +557,6 @@ private synchronized Map internalMerge(@Nullable Documen return results; } - private void checkCompletionContextsLimit(List fieldMappers) { - for (FieldMapper fieldMapper : fieldMappers) { - if (CompletionFieldMapper.CONTENT_TYPE.equals(fieldMapper.typeName())) { - CompletionFieldMapper.CompletionFieldType fieldType = ((CompletionFieldMapper) fieldMapper).fieldType(); - if (fieldType.hasContextMappings() && fieldType.getContextMappings().size() > COMPLETION_CONTEXTS_LIMIT) { - throw new IllegalArgumentException("Limit of contexts [" + COMPLETION_CONTEXTS_LIMIT + "] in index [" - + index().getName() + "] has been exceeded"); - } - } - } - } - private boolean assertMappersShareSameFieldType() { if (mapper != null) { List fieldMappers = new ArrayList<>(); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java index 7354af17043eb..7d16c01e9daaf 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java @@ -908,6 +908,28 @@ public void testEmptyName() throws IOException { assertThat(e.getMessage(), containsString("name cannot be empty string")); } + public void testLimitOfContextMappings() throws Throwable { + final String index = "test"; + XContentBuilder mappingBuilder = XContentFactory.jsonBuilder().startObject().startObject("properties") + .startObject("suggest").field("type", "completion").startArray("contexts"); + for (int i = 0; i < CompletionFieldMapper.COMPLETION_CONTEXTS_LIMIT + 1; i++) { + mappingBuilder.startObject(); + mappingBuilder.field("name", Integer.toString(i)); + mappingBuilder.field("type", "category"); + mappingBuilder.endObject(); + } + + mappingBuilder.endArray().endObject().endObject().endObject(); + String mappings = Strings.toString(mappingBuilder); + + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> { + createIndex(index).mapperService().documentMapperParser().parse("type1", new CompressedXContent(mappings)); + }); + assertTrue(e.getMessage(), + e.getMessage().contains("Limit of completion field contexts [" + + CompletionFieldMapper.COMPLETION_CONTEXTS_LIMIT + "] has been exceeded")); + } + private Matcher suggestField(String value) { return Matchers.allOf(hasProperty(IndexableField::stringValue, equalTo(value)), Matchers.instanceOf(SuggestField.class)); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java b/server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java index cd5e383bc3998..2c1a75b40d4c0 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java @@ -323,25 +323,4 @@ public void testDefaultMappingIsRejectedOn7() throws IOException { + " that indices can have at most one type.", e.getMessage()); } - public void testLimitOfContextMappings() throws Throwable { - final String index = "test"; - XContentBuilder mappingBuilder = XContentFactory.jsonBuilder().startObject().startObject("properties") - .startObject("suggest").field("type", "completion").startArray("contexts"); - for (int i = 0; i < MapperService.COMPLETION_CONTEXTS_LIMIT + 1; i++) { - mappingBuilder.startObject(); - mappingBuilder.field("name", Integer.toString(i)); - mappingBuilder.field("type", "category"); - mappingBuilder.endObject(); - } - - mappingBuilder.endArray().endObject().endObject().endObject(); - String mappings = Strings.toString(mappingBuilder); - - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> { - createIndex(index).mapperService().merge("type1", new CompressedXContent(mappings), MergeReason.MAPPING_UPDATE); - }); - assertTrue(e.getMessage(), - e.getMessage().contains("Limit of contexts [" + MapperService.COMPLETION_CONTEXTS_LIMIT + - "] in index [" + index + "] has been exceeded")); - } } From 2fb264a7f1d55a6285c3fb0810bdc79beb56bdf6 Mon Sep 17 00:00:00 2001 From: Tamara Braun Date: Mon, 11 Feb 2019 22:27:25 +0100 Subject: [PATCH 3/5] fix npe --- .../org/elasticsearch/index/mapper/CompletionFieldMapper.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java index d31045758026d..5fd06633bcfc8 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java @@ -418,7 +418,7 @@ public CompletionFieldMapper build(BuilderContext context) { } private void checkCompletionContextsLimit(BuilderContext context) { - if (this.contextMappings.size() > COMPLETION_CONTEXTS_LIMIT) { + if (this.contextMappings != null && this.contextMappings.size() > COMPLETION_CONTEXTS_LIMIT) { if (context.indexCreatedVersion().onOrAfter(Version.V_8_0_0)) { throw new IllegalArgumentException( "Limit of completion field contexts [" + COMPLETION_CONTEXTS_LIMIT + "] has been exceeded"); From 50d3e89a8a476c8dce3145a1734d5ab1c3d89a48 Mon Sep 17 00:00:00 2001 From: Tamara Braun Date: Tue, 12 Feb 2019 19:19:18 +0100 Subject: [PATCH 4/5] add docs --- docs/reference/search/suggesters/context-suggest.asciidoc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/reference/search/suggesters/context-suggest.asciidoc b/docs/reference/search/suggesters/context-suggest.asciidoc index 63692f0b06f82..77dd5745e159b 100644 --- a/docs/reference/search/suggesters/context-suggest.asciidoc +++ b/docs/reference/search/suggesters/context-suggest.asciidoc @@ -16,6 +16,8 @@ the field mapping. NOTE: It is mandatory to provide a context when indexing and querying a context enabled completion field. +NOTE: The maximum allowed number of completion field context mappings is 10. + The following defines types, each with two context mappings for a completion field: From ce29428e032601e2a27b12a2797c879836243979 Mon Sep 17 00:00:00 2001 From: jimczi Date: Mon, 18 Feb 2019 09:19:10 +0100 Subject: [PATCH 5/5] add breaking change entry --- docs/reference/migration/migrate_8_0.asciidoc | 20 ++++++++++++++++++- .../migration/migrate_8_0/mappings.asciidoc | 9 +++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 docs/reference/migration/migrate_8_0/mappings.asciidoc diff --git a/docs/reference/migration/migrate_8_0.asciidoc b/docs/reference/migration/migrate_8_0.asciidoc index 4477090dc16b6..63e503cc43990 100644 --- a/docs/reference/migration/migrate_8_0.asciidoc +++ b/docs/reference/migration/migrate_8_0.asciidoc @@ -9,4 +9,22 @@ your application to {es} 8.0. See also <> and <>. -coming[8.0.0] \ No newline at end of file +* <> + +[float] +=== Indices created before 7.0 + +Elasticsearch 8.0 can read indices created in version 7.0 or above. An +Elasticsearch 8.0 node will not start in the presence of indices created in a +version of Elasticsearch before 7.0. + +[IMPORTANT] +.Reindex indices from Elasticsearch 6.x or before +========================================= + +Indices created in Elasticsearch 6.x or before will need to be reindexed with +Elasticsearch 7.x in order to be readable by Elasticsearch 8.x. + +========================================= + +include::migrate_8_0/mappings.asciidoc[] \ No newline at end of file diff --git a/docs/reference/migration/migrate_8_0/mappings.asciidoc b/docs/reference/migration/migrate_8_0/mappings.asciidoc new file mode 100644 index 0000000000000..f777b8775f872 --- /dev/null +++ b/docs/reference/migration/migrate_8_0/mappings.asciidoc @@ -0,0 +1,9 @@ +[float] +[[breaking_90_mappings_changes]] +=== Mapping changes + +[float] +==== Limiting the number of completion contexts + +The number of completion contexts within a single completion field +has been limited to 10. \ No newline at end of file