Skip to content

Commit

Permalink
Suggest should ignore empty shards
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dadoonet committed Aug 16, 2013
1 parent 459d59a commit ac06722
Show file tree
Hide file tree
Showing 7 changed files with 97 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,17 @@ public Suggest execute(SuggestionSearchContext suggest, IndexReader reader) {
try {
CharsRef spare = new CharsRef(); // Maybe add CharsRef to CacheRecycler?
final List<Suggestion<? extends Entry<? extends Option>>> suggestions = new ArrayList<Suggestion<? extends Entry<? extends Option>>>(suggest.suggestions().size());

for (Map.Entry<String, SuggestionSearchContext.SuggestionContext> entry : suggest.suggestions().entrySet()) {
SuggestionSearchContext.SuggestionContext suggestion = entry.getValue();
Suggester<SuggestionContext> suggester = suggestion.getSuggester();
Suggestion<? extends Entry<? extends Option>> 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);
Expand Down
31 changes: 20 additions & 11 deletions src/main/java/org/elasticsearch/search/suggest/Suggester.java
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
/*
* Licensed to ElasticSearch and Shay Banon under one
* 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. ElasticSearch licenses this
* 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
Expand All @@ -16,21 +16,30 @@
* specific language governing permissions and limitations
* under the License.
*/

package org.elasticsearch.search.suggest;
import java.io.IOException;

import org.apache.lucene.index.IndexReader;
import org.apache.lucene.util.CharsRef;
import org.elasticsearch.search.suggest.Suggest.Suggestion;
import org.elasticsearch.search.suggest.Suggest.Suggestion.Entry;
import org.elasticsearch.search.suggest.Suggest.Suggestion.Entry.Option;

public interface Suggester<T extends SuggestionSearchContext.SuggestionContext> {
import java.io.IOException;

public abstract class Suggester<T extends SuggestionSearchContext.SuggestionContext> {

protected abstract Suggest.Suggestion<? extends Suggest.Suggestion.Entry<? extends Suggest.Suggestion.Entry.Option>>
innerExecute(String name, T suggestion, IndexReader indexReader, CharsRef spare) throws IOException;

public abstract String[] names();

public Suggestion<? extends Entry<? extends Option>> execute(String name, T suggestion, IndexReader indexReader, CharsRef spare)
throws IOException;
public abstract SuggestContextParser getContextParser();

public String[] names();
public Suggest.Suggestion<? extends Suggest.Suggestion.Entry<? extends Suggest.Suggestion.Entry.Option>>
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);
}

public SuggestContextParser getContextParser();
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -42,14 +40,14 @@
import java.util.List;
import java.util.Map;

public class CompletionSuggester implements Suggester<CompletionSuggestionContext> {
public class CompletionSuggester extends Suggester<CompletionSuggestionContext> {

private static final ScoreComparator scoreComparator = new ScoreComparator();


@Override
public Suggest.Suggestion<? extends Suggest.Suggestion.Entry<? extends Suggest.Suggestion.Entry.Option>> execute(String name,
protected Suggest.Suggestion<? extends Suggest.Suggestion.Entry<? extends Suggest.Suggestion.Entry.Option>> 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");
}
Expand All @@ -70,7 +68,7 @@ public Suggest.Suggestion<? extends Suggest.Suggestion.Entry<? extends Suggest.S
Lookup lookup = lookupTerms.getLookup(suggestionContext.mapper(), suggestionContext);
List<Lookup.LookupResult> 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);
Expand All @@ -79,8 +77,8 @@ public Suggest.Suggestion<? extends Suggest.Suggestion.Entry<? extends Suggest.S
: new BytesArray(res.payload));
results.put(key, option);
} else if (value.getScore() < score) {
value.setScore(score);
value.setPayload(res.payload == null ? null : new BytesArray(res.payload));
value.setScore(score);
value.setPayload(res.payload == null ? null : new BytesArray(res.payload));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@
package org.elasticsearch.search.suggest.phrase;


import java.io.IOException;
import java.util.List;

import org.apache.lucene.analysis.TokenStream;
import org.apache.lucene.index.IndexReader;
import org.apache.lucene.search.spell.DirectSpellChecker;
Expand All @@ -37,7 +34,10 @@
import org.elasticsearch.search.suggest.SuggestUtils;
import org.elasticsearch.search.suggest.Suggester;

public final class PhraseSuggester implements Suggester<PhraseSuggestionContext> {
import java.io.IOException;
import java.util.List;

public final class PhraseSuggester extends Suggester<PhraseSuggestionContext> {
private final BytesRef SEPARATOR = new BytesRef(" ");

/*
Expand All @@ -49,7 +49,7 @@ public final class PhraseSuggester implements Suggester<PhraseSuggestionContext>
* - phonetic filters could be interesting here too for candidate selection
*/
@Override
public Suggestion<? extends Entry<? extends Option>> execute(String name, PhraseSuggestionContext suggestion,
public Suggestion<? extends Entry<? extends Option>> innerExecute(String name, PhraseSuggestionContext suggestion,
IndexReader indexReader, CharsRef spare) throws IOException {
double realWordErrorLikelihood = suggestion.realworldErrorLikelyhood();
List<PhraseSuggestionContext.DirectCandidateGenerator> generators = suggestion.generators();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -37,10 +33,14 @@
import org.elasticsearch.search.suggest.Suggester;
import org.elasticsearch.search.suggest.SuggestionSearchContext.SuggestionContext;

public final class TermSuggester implements Suggester<TermSuggestionContext> {
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;

public final class TermSuggester extends Suggester<TermSuggestionContext> {

@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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -35,12 +32,12 @@
/**
*
*/
public class CustomSuggester implements Suggester<CustomSuggester.CustomSuggestionsContext> {
public class CustomSuggester extends Suggester<CustomSuggester.CustomSuggestionsContext> {


// This is a pretty dumb implementation which returns the original text + fieldName + custom config option + 12 or 123
@Override
public Suggest.Suggestion<? extends Suggest.Suggestion.Entry<? extends Suggest.Suggestion.Entry.Option>> execute(String name, CustomSuggestionsContext suggestion, IndexReader indexReader, CharsRef spare) throws IOException {
public Suggest.Suggestion<? extends Suggest.Suggestion.Entry<? extends Suggest.Suggestion.Entry.Option>> innerExecute(String name, CustomSuggestionsContext suggestion, IndexReader indexReader, CharsRef spare) throws IOException {
// Get the suggestion context
String text = suggestion.getText().utf8ToString();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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");
}

}

0 comments on commit ac06722

Please sign in to comment.