From 3b717f23eeda88cff5d45a94804894d8b83db855 Mon Sep 17 00:00:00 2001 From: David Pilato Date: Fri, 9 Aug 2013 12:53:43 +0200 Subject: [PATCH] Suggest should ignore empty shards From #3469. When running suggest on empty shards, it raises an error like: ``` "failures" : [ { "status" : 400, "reason" : "ElasticSearchIllegalArgumentException[generator field [title] doesn't exist]" } ] ``` We should ignore empty shards. Closes #3473. --- .../search/suggest/AbstractSuggester.java | 41 +++++++++++++++ .../search/suggest/SuggestPhase.java | 8 ++- .../completion/CompletionSuggester.java | 16 +++--- .../suggest/phrase/PhraseSuggester.java | 12 ++--- .../search/suggest/term/TermSuggester.java | 14 +++--- .../search/suggest/CustomSuggester.java | 9 ++-- .../search/suggest/SuggestSearchTests.java | 50 +++++++++++++++++++ 7 files changed, 120 insertions(+), 30 deletions(-) create mode 100644 src/main/java/org/elasticsearch/search/suggest/AbstractSuggester.java diff --git a/src/main/java/org/elasticsearch/search/suggest/AbstractSuggester.java b/src/main/java/org/elasticsearch/search/suggest/AbstractSuggester.java new file mode 100644 index 0000000000000..27d89545028dc --- /dev/null +++ b/src/main/java/org/elasticsearch/search/suggest/AbstractSuggester.java @@ -0,0 +1,41 @@ +/* + * Licensed to Elasticsearch (the "Author") under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Author licenses this + * file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.search.suggest; + +import org.apache.lucene.index.IndexReader; +import org.apache.lucene.util.CharsRef; + +import java.io.IOException; + +public abstract class AbstractSuggester implements Suggester { + + protected abstract Suggest.Suggestion> + innerExecute(String name, T suggestion, IndexReader indexReader, CharsRef spare) throws IOException; + + public Suggest.Suggestion> + execute(String name, T suggestion, IndexReader indexReader, CharsRef spare) throws IOException { + // #3469 We want to ignore empty shards + if (indexReader.numDocs() == 0) { + return null; + } + return innerExecute(name, suggestion, indexReader, spare); + } + +} diff --git a/src/main/java/org/elasticsearch/search/suggest/SuggestPhase.java b/src/main/java/org/elasticsearch/search/suggest/SuggestPhase.java index b9e5687ae228d..8216556f6db03 100644 --- a/src/main/java/org/elasticsearch/search/suggest/SuggestPhase.java +++ b/src/main/java/org/elasticsearch/search/suggest/SuggestPhase.java @@ -78,13 +78,17 @@ public Suggest execute(SuggestionSearchContext suggest, IndexReader reader) { try { CharsRef spare = new CharsRef(); // Maybe add CharsRef to CacheRecycler? final List>> suggestions = new ArrayList>>(suggest.suggestions().size()); + for (Map.Entry entry : suggest.suggestions().entrySet()) { SuggestionSearchContext.SuggestionContext suggestion = entry.getValue(); Suggester suggester = suggestion.getSuggester(); Suggestion> result = suggester.execute(entry.getKey(), suggestion, reader, spare); - assert entry.getKey().equals(result.name); - suggestions.add(result); + if (result != null) { + assert entry.getKey().equals(result.name); + suggestions.add(result); + } } + return new Suggest(Suggest.Fields.SUGGEST, suggestions); } catch (IOException e) { throw new ElasticSearchException("I/O exception during suggest phase", e); diff --git a/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggester.java b/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggester.java index 6cc9c6ab0a3e8..2bb70bed97886 100644 --- a/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggester.java +++ b/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggester.java @@ -31,9 +31,7 @@ import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.text.StringText; import org.elasticsearch.index.mapper.core.CompletionFieldMapper; -import org.elasticsearch.search.suggest.Suggest; -import org.elasticsearch.search.suggest.SuggestContextParser; -import org.elasticsearch.search.suggest.Suggester; +import org.elasticsearch.search.suggest.*; import org.elasticsearch.search.suggest.completion.CompletionSuggestion.Entry.Option; import java.io.IOException; @@ -42,14 +40,14 @@ import java.util.List; import java.util.Map; -public class CompletionSuggester implements Suggester { +public class CompletionSuggester extends AbstractSuggester { private static final ScoreComparator scoreComparator = new ScoreComparator(); + @Override - public Suggest.Suggestion> execute(String name, + protected Suggest.Suggestion> innerExecute(String name, CompletionSuggestionContext suggestionContext, IndexReader indexReader, CharsRef spare) throws IOException { - if (suggestionContext.mapper() == null || !(suggestionContext.mapper() instanceof CompletionFieldMapper)) { throw new ElasticSearchException("Field [" + suggestionContext.getField() + "] is not a completion suggest field"); } @@ -70,7 +68,7 @@ public Suggest.Suggestion lookupResults = lookup.lookup(spare, false, suggestionContext.getSize()); for (Lookup.LookupResult res : lookupResults) { - + final String key = res.key.toString(); final float score = res.value; final Option value = results.get(key); @@ -79,8 +77,8 @@ public Suggest.Suggestion { +import java.io.IOException; +import java.util.List; + +public final class PhraseSuggester extends AbstractSuggester { private final BytesRef SEPARATOR = new BytesRef(" "); /* @@ -49,7 +49,7 @@ public final class PhraseSuggester implements Suggester * - phonetic filters could be interesting here too for candidate selection */ @Override - public Suggestion> execute(String name, PhraseSuggestionContext suggestion, + public Suggestion> innerExecute(String name, PhraseSuggestionContext suggestion, IndexReader indexReader, CharsRef spare) throws IOException { double realWordErrorLikelihood = suggestion.realworldErrorLikelyhood(); List generators = suggestion.generators(); diff --git a/src/main/java/org/elasticsearch/search/suggest/term/TermSuggester.java b/src/main/java/org/elasticsearch/search/suggest/term/TermSuggester.java index a4783c5d55957..55cc4ef98e7a0 100644 --- a/src/main/java/org/elasticsearch/search/suggest/term/TermSuggester.java +++ b/src/main/java/org/elasticsearch/search/suggest/term/TermSuggester.java @@ -18,10 +18,6 @@ */ package org.elasticsearch.search.suggest.term; -import java.io.IOException; -import java.util.ArrayList; -import java.util.List; - import org.apache.lucene.index.IndexReader; import org.apache.lucene.index.Term; import org.apache.lucene.search.spell.DirectSpellChecker; @@ -32,15 +28,19 @@ import org.elasticsearch.common.text.BytesText; import org.elasticsearch.common.text.StringText; import org.elasticsearch.common.text.Text; +import org.elasticsearch.search.suggest.AbstractSuggester; import org.elasticsearch.search.suggest.SuggestContextParser; import org.elasticsearch.search.suggest.SuggestUtils; -import org.elasticsearch.search.suggest.Suggester; import org.elasticsearch.search.suggest.SuggestionSearchContext.SuggestionContext; -public final class TermSuggester implements Suggester { +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +public final class TermSuggester extends AbstractSuggester { @Override - public TermSuggestion execute(String name, TermSuggestionContext suggestion, IndexReader indexReader, CharsRef spare) throws IOException { + public TermSuggestion innerExecute(String name, TermSuggestionContext suggestion, IndexReader indexReader, CharsRef spare) throws IOException { DirectSpellChecker directSpellChecker = SuggestUtils.getDirectSpellChecker(suggestion.getDirectSpellCheckerSettings()); TermSuggestion response = new TermSuggestion( diff --git a/src/test/java/org/elasticsearch/test/integration/search/suggest/CustomSuggester.java b/src/test/java/org/elasticsearch/test/integration/search/suggest/CustomSuggester.java index 5914b3a6f1d65..e475ae88cc426 100644 --- a/src/test/java/org/elasticsearch/test/integration/search/suggest/CustomSuggester.java +++ b/src/test/java/org/elasticsearch/test/integration/search/suggest/CustomSuggester.java @@ -23,10 +23,7 @@ import org.elasticsearch.common.text.StringText; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.mapper.MapperService; -import org.elasticsearch.search.suggest.Suggest; -import org.elasticsearch.search.suggest.SuggestContextParser; -import org.elasticsearch.search.suggest.Suggester; -import org.elasticsearch.search.suggest.SuggestionSearchContext; +import org.elasticsearch.search.suggest.*; import java.io.IOException; import java.util.Locale; @@ -35,12 +32,12 @@ /** * */ -public class CustomSuggester implements Suggester { +public class CustomSuggester extends AbstractSuggester { // This is a pretty dumb implementation which returns the original text + fieldName + custom config option + 12 or 123 @Override - public Suggest.Suggestion> execute(String name, CustomSuggestionsContext suggestion, IndexReader indexReader, CharsRef spare) throws IOException { + public Suggest.Suggestion> innerExecute(String name, CustomSuggestionsContext suggestion, IndexReader indexReader, CharsRef spare) throws IOException { // Get the suggestion context String text = suggestion.getText().utf8ToString(); diff --git a/src/test/java/org/elasticsearch/test/integration/search/suggest/SuggestSearchTests.java b/src/test/java/org/elasticsearch/test/integration/search/suggest/SuggestSearchTests.java index 839ba09174a43..363a7b1ed4e52 100644 --- a/src/test/java/org/elasticsearch/test/integration/search/suggest/SuggestSearchTests.java +++ b/src/test/java/org/elasticsearch/test/integration/search/suggest/SuggestSearchTests.java @@ -240,6 +240,9 @@ public void testEmpty() throws Exception { .put(SETTING_NUMBER_OF_REPLICAS, 0)) .execute().actionGet(); client().admin().cluster().prepareHealth("test").setWaitForGreenStatus().execute().actionGet(); + client().prepareIndex("test", "type1", "1") + .setSource(XContentFactory.jsonBuilder().startObject().field("foo", "bar").endObject()).execute().actionGet(); + client().admin().indices().prepareRefresh().execute().actionGet(); Suggest suggest = searchSuggest(client(), termSuggestion("test").suggestMode("always") // Always, otherwise the results can vary between requests. .text("abcd") @@ -1078,4 +1081,51 @@ public void testDifferentShardSize() throws Exception { assertThat(suggest.getSuggestion("simple").getEntries().get(0).getOptions().size(), equalTo(3)); } + @Test // see #3469 + public void testEmptyShards() throws IOException, InterruptedException { + Builder builder = ImmutableSettings.builder(); + builder.put("index.number_of_shards", 5).put("index.number_of_replicas", 0); + builder.put("index.analysis.analyzer.suggest.tokenizer", "standard"); + builder.putArray("index.analysis.analyzer.suggest.filter", "standard", "lowercase", "shingler"); + builder.put("index.analysis.filter.shingler.type", "shingle"); + builder.put("index.analysis.filter.shingler.min_shingle_size", 2); + builder.put("index.analysis.filter.shingler.max_shingle_size", 5); + builder.put("index.analysis.filter.shingler.output_unigrams", true); + + XContentBuilder mapping = XContentFactory.jsonBuilder().startObject().startObject("type1") + .startObject("properties") + .startObject("name") + .field("type", "multi_field") + .field("path", "just_name") + .startObject("fields") + .startObject("name") + .field("type", "string") + .field("analyzer", "suggest") + .endObject() + .endObject() + .endObject() + .endObject() + .endObject().endObject(); + client().admin().indices().prepareDelete().execute().actionGet(); + client().admin().indices().prepareCreate("test").setSettings(builder.build()).addMapping("type1", mapping).execute().actionGet(); + client().admin().cluster().prepareHealth("test").setWaitForGreenStatus().execute().actionGet(); + client().prepareIndex("test", "type2", "1") + .setSource(XContentFactory.jsonBuilder().startObject().field("foo", "bar").endObject()).execute().actionGet(); + client().prepareIndex("test", "type2", "2") + .setSource(XContentFactory.jsonBuilder().startObject().field("foo", "bar").endObject()).execute().actionGet(); + client().prepareIndex("test", "type1", "1") + .setSource(XContentFactory.jsonBuilder().startObject().field("name", "Just testing the suggestions api").endObject()).execute().actionGet(); + client().prepareIndex("test", "type1", "2") + .setSource(XContentFactory.jsonBuilder().startObject().field("name", "An other title").endObject()).execute().actionGet(); + client().admin().indices().prepareRefresh().execute().actionGet(); + + SearchRequestBuilder suggestBuilder = client().prepareSearch().setSearchType(SearchType.COUNT); + suggestBuilder.setSuggestText("tetsting sugestion"); + suggestBuilder.addSuggestion(phraseSuggestion("did_you_mean").field("name").maxErrors(5.0f)); + SearchResponse searchResponse = suggestBuilder.execute().actionGet(); + + ElasticsearchAssertions.assertNoFailures(searchResponse); + ElasticsearchAssertions.assertSuggestion(searchResponse.getSuggest(), 0, 0, "did_you_mean", "testing suggestions"); + } + }