From e74f336c0a40682aa88d7cc40b11c93d4540408e Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 31 May 2018 16:09:48 +0200 Subject: [PATCH 1/2] Deprecates indexing and querying a context completion field without context (#30712) This change deprecates completion queries and documents without context that target a context enabled completion field. Querying without context degrades the search performance considerably (even when the number of indexed contexts is low). This commit targets master but the deprecation will take place in 6.x and the functionality will be removed in 7 in a follow up. Closes #29222 --- .../suggesters/context-suggest.asciidoc | 10 ++- .../rest-api-spec/test/suggest/30_context.yml | 72 +++++++++++++++++-- .../test/suggest/40_typed_keys.yml | 10 ++- .../index/mapper/CompletionFieldMapper.java | 1 - .../CompletionSuggestionBuilder.java | 1 + .../completion/context/ContextMappings.java | 16 +++++ 6 files changed, 101 insertions(+), 9 deletions(-) diff --git a/docs/reference/search/suggesters/context-suggest.asciidoc b/docs/reference/search/suggesters/context-suggest.asciidoc index 9226c29b9ad6e..2b522062ec06c 100644 --- a/docs/reference/search/suggesters/context-suggest.asciidoc +++ b/docs/reference/search/suggesters/context-suggest.asciidoc @@ -84,6 +84,10 @@ PUT place_path_category NOTE: Adding context mappings increases the index size for completion field. The completion index is entirely heap resident, you can monitor the completion field index size using <>. +NOTE: deprecated[7.0.0, Indexing a suggestion without context on a context enabled completion field is deprecated +and will be removed in the next major release. If you want to index a suggestion that matches all contexts you should +add a special context for it.] + [[suggester-context-category]] [float] ==== Category Context @@ -156,9 +160,9 @@ POST place/_search?pretty // CONSOLE // TEST[continued] -NOTE: When no categories are provided at query-time, all indexed documents are considered. -Querying with no categories on a category enabled completion field should be avoided, as it -will degrade search performance. +Note: deprecated[7.0.0, When no categories are provided at query-time, all indexed documents are considered. +Querying with no categories on a category enabled completion field is deprecated and will be removed in the next major release +as it degrades search performance considerably.] Suggestions with certain categories can be boosted higher than others. The following filters suggestions by categories and additionally boosts diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/suggest/30_context.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/suggest/30_context.yml index f0d97382eeb8e..dfb849fff5700 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/suggest/30_context.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/suggest/30_context.yml @@ -336,16 +336,80 @@ setup: - length: { suggest.result.0.options: 1 } - match: { suggest.result.0.options.0.text: "foo" } +--- +"Indexing and Querying without contexts is deprecated": + - skip: + version: " - 6.99.99" + reason: this feature was deprecated in 7.0 + features: "warnings" + + - do: + index: + index: test + type: test + id: 1 + body: + suggest_context: + input: "foo" + contexts: + color: "red" + suggest_multi_contexts: + input: "bar" + contexts: + color: "blue" + + - do: + warnings: + - "The ability to index a suggestion with no context on a context enabled completion field is deprecated and will be removed in the next major release." + index: + index: test + type: test + id: 2 + body: + suggest_context: + input: "foo" + + - do: + indices.refresh: {} + - do: - search: + warnings: + - "The ability to query with no context on a context enabled completion field is deprecated and will be removed in the next major release." + search: body: suggest: result: text: "foo" completion: - skip_duplicates: true field: suggest_context - length: { suggest.result: 1 } - - length: { suggest.result.0.options: 1 } - - match: { suggest.result.0.options.0.text: "foo" } + + - do: + warnings: + - "The ability to query with no context on a context enabled completion field is deprecated and will be removed in the next major release." + search: + body: + suggest: + result: + text: "foo" + completion: + field: suggest_context + contexts: {} + + - length: { suggest.result: 1 } + + - do: + warnings: + - "The ability to query with no context on a context enabled completion field is deprecated and will be removed in the next major release." + search: + body: + suggest: + result: + text: "foo" + completion: + field: suggest_multi_contexts + contexts: + location: [] + + - length: { suggest.result: 1 } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/suggest/40_typed_keys.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/suggest/40_typed_keys.yml index 139c972eea677..02b398923a5d2 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/suggest/40_typed_keys.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/suggest/40_typed_keys.yml @@ -22,7 +22,9 @@ setup: "type" : "category" - do: - bulk: + warnings: + - "The ability to index a suggestion with no context on a context enabled completion field is deprecated and will be removed in the next major release." + bulk: refresh: true index: test type: test @@ -34,8 +36,14 @@ setup: --- "Test typed keys parameter for suggesters": + - skip: + version: " - 6.99.99" + reason: queying a context suggester with no context was deprecated in 7.0 + features: "warnings" - do: + warnings: + - "The ability to query with no context on a context enabled completion field is deprecated and will be removed in the next major release." search: typed_keys: true body: 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 3d76c48f4c5cb..bf3522653aea4 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java @@ -83,7 +83,6 @@ * for query-time filtering and boosting (see {@link ContextMappings} */ public class CompletionFieldMapper extends FieldMapper implements ArrayValueMapperParser { - public static final String CONTENT_TYPE = "completion"; public static class Defaults { diff --git a/server/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestionBuilder.java b/server/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestionBuilder.java index 3102ddd0e7787..25f2f7fa382fa 100644 --- a/server/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestionBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestionBuilder.java @@ -57,6 +57,7 @@ * indexing. */ public class CompletionSuggestionBuilder extends SuggestionBuilder { + private static final XContentType CONTEXT_BYTES_XCONTENT_TYPE = XContentType.JSON; static final String SUGGESTION_NAME = "completion"; static final ParseField CONTEXTS_FIELD = new ParseField("contexts", "context"); diff --git a/server/src/main/java/org/elasticsearch/search/suggest/completion/context/ContextMappings.java b/server/src/main/java/org/elasticsearch/search/suggest/completion/context/ContextMappings.java index a0a6ce59f0d9d..4d6b53296f157 100644 --- a/server/src/main/java/org/elasticsearch/search/suggest/completion/context/ContextMappings.java +++ b/server/src/main/java/org/elasticsearch/search/suggest/completion/context/ContextMappings.java @@ -25,6 +25,8 @@ import org.apache.lucene.util.CharsRefBuilder; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.Version; +import org.elasticsearch.common.logging.DeprecationLogger; +import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.index.mapper.CompletionFieldMapper; @@ -51,6 +53,10 @@ * for a {@link CompletionFieldMapper} */ public class ContextMappings implements ToXContent { + + private static final DeprecationLogger DEPRECATION_LOGGER = + new DeprecationLogger(Loggers.getLogger(ContextMappings.class)); + private final List contextMappings; private final Map contextNameMap; @@ -143,6 +149,10 @@ protected Iterable contexts() { scratch.setLength(1); } } + if (typedContexts.isEmpty()) { + DEPRECATION_LOGGER.deprecated("The ability to index a suggestion with no context on a context enabled completion field" + + " is deprecated and will be removed in the next major release."); + } return typedContexts; } } @@ -156,6 +166,7 @@ protected Iterable contexts() { */ public ContextQuery toContextQuery(CompletionQuery query, Map> queryContexts) { ContextQuery typedContextQuery = new ContextQuery(query); + boolean hasContext = false; if (queryContexts.isEmpty() == false) { CharsRefBuilder scratch = new CharsRefBuilder(); scratch.grow(1); @@ -169,10 +180,15 @@ public ContextQuery toContextQuery(CompletionQuery query, Map Date: Thu, 31 May 2018 17:38:37 +0200 Subject: [PATCH 2/2] Adapt rest test skip version after backport Relates #30712 --- .../migration/migrate_6_0/search.asciidoc | 8 +++++++- .../rest-api-spec/test/suggest/30_context.yml | 4 ++-- .../test/suggest/40_typed_keys.yml | 20 +++---------------- 3 files changed, 12 insertions(+), 20 deletions(-) diff --git a/docs/reference/migration/migrate_6_0/search.asciidoc b/docs/reference/migration/migrate_6_0/search.asciidoc index 3a2c46506cf60..f491fc97b98a4 100644 --- a/docs/reference/migration/migrate_6_0/search.asciidoc +++ b/docs/reference/migration/migrate_6_0/search.asciidoc @@ -207,4 +207,10 @@ for a particular index with the index setting `index.max_terms_count`. For 6.x and starting in 6.3 a deprecation warning will be printed to warn against search requests that contain extra tokens after the main object. These extra tokens were ignored by the query parser before 6.3 but the next - major version will not accept invalid body anymore. \ No newline at end of file + major version will not accept invalid body anymore. + +==== Context suggester without contexts + +The ability to query and index context enabled suggestions without contexts +has been deprecated. Context enabled suggestion queries without contexts have +to visit every suggestion, which degrades the search performance considerably. \ No newline at end of file diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/suggest/30_context.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/suggest/30_context.yml index dfb849fff5700..7e1f0a9e4ad7f 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/suggest/30_context.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/suggest/30_context.yml @@ -339,8 +339,8 @@ setup: --- "Indexing and Querying without contexts is deprecated": - skip: - version: " - 6.99.99" - reason: this feature was deprecated in 7.0 + version: " - 6.3.99" + reason: this feature was deprecated in 6.4 features: "warnings" - do: diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/suggest/40_typed_keys.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/suggest/40_typed_keys.yml index 02b398923a5d2..b37a5df82fa1c 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/suggest/40_typed_keys.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/suggest/40_typed_keys.yml @@ -20,10 +20,10 @@ setup: - "name" : "format" "type" : "category" +--- +"Test typed keys parameter for suggesters": - do: - warnings: - - "The ability to index a suggestion with no context on a context enabled completion field is deprecated and will be removed in the next major release." bulk: refresh: true index: test @@ -32,18 +32,9 @@ setup: - '{"index": {}}' - '{"title": "Elasticsearch in Action", "suggestions": {"input": "ELK in Action", "contexts": {"format": "ebook"}}}' - '{"index": {}}' - - '{"title": "Elasticsearch - The Definitive Guide", "suggestions": {"input": ["Elasticsearch in Action"]}}' - ---- -"Test typed keys parameter for suggesters": - - skip: - version: " - 6.99.99" - reason: queying a context suggester with no context was deprecated in 7.0 - features: "warnings" + - '{"title": "Elasticsearch - The Definitive Guide", "suggestions": {"input": ["Elasticsearch in Action"], "contexts": {"format": "ebook"}}}' - do: - warnings: - - "The ability to query with no context on a context enabled completion field is deprecated and will be removed in the next major release." search: typed_keys: true body: @@ -54,10 +45,6 @@ setup: term_suggester: term: field: title - completion_suggester: - prefix: "Elastic" - completion: - field: suggestions context_suggester: prefix: "Elastic" completion: @@ -69,6 +56,5 @@ setup: field: title - is_true: suggest.term#term_suggester - - is_true: suggest.completion#completion_suggester - is_true: suggest.completion#context_suggester - is_true: suggest.phrase#phrase_suggester