diff options
Diffstat (limited to 'container-search/src/main/java/com')
80 files changed, 970 insertions, 848 deletions
diff --git a/container-search/src/main/java/com/yahoo/prelude/Index.java b/container-search/src/main/java/com/yahoo/prelude/Index.java index 65d5879b004..0dfbf6470ad 100644 --- a/container-search/src/main/java/com/yahoo/prelude/Index.java +++ b/container-search/src/main/java/com/yahoo/prelude/Index.java @@ -1,7 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.prelude; - import com.yahoo.language.process.StemMode; import java.util.ArrayList; @@ -10,7 +9,6 @@ import java.util.Iterator; import java.util.List; import java.util.Set; - /** * Information about configured settings of a field or field collection (an actual index or not) in a search definition. * There are two types of settings: @@ -26,6 +24,7 @@ import java.util.Set; public class Index { public static class Attribute { + private boolean tokenizedContent = false; public final String name; @@ -64,6 +63,7 @@ public class Index { private boolean normalize = false; private boolean literalBoost = false; private boolean numerical = false; + private boolean predicate = false; private long predicateUpperBound = Long.MAX_VALUE; private long predicateLowerBound = Long.MIN_VALUE; @@ -73,8 +73,8 @@ public class Index { private boolean isNGram = false; private int gramSize = 2; - /** Whether implicit phrases should lead to a phrase item or an and item */ - private boolean phraseSegmenting = true; + /** Whether implicit phrases should lead to a phrase item or an and item. */ + private Boolean phraseSegmenting = false; /** The string terminating an exact token in this index, or null to use the default (space) */ private String exactTerminator = null; @@ -182,6 +182,8 @@ public class Index { setLiteralBoost(true); } else if (commandString.equals("numerical")) { setNumerical(true); + } else if (commandString.equals("predicate")) { + setPredicate(true); } else if (commandString.startsWith("predicate-bounds ")) { setPredicateBounds(commandString.substring(17)); } else if (commandString.equals("phrase-segmenting")) { @@ -207,20 +209,12 @@ public class Index { } } - /** - * Whether terms in this field are lower cased when indexing. - * - * @param lowercase true if terms are lowercased - */ + /** Sets whether terms in this field are lowercased when indexing. */ public void setLowercase(boolean lowercase) { this.lowercase = lowercase; } - /** - * Whether terms in this field are lower cased when indexing. - * - * @return true if terms are lowercased - */ + /** Returns whether terms in this field are lowercased when indexing. */ public boolean isLowercase() { return lowercase; } @@ -313,6 +307,10 @@ public class Index { public boolean isNumerical() { return numerical; } + public void setPredicate(boolean isPredicate) { this.predicate = isPredicate; } + + public boolean isPredicate() { return predicate; } + public long getPredicateUpperBound() { return predicateUpperBound; } public long getPredicateLowerBound() { return predicateLowerBound; } diff --git a/container-search/src/main/java/com/yahoo/prelude/IndexFacts.java b/container-search/src/main/java/com/yahoo/prelude/IndexFacts.java index 76eef33d6c0..aa3d6a2c0f8 100644 --- a/container-search/src/main/java/com/yahoo/prelude/IndexFacts.java +++ b/container-search/src/main/java/com/yahoo/prelude/IndexFacts.java @@ -48,7 +48,6 @@ public class IndexFacts { static final String unionName = "unionOfAllKnown"; /** A search definition which contains the union of all settings. */ - @SuppressWarnings("deprecation") private SearchDefinition unionSearchDefinition = new SearchDefinition(unionName); private boolean frozen; diff --git a/container-search/src/main/java/com/yahoo/prelude/IndexModel.java b/container-search/src/main/java/com/yahoo/prelude/IndexModel.java index 062a514056b..00935392683 100644 --- a/container-search/src/main/java/com/yahoo/prelude/IndexModel.java +++ b/container-search/src/main/java/com/yahoo/prelude/IndexModel.java @@ -109,7 +109,6 @@ public final class IndexModel { return searchDefinitions; } - @SuppressWarnings("deprecation") private SearchDefinition unionOf(Collection<SearchDefinition> searchDefinitions) { SearchDefinition union = new SearchDefinition(IndexFacts.unionName); diff --git a/container-search/src/main/java/com/yahoo/prelude/Location.java b/container-search/src/main/java/com/yahoo/prelude/Location.java index 37284bd6bcc..908bf835e3c 100644 --- a/container-search/src/main/java/com/yahoo/prelude/Location.java +++ b/container-search/src/main/java/com/yahoo/prelude/Location.java @@ -126,8 +126,8 @@ public class Location { if (ns < -90.1 || ns > +90.1) { throw new IllegalArgumentException("n/s location must be in range [-90,+90]"); } - if (radius_in_degrees < 0 || radius_in_degrees > 180.0) { - throw new IllegalArgumentException("radius must be in range [0,180] degrees, approximately upto 20000km"); + if (radius_in_degrees < 0) { + pr = 512 * 1024 * 1024; } x = px; y = py; diff --git a/container-search/src/main/java/com/yahoo/prelude/query/AndSegmentItem.java b/container-search/src/main/java/com/yahoo/prelude/query/AndSegmentItem.java index bac227ac3e3..55e16804602 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/AndSegmentItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/AndSegmentItem.java @@ -31,14 +31,17 @@ public class AndSegmentItem extends SegmentItem implements BlockItem { } } + @Override public ItemType getItemType() { return ItemType.AND; } + @Override public String getName() { return "SAND"; } + @Override public String getIndexName() { if (getItemCount() == 0) { return ""; @@ -54,4 +57,5 @@ public class AndSegmentItem extends SegmentItem implements BlockItem { i.next().setWeight(w); } } + } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/BlockItem.java b/container-search/src/main/java/com/yahoo/prelude/query/BlockItem.java index 13673144a0a..d0ffcd2d0e0 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/BlockItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/BlockItem.java @@ -3,10 +3,9 @@ package com.yahoo.prelude.query; /** - * An interface used for anything which represents a single block - * of query input. + * An interface used for anything which represents a single block of query input. * - * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a> + * @author Steinar Knutsen */ public interface BlockItem extends HasIndexItem { @@ -39,4 +38,5 @@ public interface BlockItem extends HasIndexItem { * is necessary to change operator? */ SegmentingRule getSegmentingRule(); + } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/IndexedSegmentItem.java b/container-search/src/main/java/com/yahoo/prelude/query/IndexedSegmentItem.java index a06009e642a..300d40d4366 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/IndexedSegmentItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/IndexedSegmentItem.java @@ -79,4 +79,5 @@ public abstract class IndexedSegmentItem extends TaggableSegmentItem implements super.disclose(discloser); discloser.addProperty("index", index); } + } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/NearestNeighborItem.java b/container-search/src/main/java/com/yahoo/prelude/query/NearestNeighborItem.java index 35b87ec0190..e8fa70afd1b 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/NearestNeighborItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/NearestNeighborItem.java @@ -20,6 +20,8 @@ import java.nio.ByteBuffer; public class NearestNeighborItem extends SimpleTaggableItem { private int targetNumHits = 0; + private int hnswExploreAdditionalHits = 0; + private boolean approximate = true; private String field; private String queryTensorName; @@ -34,12 +36,24 @@ public class NearestNeighborItem extends SimpleTaggableItem { /** Returns the field name */ public String getIndexName() { return field; } + /** Returns the number of extra hits to explore in HNSW algorithm */ + public int getHnswExploreAdditionalHits() { return hnswExploreAdditionalHits; } + + /** Returns whether approximation is allowed */ + public boolean getAllowApproximate() { return approximate; } + /** Returns the name of the query tensor */ public String getQueryTensorName() { return queryTensorName; } /** Set the K number of hits to produce */ public void setTargetNumHits(int target) { this.targetNumHits = target; } + /** Set the number of extra hits to explore in HNSW algorithm */ + public void setHnswExploreAdditionalHits(int num) { this.hnswExploreAdditionalHits = num; } + + /** Set whether approximation is allowed */ + public void setAllowApproximate(boolean value) { this.approximate = value; } + @Override public void setIndexName(String index) { this.field = index; } @@ -58,6 +72,8 @@ public class NearestNeighborItem extends SimpleTaggableItem { putString(field, buffer); putString(queryTensorName, buffer); IntegerCompressor.putCompressedPositiveNumber(targetNumHits, buffer); + IntegerCompressor.putCompressedPositiveNumber((approximate ? 1 : 0), buffer); + IntegerCompressor.putCompressedPositiveNumber(hnswExploreAdditionalHits, buffer); return 1; // number of encoded stack dump items } @@ -65,6 +81,9 @@ public class NearestNeighborItem extends SimpleTaggableItem { protected void appendBodyString(StringBuilder buffer) { buffer.append("{field=").append(field); buffer.append(",queryTensorName=").append(queryTensorName); + buffer.append(",hnsw.exploreAdditionalHits=").append(hnswExploreAdditionalHits); + buffer.append(",approximate=").append(String.valueOf(approximate)); buffer.append(",targetNumHits=").append(targetNumHits).append("}"); } + } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/AllParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/AllParser.java index d9b969757c2..49bdba2c90f 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/AllParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/AllParser.java @@ -30,6 +30,7 @@ public class AllParser extends SimpleParser { super(environment); } + @Override protected Item parseItems() { int position = tokens.getPosition(); try { diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/AnyParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/AnyParser.java index dd836e9c8e1..b714a1d8b34 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/AnyParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/AnyParser.java @@ -35,21 +35,12 @@ public class AnyParser extends SimpleParser { return anyItems(true); } - Item parseFilter(String filter, Language queryLanguage, Set<String> searchDefinitions) { - return parseFilter(filter, queryLanguage, environment.getIndexFacts().newSession(searchDefinitions, Collections.emptySet())); - } - Item parseFilter(String filter, Language queryLanguage, IndexFacts.Session indexFacts) { - Item filterRoot; - setState(queryLanguage, indexFacts); tokenize(filter, null, indexFacts, queryLanguage); - filterRoot = anyItems(true); - - if (filterRoot == null) { - return null; - } + Item filterRoot = anyItems(true); + if (filterRoot == null) return null; markAllTermsAsFilters(filterRoot); return filterRoot; @@ -61,18 +52,10 @@ public class AnyParser extends SimpleParser { try { tokens.skipMultiple(PLUS); + if ( ! tokens.skipMultiple(MINUS)) return null; + if (tokens.currentIsNoIgnore(SPACE)) return null; - if (!tokens.skipMultiple(MINUS)) { - return null; - } - - if (tokens.currentIsNoIgnore(SPACE)) { - return null; - } - - if (item == null) { - item = indexableItem(); - } + item = indexableItem(); if (item == null) { item = compositeItem(); @@ -88,13 +71,13 @@ public class AnyParser extends SimpleParser { } } } - if (item!=null) + if (item != null) item.setProtected(true); + return item; } finally { - if (item == null) { + if (item == null) tokens.setPosition(position); - } } } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/SimpleParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/SimpleParser.java index 9ddfea6dffb..0686a4bdb43 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/SimpleParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/SimpleParser.java @@ -50,32 +50,28 @@ abstract class SimpleParser extends StructuredParser { private Item anyItemsBody(boolean topLevel) { Item topLevelItem = null; NotItem not = null; - Item item; + Item item = null; do { - item = null; - - if (item == null) { - item = positiveItem(); - if (item != null) { - if (not == null) { - not = new NotItem(); - not.addPositiveItem(item); - topLevelItem = combineItems(topLevelItem, not); - } else { - not.addPositiveItem(item); - } + item = positiveItem(); + if (item != null) { + if (not == null) { + not = new NotItem(); + not.addPositiveItem(item); + topLevelItem = combineItems(topLevelItem, not); + } else { + not.addPositiveItem(item); } } if (item == null) { item = negativeItem(); if (item != null) { - if (not == null && item != null) { + if (not == null) { not = new NotItem(); not.addNegativeItem(item); topLevelItem = combineItems(topLevelItem, not); - } else if (item != null) { + } else { not.addNegativeItem(item); } } @@ -97,9 +93,8 @@ abstract class SimpleParser extends StructuredParser { if (item != null) { if (topLevelItem == null) { topLevelItem = item; - } else if (needNewTopLevel(topLevelItem, item)) { + } else if (needNewORTopLevel(topLevelItem, item)) { CompositeItem newTop = new OrItem(); - newTop.addItem(topLevelItem); newTop.addItem(item); topLevelItem = newTop; @@ -144,21 +139,13 @@ abstract class SimpleParser extends StructuredParser { } } - - /** Says whether we need a new top level item given the new item */ - private boolean needNewTopLevel(Item topLevelItem, Item item) { - if (item == null) { - return false; - } - if (topLevelItem instanceof TermItem) { - return true; - } - if (topLevelItem instanceof PhraseItem) { - return true; - } - if (topLevelItem instanceof BlockItem) { - return true; - } + /** Says whether we need a new top level OR item given the new item */ + private boolean needNewORTopLevel(Item topLevelItem, Item item) { + if (item == null) return false; + if (topLevelItem instanceof TermItem) return true; + if (topLevelItem instanceof PhraseItem) return true; + if (topLevelItem instanceof BlockItem) return true; + if ( topLevelItem instanceof AndItem) return true; return false; } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/SpecialTokens.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/SpecialTokens.java index c206ff7567e..8020088c2e3 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/SpecialTokens.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/SpecialTokens.java @@ -159,7 +159,7 @@ public class SpecialTokens { @Override public int hashCode() { return token.hashCode(); } - public Token toToken(int start,String rawSource) { + public Token toToken(int start, String rawSource) { return new Token(Token.Kind.WORD, replace(), true, new Substring(start, start + token.length(), rawSource)); // XXX: Unsafe? } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java index ee4c0d4d9f0..9ba6c1a8101 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java @@ -442,9 +442,9 @@ abstract class StructuredParser extends AbstractParser { Item item = null; try { - if (!tokens.currentIs(WORD) - && ((!tokens.currentIs(NUMBER) && !tokens.currentIs(MINUS) - && !tokens.currentIs(UNDERSCORE)) || (!submodes.url && !submodes.site))) { + if ( ! tokens.currentIs(WORD) + && ((!tokens.currentIs(NUMBER) && !tokens.currentIs(MINUS) + && !tokens.currentIs(UNDERSCORE)) || (!submodes.url && !submodes.site))) { return null; } Token word = tokens.next(); @@ -520,7 +520,7 @@ abstract class StructuredParser extends AbstractParser { /** Returns a word, a phrase, or another composite */ private Item phraseBody(String indexName) { boolean quoted = false; - PhraseItem phrase = null; + CompositeItem composite = null; Item firstWord = null; boolean starAfterFirst = false; boolean starBeforeFirst; @@ -539,7 +539,7 @@ abstract class StructuredParser extends AbstractParser { quoted = !quoted; } - Item word = phraseWord(indexName, (firstWord != null) || (phrase != null)); + Item word = phraseWord(indexName, (firstWord != null) || (composite != null)); if (word == null) { if (tokens.skipMultiple(QUOTE)) { @@ -555,34 +555,39 @@ abstract class StructuredParser extends AbstractParser { ((PhraseSegmentItem) word).setExplicit(true); } - if (phrase != null) { - phrase.addItem(word); + if (composite != null) { + composite.addItem(word); + connectLastTermsIn(composite); } else if (firstWord != null) { if (submodes.site || submodes.url) { UriItem uriItem = new UriItem(); if (submodes.site) uriItem.setEndAnchorDefault(true); - phrase = uriItem; + composite = uriItem; } else { - phrase = new PhraseItem(); + if (quoted || indexFacts.getIndex(indexName).getPhraseSegmenting()) + composite = new PhraseItem(); + else + composite = new AndItem(); } - if (quoted || submodes.site || submodes.url) { - phrase.setExplicit(true); + if ( (quoted || submodes.site || submodes.url) && composite instanceof PhraseItem) { + ((PhraseItem)composite).setExplicit(true); } if (addStartOfHostMarker) { - phrase.addItem(MarkerWordItem.createStartOfHost()); + composite.addItem(MarkerWordItem.createStartOfHost()); } if (firstWord instanceof IntItem) { IntItem asInt = (IntItem) firstWord; firstWord = new WordItem(asInt.stringValue(), asInt.getIndexName(), true, asInt.getOrigin()); } - phrase.addItem(firstWord); - phrase.addItem(word); + composite.addItem(firstWord); + composite.addItem(word); + connectLastTermsIn(composite); } else if (word instanceof PhraseItem) { - phrase = (PhraseItem) word; + composite = (PhraseItem)word; } else { firstWord = word; starAfterFirst = tokens.skipNoIgnore(STAR); @@ -609,29 +614,29 @@ abstract class StructuredParser extends AbstractParser { braceLevelURL = 0; - if (phrase != null) { + if (composite != null) { if (addEndMarking()) { - phrase.addItem(MarkerWordItem.createEndOfHost()); + composite.addItem(MarkerWordItem.createEndOfHost()); } - return phrase; + return composite; } else if (firstWord != null && submodes.site) { if (starAfterFirst && !addStartOfHostMarker) { return firstWord; } else { - phrase = new PhraseItem(); + composite = new PhraseItem(); + ((PhraseItem)composite).setExplicit(true); if (addStartOfHostMarker) { - phrase.addItem(MarkerWordItem.createStartOfHost()); + composite.addItem(MarkerWordItem.createStartOfHost()); } if (firstWord instanceof IntItem) { IntItem asInt = (IntItem) firstWord; firstWord = new WordItem(asInt.stringValue(), asInt.getIndexName(), true, asInt.getOrigin()); } - phrase.addItem(firstWord); + composite.addItem(firstWord); if (!starAfterFirst) { - phrase.addItem(MarkerWordItem.createEndOfHost()); + composite.addItem(MarkerWordItem.createEndOfHost()); } - phrase.setExplicit(true); - return phrase; + return composite; } } else { if (firstWord != null && firstWord instanceof TermItem && (starAfterFirst || starBeforeFirst)) { @@ -651,6 +656,15 @@ abstract class StructuredParser extends AbstractParser { } } + private void connectLastTermsIn(CompositeItem composite) { + int items = composite.items().size(); + if (items < 2) return; + Item nextToLast = composite.items().get(items - 2); + Item last = composite.items().get(items - 1); + if ( ! (nextToLast instanceof TermItem)) return; + ((TermItem)nextToLast).setConnectivity(last, 1); + } + private boolean addStartMarking() { if (submodes.explicitAnchoring() && tokens.currentIs(HAT)) { tokens.skip(); diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/Tokenizer.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/Tokenizer.java index 61f09e2f7b7..5e243e52057 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/Tokenizer.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/Tokenizer.java @@ -108,8 +108,7 @@ public final class Tokenizer { if (i >= source.length()) break; int c = source.codePointAt(i); - if (characterClasses.isLetterOrDigit(c) - || (c == '\'' && acceptApostropheAsWordCharacter(currentIndex))) { + if (characterClasses.isLetterOrDigit(c) || (c == '\'' && acceptApostropheAsWordCharacter(currentIndex))) { i = consumeWordOrNumber(i, currentIndex); } else if (Character.isWhitespace(c)) { addToken(SPACE, " ", i, i + 1); @@ -187,7 +186,6 @@ public final class Tokenizer { return true; } - @SuppressWarnings({"deprecation"}) private Index determineCurrentIndex(Index defaultIndex, IndexFacts.Session indexFacts) { int backtrack = tokens.size(); int tokencnt = 0; @@ -328,7 +326,6 @@ public final class Tokenizer { wantEndQuote = true; actualStart = curPos+1; } else if (wantEndQuote && looksLikeExactEnd(curPos+1)) { - // System.err.println("seen quoted token from "+actualStart+" to "+curPos); seenSome = true; wantEndQuote = false; isQuoted = true; @@ -435,7 +432,7 @@ public final class Tokenizer { if (suffStar) { addToken(STAR, "*", starPos, starPos + 1); } - tokens.add(new Token(WORD, source.substring(actualStart, end), true, new Substring(actualStart, end, source))); // XXX: Unsafe? + tokens.add(new Token(WORD, source.substring(actualStart, end), true, new Substring(actualStart, end, source))); // skip terminating quote if (isQuoted) { @@ -451,17 +448,17 @@ public final class Tokenizer { break; end++; } - tokens.add(new Token(WORD, source.substring(start, end), true, new Substring(start, end, source))); // XXX: Unsafe start? - if (end>=source.length()) + tokens.add(new Token(WORD, source.substring(start, end), true, new Substring(start, end, source))); + if (end >= source.length()) return end; else - return end+terminator.length(); // Don't create a token for the terminator + return end + terminator.length(); // Don't create a token for the terminator } private boolean terminatorStartsAt(int start,String terminator) { - int terminatorPosition=0; - while ((terminatorPosition+start)<source.length()) { - if (source.charAt(start+terminatorPosition)!=terminator.charAt(terminatorPosition)) + int terminatorPosition = 0; + while ((terminatorPosition + start) < source.length()) { + if (source.charAt(start+terminatorPosition) != terminator.charAt(terminatorPosition)) return false; terminatorPosition++; if (terminatorPosition >= terminator.length()) @@ -481,8 +478,8 @@ public final class Tokenizer { while (tokenEnd < source.length()) { if (substringSpecialTokens) { - substringSpecialToken=getSpecialToken(tokenEnd); - if (substringSpecialToken!=null) break; + substringSpecialToken = getSpecialToken(tokenEnd); + if (substringSpecialToken != null) break; } int c = source.codePointAt(tokenEnd); @@ -506,7 +503,7 @@ public final class Tokenizer { // underscoresOnly = false; quotesOnly = false; } else if (c == '\'') { - if (!acceptApostropheAsWordCharacter(currentIndex)) { + if ( ! acceptApostropheAsWordCharacter(currentIndex)) { break; } // Otherwise consume apostrophes... @@ -530,15 +527,15 @@ public final class Tokenizer { } } - if (substringSpecialToken==null) + if (substringSpecialToken == null) return --tokenEnd; // TODO: test the logic around tokenEnd with friends - addToken(substringSpecialToken.toToken(tokenEnd,source)); - return --tokenEnd+substringSpecialToken.token().length(); + addToken(substringSpecialToken.toToken(tokenEnd, source)); + return --tokenEnd + substringSpecialToken.token().length(); } private void addToken(Token.Kind kind, String word, int start, int end) { - addToken(new Token(kind, word, false, new Substring(start, end, source))); // XXX: Unsafe? + addToken(new Token(kind, word, false, new Substring(start, end, source))); } private void addToken(Token token) { diff --git a/container-search/src/main/java/com/yahoo/prelude/querytransform/NormalizingSearcher.java b/container-search/src/main/java/com/yahoo/prelude/querytransform/NormalizingSearcher.java index fdd6ad47a98..ce13045b518 100644 --- a/container-search/src/main/java/com/yahoo/prelude/querytransform/NormalizingSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/querytransform/NormalizingSearcher.java @@ -111,25 +111,17 @@ public class NormalizingSearcher extends Searcher { } private void normalizeAlternatives(Language language, Session indexFacts, WordAlternativesItem block) { - if (!block.isNormalizable()) { - return; - } - { - Index index = indexFacts.getIndex(block.getIndexName()); - if (index.isAttribute()) { - return; - } - if (!index.getNormalize()) { - return; - } - } + if ( ! block.isNormalizable()) return; + + Index index = indexFacts.getIndex(block.getIndexName()); + if (index.isAttribute()) return; + if ( ! index.getNormalize()) return; List<Alternative> terms = block.getAlternatives(); for (Alternative term : terms) { String accentDropped = linguistics.getTransformer().accentDrop(term.word, language); - if (!term.word.equals(accentDropped) && accentDropped.length() > 0) { + if ( ! term.word.equals(accentDropped) && accentDropped.length() > 0) block.addTerm(accentDropped, term.exactness * .7d); - } } } diff --git a/container-search/src/main/java/com/yahoo/prelude/querytransform/QueryRewrite.java b/container-search/src/main/java/com/yahoo/prelude/querytransform/QueryRewrite.java index 84c793a6df1..5a936d42ccc 100644 --- a/container-search/src/main/java/com/yahoo/prelude/querytransform/QueryRewrite.java +++ b/container-search/src/main/java/com/yahoo/prelude/querytransform/QueryRewrite.java @@ -4,6 +4,8 @@ package com.yahoo.prelude.querytransform; import com.yahoo.prelude.query.AndItem; import com.yahoo.prelude.query.CompositeItem; import com.yahoo.prelude.query.EquivItem; +import com.yahoo.prelude.query.HasIndexItem; +import com.yahoo.prelude.query.IndexedItem; import com.yahoo.prelude.query.Item; import com.yahoo.prelude.query.NearItem; import com.yahoo.prelude.query.NotItem; @@ -169,7 +171,9 @@ public class QueryRewrite { removeOtherNonrankedChildren(item, i); recall = Recall.RECALLS_EVERYTHING; } else if ((item instanceof AndItem) || (item instanceof NearItem)) { - item.removeItem(i); + if ( ! isRanked(item.getItem(i))) { + item.removeItem(i); + } } else if (item instanceof RankItem) { // empty } else { @@ -200,6 +204,20 @@ public class QueryRewrite { parent.removeItem(i); } } + + private static boolean isRanked(Item item) { + if (item instanceof CompositeItem) { + for (Item child : ((CompositeItem)item).items()) + if (isRanked(child)) return true; + return false; + } + else if (item instanceof HasIndexItem && Hit.SDDOCNAME_FIELD.equals(((HasIndexItem)item).getIndexName())) { + return false; // No point in ranking by sddocname + } + else { + return item.isRanked(); + } + } private static Item collapseSingleComposites(Item item) { if (!(item instanceof CompositeItem)) { diff --git a/container-search/src/main/java/com/yahoo/prelude/querytransform/StemmingSearcher.java b/container-search/src/main/java/com/yahoo/prelude/querytransform/StemmingSearcher.java index 655fbf6acc3..9a9044def2d 100644 --- a/container-search/src/main/java/com/yahoo/prelude/querytransform/StemmingSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/querytransform/StemmingSearcher.java @@ -188,13 +188,10 @@ public class StemmingSearcher extends Searcher { return (Item) w; } - if (context.isCJK) { - composite = chooseCompositeForCJK(current, - ((Item) current).getParent(), - indexName); - } else { - composite = phraseSegment(current, indexName); - } + if (context.isCJK) + composite = chooseCompositeForCJK(current, ((Item) current).getParent(), indexName); + else + composite = chooseComposite(current, ((Item) current).getParent(), indexName); for (StemList segment : segments) { TaggableItem w = singleWordSegment(current, segment, index, substring, context.insidePhrase); @@ -331,39 +328,34 @@ public class StemmingSearcher extends Searcher { } } + private CompositeItem chooseComposite(BlockItem current, CompositeItem parent, String indexName) { + if (parent instanceof PhraseItem || current instanceof PhraseSegmentItem) + return createPhraseSegment(current, indexName); + else + return createAndSegment(current); + + } + private CompositeItem chooseCompositeForCJK(BlockItem current, CompositeItem parent, String indexName) { - CompositeItem composite; - if (current.getSegmentingRule() == SegmentingRule.LANGUAGE_DEFAULT) { - if (parent instanceof PhraseItem || current instanceof PhraseSegmentItem) { - composite = phraseSegment(current, indexName); - } else - composite = createAndSegment(current); - } else { - switch (current.getSegmentingRule()) { - case PHRASE: - composite = phraseSegment(current, indexName); - break; - case BOOLEAN_AND: - composite = createAndSegment(current); - break; + if (current.getSegmentingRule() == SegmentingRule.LANGUAGE_DEFAULT) + return chooseComposite(current, parent, indexName); + + switch (current.getSegmentingRule()) { // TODO: Why for CJK only? The segmentingRule says nothing about being for CJK only + case PHRASE: return createPhraseSegment(current, indexName); + case BOOLEAN_AND: return createAndSegment(current); default: - throw new IllegalArgumentException( - "Unknown segmenting rule: " - + current.getSegmentingRule() - + ". This is a bug in Vespa, as the implementation has gotten out of sync." - + " Please create a ticket as soon as possible."); - } + throw new IllegalArgumentException("Unknown segmenting rule: " + current.getSegmentingRule() + + ". This is a bug in Vespa, as the implementation has gotten out of sync." + + " Please create a ticket as soon as possible."); } - return composite; } private AndSegmentItem createAndSegment(BlockItem current) { return new AndSegmentItem(current.stringValue(), true, true); } - private CompositeItem phraseSegment(BlockItem current, String indexName) { - CompositeItem composite; - composite = new PhraseSegmentItem(current.getRawWord(), current.stringValue(), true, true); + private CompositeItem createPhraseSegment(BlockItem current, String indexName) { + CompositeItem composite = new PhraseSegmentItem(current.getRawWord(), current.stringValue(), true, true); composite.setIndexName(indexName); return composite; } diff --git a/container-search/src/main/java/com/yahoo/prelude/searcher/PosSearcher.java b/container-search/src/main/java/com/yahoo/prelude/searcher/PosSearcher.java index 43717ecf6cd..37561d3a0f5 100644 --- a/container-search/src/main/java/com/yahoo/prelude/searcher/PosSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/searcher/PosSearcher.java @@ -147,7 +147,9 @@ public class PosSearcher extends Searcher { String radius = query.properties().getString(posRadius); int radiusUnits; if (radius == null) { - radiusUnits = 5000; + double radiuskm = 50.0; + double radiusdegrees = radiuskm * km2deg; + radiusUnits = (int)(radiusdegrees * 1000000); } else if (radius.endsWith("km")) { double radiuskm = Double.valueOf(radius.substring(0, radius.length()-2)); double radiusdegrees = radiuskm * km2deg; diff --git a/container-search/src/main/java/com/yahoo/prelude/searcher/ValidatePredicateSearcher.java b/container-search/src/main/java/com/yahoo/prelude/searcher/ValidatePredicateSearcher.java index 9b6f5926b61..a8b3c76fe00 100644 --- a/container-search/src/main/java/com/yahoo/prelude/searcher/ValidatePredicateSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/searcher/ValidatePredicateSearcher.java @@ -1,12 +1,12 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.prelude.searcher; -import java.util.Optional; import com.yahoo.component.chain.dependencies.After; import com.yahoo.prelude.Index; import com.yahoo.prelude.IndexFacts; import com.yahoo.prelude.query.Item; import com.yahoo.prelude.query.PredicateQueryItem; +import com.yahoo.prelude.query.SimpleIndexedItem; import com.yahoo.prelude.query.ToolBox; import com.yahoo.search.Query; import com.yahoo.search.Result; @@ -15,7 +15,8 @@ import com.yahoo.search.querytransform.BooleanSearcher; import com.yahoo.search.result.ErrorMessage; import com.yahoo.search.searchchain.Execution; -import java.util.Collection; +import java.util.ArrayList; +import java.util.List; /** * Checks that predicate queries don't use values outside the defined upper/lower bounds. @@ -27,26 +28,26 @@ public class ValidatePredicateSearcher extends Searcher { @Override public Result search(Query query, Execution execution) { - Optional<ErrorMessage> e = validate(query, execution.context().getIndexFacts().newSession(query)); - if (e.isPresent()) { + List<ErrorMessage> errorMessages = validate(query, execution.context().getIndexFacts().newSession(query)); + if (!errorMessages.isEmpty()) { Result r = new Result(query); - r.hits().addError(e.get()); + errorMessages.forEach(msg -> r.hits().addError(msg)); return r; } return execution.search(query); } - private Optional<ErrorMessage> validate(Query query, IndexFacts.Session indexFacts) { + private List<ErrorMessage> validate(Query query, IndexFacts.Session indexFacts) { ValidatePredicateVisitor visitor = new ValidatePredicateVisitor(indexFacts); ToolBox.visit(visitor, query.getModel().getQueryTree().getRoot()); - return visitor.errorMessage; + return visitor.errorMessages; } private static class ValidatePredicateVisitor extends ToolBox.QueryVisitor { private final IndexFacts.Session indexFacts; - public Optional<ErrorMessage> errorMessage = Optional.empty(); + final List<ErrorMessage> errorMessages = new ArrayList<>(); public ValidatePredicateVisitor(IndexFacts.Session indexFacts) { this.indexFacts = indexFacts; @@ -57,22 +58,37 @@ public class ValidatePredicateSearcher extends Searcher { if (item instanceof PredicateQueryItem) { visit((PredicateQueryItem) item); } + if (item instanceof SimpleIndexedItem) { + visit((SimpleIndexedItem) item); + } return true; } private void visit(PredicateQueryItem item) { - Index index = getIndexFromUnionOfDocumentTypes(item); + Index index = getIndexFromUnionOfDocumentTypes(item.getIndexName()); + if (!index.isPredicate()) { + errorMessages.add(ErrorMessage.createIllegalQuery(String.format("Index '%s' is not a predicate attribute.", index.getName()))); + } for (PredicateQueryItem.RangeEntry entry : item.getRangeFeatures()) { long value = entry.getValue(); if (value < index.getPredicateLowerBound() || value > index.getPredicateUpperBound()) { - errorMessage = Optional.of(ErrorMessage.createIllegalQuery( - String.format("%s=%d outside configured predicate bounds.", entry.getKey(), value))); + errorMessages.add( + ErrorMessage.createIllegalQuery(String.format("%s=%d outside configured predicate bounds.", entry.getKey(), value))); } } } - private Index getIndexFromUnionOfDocumentTypes(PredicateQueryItem item) { - return indexFacts.getIndex(item.getIndexName()); + private void visit(SimpleIndexedItem item) { + String indexName = item.getIndexName(); + Index index = getIndexFromUnionOfDocumentTypes(indexName); + if (index.isPredicate()) { + errorMessages.add( + ErrorMessage.createIllegalQuery(String.format("Index '%s' is predicate attribute and can only be used in conjunction with a predicate query operator.", indexName))); + } + } + + private Index getIndexFromUnionOfDocumentTypes(String indexName) { + return indexFacts.getIndex(indexName); } @Override diff --git a/container-search/src/main/java/com/yahoo/prelude/searcher/ValidateSortingSearcher.java b/container-search/src/main/java/com/yahoo/prelude/searcher/ValidateSortingSearcher.java index bbdb3b796a2..82148cf54e6 100644 --- a/container-search/src/main/java/com/yahoo/prelude/searcher/ValidateSortingSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/searcher/ValidateSortingSearcher.java @@ -119,6 +119,9 @@ public class ValidateSortingSearcher extends Searcher { String name = f.getFieldName(); if ("[rank]".equals(name) || "[docid]".equals(name)) { // built-in constants - ok + } else if ("[relevance]".equals(name)) { + // built-in constant '[relevance]' must map to '[rank]' + f.getSorter().setName("[rank]"); } else if ("[relevancy]".equals(name)) { // built-in constant '[relevancy]' must map to '[rank]' f.getSorter().setName("[rank]"); diff --git a/container-search/src/main/java/com/yahoo/search/Query.java b/container-search/src/main/java/com/yahoo/search/Query.java index 395d8853603..dc8e2b70740 100644 --- a/container-search/src/main/java/com/yahoo/search/Query.java +++ b/container-search/src/main/java/com/yahoo/search/Query.java @@ -288,7 +288,6 @@ public class Query extends com.yahoo.processing.Request implements Cloneable { this(""); } - /** * Construct a query from a string formatted in the http style, e.g <code>?query=test&offset=10&hits=13</code> * The query must be uri encoded. @@ -297,7 +296,6 @@ public class Query extends com.yahoo.processing.Request implements Cloneable { this(query, null); } - /** * Creates a query from a request * @@ -665,7 +663,7 @@ public class Query extends com.yahoo.processing.Request implements Cloneable { // getQueryTree isn't exception safe try { queryTree = model.getQueryTree().toString(); - } catch (Exception e) { + } catch (Exception | StackOverflowError e) { queryTree = "[Could not parse user input: " + model.getQueryString() + "]"; } return "query '" + queryTree + "'"; @@ -677,7 +675,7 @@ public class Query extends com.yahoo.processing.Request implements Cloneable { // getQueryTree isn't exception safe try { queryTree = model.getQueryTree().toString(); - } catch (Exception e) { + } catch (Exception | StackOverflowError e) { queryTree = "Could not parse user input: " + model.getQueryString(); } return "query=[" + queryTree + "]" + " offset=" + getOffset() + " hits=" + getHits() + "]"; diff --git a/container-search/src/main/java/com/yahoo/search/Result.java b/container-search/src/main/java/com/yahoo/search/Result.java index 4080b09f40b..ab48d5797b2 100644 --- a/container-search/src/main/java/com/yahoo/search/Result.java +++ b/container-search/src/main/java/com/yahoo/search/Result.java @@ -89,7 +89,6 @@ public final class Result extends com.yahoo.processing.Response implements Clone * with a result. It should <b>always</b> be called when adding * hits from a result, but there is no constraints on the order of the calls. */ - @SuppressWarnings("deprecation") public void mergeWith(Result result) { totalHitCount += result.getTotalHitCount(); deepHitCount += result.getDeepHitCount(); diff --git a/container-search/src/main/java/com/yahoo/search/cluster/BaseNodeMonitor.java b/container-search/src/main/java/com/yahoo/search/cluster/BaseNodeMonitor.java index dd01d895963..0d491d2f0c1 100644 --- a/container-search/src/main/java/com/yahoo/search/cluster/BaseNodeMonitor.java +++ b/container-search/src/main/java/com/yahoo/search/cluster/BaseNodeMonitor.java @@ -79,6 +79,8 @@ public abstract class BaseNodeMonitor<T> { */ public abstract void responded(); + /** @deprecated Not used */ + @Deprecated // TODO: Remove on Vespa 8 public boolean isIdle() { return (now()-respondedAt) >= configuration.getIdleLimit(); } diff --git a/container-search/src/main/java/com/yahoo/search/cluster/ClusterMonitor.java b/container-search/src/main/java/com/yahoo/search/cluster/ClusterMonitor.java index d4b6279be89..15cf4995b77 100644 --- a/container-search/src/main/java/com/yahoo/search/cluster/ClusterMonitor.java +++ b/container-search/src/main/java/com/yahoo/search/cluster/ClusterMonitor.java @@ -38,6 +38,9 @@ public class ClusterMonitor<T> { /** A map from Node to corresponding MonitoredNode */ private final Map<T, TrafficNodeMonitor<T>> nodeMonitors = Collections.synchronizedMap(new java.util.LinkedHashMap<>()); + /** @deprecated It is not advised to start the monitoring thread in the constructor. + * Use ClusterMonitor(NodeManager manager, false) and explicit start(). */ + @Deprecated public ClusterMonitor(NodeManager<T> manager) { this(manager, true); } @@ -50,6 +53,12 @@ public class ClusterMonitor<T> { } } + public void start() { + if ( ! monitorThread.isAlive()) { + monitorThread.start(); + } + } + /** Returns the configuration of this cluster monitor */ public MonitorConfiguration getConfiguration() { return configuration; } @@ -92,7 +101,7 @@ public class ClusterMonitor<T> { Boolean wasWorking = monitor.isKnownWorking(); monitor.responded(); if (wasWorking != monitor.isKnownWorking()) - nodeManager.working(monitor.getNode()); + nodeManager.working(node); } /** @@ -101,7 +110,7 @@ public class ClusterMonitor<T> { public void ping(Executor executor) { for (Iterator<BaseNodeMonitor<T>> i = nodeMonitorIterator(); i.hasNext() && !closed.get(); ) { BaseNodeMonitor<T> monitor= i.next(); - nodeManager.ping(monitor.getNode(), executor); // Cause call to failed or responded + nodeManager.ping(this, monitor.getNode(), executor); // Cause call to failed or responded } if (closed.get()) return; // Do nothing to change state if close has started. nodeManager.pingIterationCompleted(); diff --git a/container-search/src/main/java/com/yahoo/search/cluster/ClusterSearcher.java b/container-search/src/main/java/com/yahoo/search/cluster/ClusterSearcher.java index 20f56c86f7b..645c6446ef1 100644 --- a/container-search/src/main/java/com/yahoo/search/cluster/ClusterSearcher.java +++ b/container-search/src/main/java/com/yahoo/search/cluster/ClusterSearcher.java @@ -58,7 +58,7 @@ public abstract class ClusterSearcher<T> extends PingableSearcher implements Nod this(id, connections, hasher, internal, true); } - public ClusterSearcher(ComponentId id, List<T> connections, Hasher<T> hasher, boolean internal, boolean startPingThread) { + protected ClusterSearcher(ComponentId id, List<T> connections, Hasher<T> hasher, boolean internal, boolean startPingThread) { super(id); this.hasher = hasher; this.monitor = new ClusterMonitor<>(this, startPingThread); @@ -70,7 +70,7 @@ public abstract class ClusterSearcher<T> extends PingableSearcher implements Nod /** Pinging a node, called from ClusterMonitor */ @Override - public final void ping(T p, Executor executor) { + public final void ping(ClusterMonitor<T> clusterMonitor, T p, Executor executor) { log(LogLevel.FINE, "Sending ping to: ", p); Pinger pinger = new Pinger(p); FutureTask<Pong> future = new FutureTask<>(pinger); @@ -80,7 +80,7 @@ public abstract class ClusterSearcher<T> extends PingableSearcher implements Nod Throwable logThrowable = null; try { - pong = future.get(monitor.getConfiguration().getFailLimit(), TimeUnit.MILLISECONDS); + pong = future.get(clusterMonitor.getConfiguration().getFailLimit(), TimeUnit.MILLISECONDS); } catch (InterruptedException e) { pong = new Pong(ErrorMessage.createUnspecifiedError("Ping was interrupted: " + p)); logThrowable = e; @@ -96,10 +96,10 @@ public abstract class ClusterSearcher<T> extends PingableSearcher implements Nod future.cancel(true); if (pong.badResponse()) { - monitor.failed(p, pong.error().get()); + clusterMonitor.failed(p, pong.error().get()); log(LogLevel.FINE, "Failed ping - ", pong); } else { - monitor.responded(p); + clusterMonitor.responded(p); log(LogLevel.FINE, "Answered ping - ", p); } @@ -220,14 +220,6 @@ public abstract class ClusterSearcher<T> extends PingableSearcher implements Nod if (result == null) result = new Result(query, ErrorMessage.createBackendCommunicationError("No result returned in " + this + " from " + connection + " for " + query)); - - if (result.hits().getError() != null) { - log(LogLevel.FINE, "FAILED: ", query); - } else if ( ! result.isCached()) { - log(LogLevel.FINE, "WORKING: ", query); - } else { - log(LogLevel.FINE, "CACHE HIT: ", query); - } return result; } @@ -263,13 +255,6 @@ public abstract class ClusterSearcher<T> extends PingableSearcher implements Nod result.hits().addError(ErrorMessage.createBackendCommunicationError("Error filling " + result + " from " + connection + ": " + Exceptions.toMessageString(e))); } - if (result.hits().getError() != null) { - log(LogLevel.FINE, "FAILED: ", result.getQuery()); - } else if ( ! result.isCached()) { - log(LogLevel.FINE, "WORKING: ", result.getQuery()); - } else { - log(LogLevel.FINE, "CACHE HIT: " + result.getQuery()); - } } /** diff --git a/container-search/src/main/java/com/yahoo/search/cluster/Hasher.java b/container-search/src/main/java/com/yahoo/search/cluster/Hasher.java index 46752b0bedb..6c83e1c64e3 100644 --- a/container-search/src/main/java/com/yahoo/search/cluster/Hasher.java +++ b/container-search/src/main/java/com/yahoo/search/cluster/Hasher.java @@ -11,6 +11,7 @@ package com.yahoo.search.cluster; public class Hasher<T> { public static class NodeFactor<T> { + private final T node; /** diff --git a/container-search/src/main/java/com/yahoo/search/cluster/MonitorConfiguration.java b/container-search/src/main/java/com/yahoo/search/cluster/MonitorConfiguration.java index 226e0180d2e..a2fb982e3c5 100644 --- a/container-search/src/main/java/com/yahoo/search/cluster/MonitorConfiguration.java +++ b/container-search/src/main/java/com/yahoo/search/cluster/MonitorConfiguration.java @@ -8,85 +8,45 @@ package com.yahoo.search.cluster; */ public class MonitorConfiguration { - /** - * The interval in ms between consecutive checks of the monitored - * nodes - */ + /** The interval in ms between consecutive checks of the monitored nodes */ private long checkInterval=1000; - /** - * The number of times a failed node must respond before getting - * traffic again - */ - private int responseAfterFailLimit=3; + /** The number of milliseconds to attempt to complete a request before giving up */ + private final long requestTimeout = 980; - /** - * The number of ms a node is allowed to stay idle before it is - * pinged - */ - private long idleLimit=3000; + /** The number of milliseconds a node is allowed to fail before we mark it as not working */ + private long failLimit = 5000; - /** - * The number of milliseconds to attempt to complete a request - * before giving up - */ - private long requestTimeout = 5000; + /** Sets the interval between each ping of idle or failing nodes. Default is 1000 ms. */ + public void setCheckInterval(long intervalMs) { this.checkInterval = intervalMs; } - /** - * The number of milliseconds a node is allowed to fail before we - * mark it as not working - */ - private long failLimit=5000; + /** Returns the interval between each ping of idle or failing nodes. Default is 1000 ms. */ + public long getCheckInterval() { return checkInterval; } /** - * The number of times a node is allowed to fail in one hour - * before it is quarantined for an hour + * Sets the number of times a failed node must respond before it is put back in service. Default is 3. + * + * @deprecated will go away in Vespa 8 */ - private int failQuarantineLimit=3; + @Deprecated // TODO: Remove on Vespa 8 + public void setResponseAfterFailLimit(int responseAfterFailLimit) { } /** - * The number of ms to quarantine an unstable node + * Sets the number of ms a node (failing or working) is allowed to stay idle before it is pinged. Default is 3000. + * + * @deprecated Will go away in Vespa 8 */ - private long quarantineTime=1000*60*60; - - /** - * Sets the interval between each ping of idle or failing nodes - * Default is 1000ms - */ - public void setCheckInterval(long intervalMs) { - this.checkInterval=intervalMs; - } + @Deprecated // TODO: Remove on Vespa 8 + public void setIdleLimit(int idleLimit) { } /** - * Returns the interval between each ping of idle or failing nodes - * Default is 1000ms - */ - public long getCheckInterval() { - return checkInterval; - } - - /** - * Sets the number of times a failed node must respond before it is put - * back in service. Default is 3. - */ - public void setResponseAfterFailLimit(int responseAfterFailLimit) { - this.responseAfterFailLimit=responseAfterFailLimit; - } - - /** - * Sets the number of ms a node (failing or working) is allowed to - * stay idle before it is pinged. Default is 3000 - */ - public void setIdleLimit(int idleLimit) { - this.idleLimit=idleLimit; - } - - /** - * Gets the number of ms a node (failing or working) - * is allowed to stay idle before it is pinged. Default is 3000 + * Gets the number of ms a node (failing or working) is allowed to stay idle before it is pinged. Default is 3000. + * + * @deprecated Will go away in Vespa 8 */ + @Deprecated // TODO: Remove on Vespa 8 public long getIdleLimit() { - return idleLimit; + return 3000; } /** @@ -112,29 +72,26 @@ public class MonitorConfiguration { * in quarantine. Once in quarantine it won't be put back in * productuion before quarantineTime has expired even if it is * working. Default is 3 + * + * @deprecated Will go away in Vespa 8 */ - public void setFailQuarantineLimit(int failQuarantineLimit) { - this.failQuarantineLimit=failQuarantineLimit; - } + @Deprecated // TODO: Remove on Vespa 8 + public void setFailQuarantineLimit(int failQuarantineLimit) { } /** - * The number of ms an unstable node is quarantined. Default is - * 100*60*60 + * The number of ms an unstable node is quarantined. Default is 100*60*60 + * + * @deprecated Will go away in Vespa 8 */ - public void setQuarantineTime(long quarantineTime) { - this.quarantineTime=quarantineTime; - } + @Deprecated // TODO: Remove on Vespa 8 + public void setQuarantineTime(long quarantineTime) { } public String toString() { return "monitor configuration [" + - "checkInterval: " + checkInterval + - " responseAfterFailLimit: " + responseAfterFailLimit + - " idleLimit: " + idleLimit + - " requestTimeout " + requestTimeout + - " feilLimit " + failLimit + - " failQuerantineLimit " + failQuarantineLimit + - " quarantineTime " + quarantineTime + - "]"; + "checkInterval: " + checkInterval + + " requestTimeout " + requestTimeout + + " failLimit " + failLimit + + "]"; } } diff --git a/container-search/src/main/java/com/yahoo/search/cluster/NodeManager.java b/container-search/src/main/java/com/yahoo/search/cluster/NodeManager.java index 9b20139e3c5..836c71089c1 100644 --- a/container-search/src/main/java/com/yahoo/search/cluster/NodeManager.java +++ b/container-search/src/main/java/com/yahoo/search/cluster/NodeManager.java @@ -7,7 +7,7 @@ import java.util.concurrent.Executor; * Must be implemented by a node collection which wants * it's node state monitored by a ClusterMonitor * - * @author bratseth + * @author bratseth */ public interface NodeManager<T> { @@ -19,9 +19,22 @@ public interface NodeManager<T> { /** * Called when a node should be pinged. - * This *must* lead to either a call to NodeMonitor.failed or NodeMonitor.responded + * This *must* lead to either a call to NodeMonitor.failed or NodeMonitor.responded + * + * @deprecated Use ping(ClusterMonitor clusterMonitor, T node, Executor executor) instead. */ - void ping(T node, Executor executor); + @Deprecated // TODO: Remove on Vespa 8 + default void ping(T node, Executor executor) { + throw new IllegalStateException("If you have not overrriden ping(ClusterMonitor<T> clusterMonitor, T node, Executor executor), you should at least have overriden this method."); + } + + /** + * Called when a node should be pinged. + * This *must* lead to either a call to ClusterMonitor.failed or ClusterMonitor.responded + */ + default void ping(ClusterMonitor<T> clusterMonitor, T node, Executor executor) { + ping(node, executor); + } /** Called right after a ping has been issued to each node. This default implementation does nothing. */ default void pingIterationCompleted() {} diff --git a/container-search/src/main/java/com/yahoo/search/cluster/TrafficNodeMonitor.java b/container-search/src/main/java/com/yahoo/search/cluster/TrafficNodeMonitor.java index ccf3e863ff3..d17f6bfbaa8 100644 --- a/container-search/src/main/java/com/yahoo/search/cluster/TrafficNodeMonitor.java +++ b/container-search/src/main/java/com/yahoo/search/cluster/TrafficNodeMonitor.java @@ -52,8 +52,8 @@ public class TrafficNodeMonitor<T> extends BaseNodeMonitor<T> { * Called when a response is received from this node. */ public void responded() { - respondedAt=now(); - succeededAt=respondedAt; + respondedAt = now(); + succeededAt = respondedAt; setWorking(true,"Responds correctly"); } @@ -69,20 +69,20 @@ public class TrafficNodeMonitor<T> extends BaseNodeMonitor<T> { atStartUp = false; if (this.isWorking == working) return; // Old news - if (explanation==null) { - explanation=""; + if (explanation == null) { + explanation = ""; } else { - explanation=": " + explanation; + explanation = ": " + explanation; } if (working) { log.info("Putting " + node + " in service" + explanation); } else { log.warning("Taking " + node + " out of service" + explanation); - failedAt=now(); + failedAt = now(); } - this.isWorking=working; + this.isWorking = working; } } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java b/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java index 03b51fbaf70..626cf087aca 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java @@ -5,18 +5,20 @@ import com.google.inject.Inject; import com.yahoo.cloud.config.ClusterInfoConfig; import com.yahoo.component.AbstractComponent; import com.yahoo.component.ComponentId; +import com.yahoo.compress.Compressor; import com.yahoo.container.handler.VipStatus; import com.yahoo.jdisc.Metric; import com.yahoo.prelude.fastsearch.VespaBackEndSearcher; import com.yahoo.processing.request.CompoundName; import com.yahoo.search.Query; import com.yahoo.search.Result; +import com.yahoo.search.cluster.ClusterMonitor; import com.yahoo.search.dispatch.SearchPath.InvalidSearchPathException; import com.yahoo.search.dispatch.rpc.RpcInvokerFactory; +import com.yahoo.search.dispatch.rpc.RpcPingFactory; import com.yahoo.search.dispatch.rpc.RpcResourcePool; import com.yahoo.search.dispatch.searchcluster.Group; import com.yahoo.search.dispatch.searchcluster.Node; -import com.yahoo.search.dispatch.searchcluster.PingFactory; import com.yahoo.search.dispatch.searchcluster.SearchCluster; import com.yahoo.search.query.profile.types.FieldDescription; import com.yahoo.search.query.profile.types.FieldType; @@ -30,6 +32,7 @@ import java.util.List; import java.util.Optional; import java.util.OptionalInt; import java.util.Set; +import java.util.stream.Collectors; /** * A dispatcher communicates with search nodes to perform queries and fill hits. @@ -48,6 +51,7 @@ public class Dispatcher extends AbstractComponent { public static final String DISPATCH = "dispatch"; private static final String INTERNAL = "internal"; private static final String PROTOBUF = "protobuf"; + private static final String TOP_K_PROBABILITY = "topKProbability"; private static final String INTERNAL_METRIC = "dispatch_internal"; @@ -56,8 +60,12 @@ public class Dispatcher extends AbstractComponent { /** If enabled, search queries will use protobuf rpc */ public static final CompoundName dispatchProtobuf = CompoundName.fromComponents(DISPATCH, PROTOBUF); + /** If set will control computation of how many hits will be fetched from each partition.*/ + public static final CompoundName topKProbability = CompoundName.fromComponents(DISPATCH, TOP_K_PROBABILITY); + /** A model of the search cluster this dispatches to */ private final SearchCluster searchCluster; + private final ClusterMonitor clusterMonitor; private final LoadBalancer loadBalancer; @@ -76,55 +84,77 @@ public class Dispatcher extends AbstractComponent { argumentType.setBuiltin(true); argumentType.addField(new FieldDescription(INTERNAL, FieldType.booleanType)); argumentType.addField(new FieldDescription(PROTOBUF, FieldType.booleanType)); + argumentType.addField(new FieldDescription(TOP_K_PROBABILITY, FieldType.doubleType)); argumentType.freeze(); } public static QueryProfileType getArgumentType() { return argumentType; } @Inject - public Dispatcher(ComponentId clusterId, + public Dispatcher(RpcResourcePool resourcePool, + ComponentId clusterId, DispatchConfig dispatchConfig, ClusterInfoConfig clusterInfoConfig, VipStatus vipStatus, Metric metric) { - this(new SearchCluster(clusterId.stringValue(), dispatchConfig, clusterInfoConfig.nodeCount(), vipStatus), - dispatchConfig, - metric); - } + this(resourcePool, new SearchCluster(clusterId.stringValue(), dispatchConfig,clusterInfoConfig.nodeCount(), + vipStatus, new RpcPingFactory(resourcePool)), + dispatchConfig, metric); - private Dispatcher(SearchCluster searchCluster, DispatchConfig dispatchConfig, Metric metric) { - this(searchCluster, - dispatchConfig, - new RpcInvokerFactory(new RpcResourcePool(dispatchConfig), searchCluster), - metric); } - /* Protected for simple mocking in tests. Beware that searchCluster is shutdown on in deconstruct() */ - protected Dispatcher(SearchCluster searchCluster, - DispatchConfig dispatchConfig, - RpcInvokerFactory rcpInvokerFactory, - Metric metric) { - this(searchCluster, dispatchConfig, rcpInvokerFactory, rcpInvokerFactory, metric); + private Dispatcher(RpcResourcePool resourcePool, SearchCluster searchCluster, DispatchConfig dispatchConfig, Metric metric) { + this(new ClusterMonitor<>(searchCluster, true), searchCluster, dispatchConfig, new RpcInvokerFactory(resourcePool, searchCluster), metric); } /* Protected for simple mocking in tests. Beware that searchCluster is shutdown on in deconstruct() */ - protected Dispatcher(SearchCluster searchCluster, + protected Dispatcher(ClusterMonitor clusterMonitor, + SearchCluster searchCluster, DispatchConfig dispatchConfig, InvokerFactory invokerFactory, - PingFactory pingFactory, Metric metric) { if (dispatchConfig.useMultilevelDispatch()) throw new IllegalArgumentException(searchCluster + " is configured with multilevel dispatch, but this is not supported"); this.searchCluster = searchCluster; + this.clusterMonitor = clusterMonitor; this.loadBalancer = new LoadBalancer(searchCluster, dispatchConfig.distributionPolicy() == DispatchConfig.DistributionPolicy.ROUNDROBIN); this.invokerFactory = invokerFactory; this.metric = metric; this.metricContext = metric.createContext(null); this.maxHitsPerNode = dispatchConfig.maxHitsPerNode(); + searchCluster.addMonitoring(clusterMonitor); + Thread warmup = new Thread(new Runnable() { + @Override + public void run() { + warmup(dispatchConfig.warmuptime()); + } + }); + warmup.start(); + try { + while ( ! searchCluster.hasInformationAboutAllNodes()) { + Thread.sleep(1); + } + warmup.join(); + } catch (InterruptedException e) {} + + /* + * No we have information from all nodes and a ping iteration has completed. + * Instead of waiting until next ping interval to update coverage and group state, + * we should compute the state ourselves, so that when the dispatcher is ready the state + * of its groups are also known. + */ + searchCluster.pingIterationCompleted(); + } - searchCluster.startClusterMonitoring(pingFactory); + /* + Will run important code in order to trigger JIT compilation and avoid cold start issues. + Currently warms up lz4 compression code. + */ + + private static long warmup(double seconds) { + return new Compressor().warmup(seconds); } /** Returns the search cluster this dispatches to */ @@ -134,8 +164,8 @@ public class Dispatcher extends AbstractComponent { @Override public void deconstruct() { - /* The seach cluster must be shutdown first as it uses the invokerfactory. */ - searchCluster.shutDown(); + /* The clustermonitor must be shutdown first as it uses the invokerfactory through the searchCluster. */ + clusterMonitor.shutdown(); invokerFactory.release(); } @@ -191,7 +221,7 @@ public class Dispatcher extends AbstractComponent { int covered = searchCluster.groupsWithSufficientCoverage(); int groups = searchCluster.orderedGroups().size(); int max = Integer.min(Integer.min(covered + 1, groups), MAX_GROUP_SELECTION_ATTEMPTS); - Set<Integer> rejected = null; + Set<Integer> rejected = rejectGroupBlockingFeed(searchCluster.orderedGroups()); for (int i = 0; i < max; i++) { Optional<Group> groupInCluster = loadBalancer.takeGroup(rejected); if (groupInCluster.isEmpty()) break; // No groups available @@ -220,4 +250,21 @@ public class Dispatcher extends AbstractComponent { throw new IllegalStateException("No suitable groups to dispatch query. Rejected: " + rejected); } + /** + * We want to avoid groups blocking feed because their data may be out of date. + * If there is a single group blocking feed, we want to reject it. + * If multiple groups are blocking feed we should use them anyway as we may not have remaining + * capacity otherwise. Same if there are no other groups. + * + * @return a modifiable set containing the single group to reject, or null otherwise + */ + private Set<Integer> rejectGroupBlockingFeed(List<Group> groups) { + if (groups.size() == 1) return null; + List<Group> groupsRejectingFeed = groups.stream().filter(Group::isBlockingWrites).collect(Collectors.toList()); + if (groupsRejectingFeed.size() != 1) return null; + Set<Integer> rejected = new HashSet<>(); + rejected.add(groupsRejectingFeed.get(0).id()); + return rejected; + } + } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java b/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java index cec3e94d551..e62848a7f9e 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java @@ -81,7 +81,12 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM int originalHits = query.getHits(); int originalOffset = query.getOffset(); - query.setHits(query.getHits() + query.getOffset()); + int neededHits = originalHits + originalOffset; + Double topkProbabilityOverrride = query.properties().getDouble(Dispatcher.topKProbability); + int q = (topkProbabilityOverrride != null) + ? searchCluster.estimateHitsToFetch(neededHits, invokers.size(), topkProbabilityOverrride) + : searchCluster.estimateHitsToFetch(neededHits, invokers.size()); + query.setHits(q); query.setOffset(0); for (SearchInvoker invoker : invokers) { @@ -321,4 +326,7 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM protected LinkedBlockingQueue<SearchInvoker> newQueue() { return new LinkedBlockingQueue<>(); } + + // For testing + Collection<SearchInvoker> invokers() { return invokers; } } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java b/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java index 1c3a90ac6ab..03160e6c9c7 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java @@ -43,7 +43,7 @@ public abstract class InvokerFactory { * @param nodes pre-selected list of content nodes * @param acceptIncompleteCoverage if some of the nodes are unavailable and this parameter is * false, verify that the remaining set of nodes has sufficient coverage - * @return Optional containing the SearchInvoker or empty if some node in the + * @return the invoker or empty if some node in the * list is invalid and the remaining coverage is not sufficient */ public Optional<SearchInvoker> createSearchInvoker(VespaBackEndSearcher searcher, @@ -82,7 +82,7 @@ public abstract class InvokerFactory { if ( ! searchCluster.isPartialGroupCoverageSufficient(groupId, success) && !acceptIncompleteCoverage) { return Optional.empty(); } - if(invokers.size() == 0) { + if (invokers.size() == 0) { return Optional.of(createCoverageErrorInvoker(nodes, failed)); } } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/LeanHit.java b/container-search/src/main/java/com/yahoo/search/dispatch/LeanHit.java index 86f1999d8b4..8a90557fa3b 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/LeanHit.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/LeanHit.java @@ -12,15 +12,11 @@ public class LeanHit implements Comparable<LeanHit> { private final int distributionKey; public LeanHit(byte [] gid, int partId, int distributionKey, double relevance) { - this.gid = gid; - this.relevance = Double.isNaN(relevance) ? Double.NEGATIVE_INFINITY : relevance; - this.sortData = null; - this.partId = partId; - this.distributionKey = distributionKey; + this(gid, partId, distributionKey, relevance, null); } - public LeanHit(byte [] gid, int partId, int distributionKey, byte [] sortData) { + public LeanHit(byte [] gid, int partId, int distributionKey, double relevance, byte [] sortData) { this.gid = gid; - this.relevance = 0.0; + this.relevance = Double.isNaN(relevance) ? Double.NEGATIVE_INFINITY : relevance; this.sortData = sortData; this.partId = partId; this.distributionKey = distributionKey; diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/LoadBalancer.java b/container-search/src/main/java/com/yahoo/search/dispatch/LoadBalancer.java index 210ab5777d2..05e1ea6e2f9 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/LoadBalancer.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/LoadBalancer.java @@ -134,6 +134,7 @@ public class LoadBalancer { } private static class RoundRobinScheduler implements GroupScheduler { + private int needle = 0; private final List<GroupStatus> scoreboard; @@ -204,6 +205,7 @@ public class LoadBalancer { } static class AdaptiveScheduler implements GroupScheduler { + private final Random random; private final List<GroupStatus> scoreboard; @@ -251,4 +253,5 @@ public class LoadBalancer { return selectGroup(needle, false, rejectedGroups); } } + } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/TopKEstimator.java b/container-search/src/main/java/com/yahoo/search/dispatch/TopKEstimator.java new file mode 100644 index 00000000000..8003d9c6744 --- /dev/null +++ b/container-search/src/main/java/com/yahoo/search/dispatch/TopKEstimator.java @@ -0,0 +1,42 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.search.dispatch; + +import org.apache.commons.math3.distribution.TDistribution; + +/** + * Use StudentT distribution and estimate how many hits you need from each partition + * to to get the globally top-k documents with the desired probability + * @author baldersheim + */ +public class TopKEstimator { + private final TDistribution studentT; + private final double defaultP; + private final boolean estimate; + + private static boolean needEstimate(double p) { + return (0.0 < p) && (p < 1.0); + } + public TopKEstimator(double freedom, double defaultProbability) { + this.studentT = new TDistribution(null, freedom); + defaultP = defaultProbability; + estimate = needEstimate(defaultP); + } + double estimateExactK(double k, double n, double p) { + double variance = k * 1/n * (1 - 1/n); + double p_inverse = 1 - (1 - p)/n; + return k/n + studentT.inverseCumulativeProbability(p_inverse) * Math.sqrt(variance); + } + double estimateExactK(double k, double n) { + return estimateExactK(k, n, defaultP); + } + public int estimateK(int k, int n) { + return (estimate && n > 1) + ? (int)Math.ceil(estimateExactK(k, n, defaultP)) + : k; + } + public int estimateK(int k, int n, double p) { + return (needEstimate(p) && (n > 1)) + ? (int)Math.ceil(estimateExactK(k, n, p)) + : k; + } +} diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/ProtobufSerialization.java b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/ProtobufSerialization.java index ae2258c4546..51290c245ac 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/ProtobufSerialization.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/ProtobufSerialization.java @@ -214,7 +214,7 @@ public class ProtobufSerialization { for (var replyHit : protobuf.getHitsList()) { LeanHit hit = (replyHit.getSortData().isEmpty()) ? new LeanHit(replyHit.getGlobalId().toByteArray(), partId, distKey, replyHit.getRelevance()) - : new LeanHit(replyHit.getGlobalId().toByteArray(), partId, distKey, replyHit.getSortData().toByteArray()); + : new LeanHit(replyHit.getGlobalId().toByteArray(), partId, distKey, replyHit.getRelevance(), replyHit.getSortData().toByteArray()); result.getLeanHits().add(hit); } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcClient.java b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcClient.java index a2821892358..52cb2b4c061 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcClient.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcClient.java @@ -44,13 +44,14 @@ class RpcClient implements Client { // The current shared connection. This will be recycled when it becomes invalid. // All access to this must be synchronized - private Target target = null; + private Target target; public RpcNodeConnection(String hostname, int port, Supervisor supervisor) { this.supervisor = supervisor; this.hostname = hostname; this.port = port; description = "rpc node connection to " + hostname + ":" + port; + target = supervisor.connect(new Spec(hostname, port)); } @Override @@ -79,17 +80,16 @@ class RpcClient implements Client { private void invokeAsync(Request req, double timeout, RequestWaiter waiter) { // TODO: Consider replacing this by a watcher on the target synchronized(this) { // ensure we have exactly 1 valid connection across threads - if (target == null || ! target.isValid()) + if (! target.isValid()) { target = supervisor.connect(new Spec(hostname, port)); + } } target.invokeAsync(req, timeout, waiter); } @Override public void close() { - if (target != null) { - target.close(); - } + target.close(); } @Override diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcInvokerFactory.java b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcInvokerFactory.java index 5c9928de924..74bc9e8bfbb 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcInvokerFactory.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcInvokerFactory.java @@ -1,28 +1,24 @@ // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.dispatch.rpc; -import com.yahoo.prelude.Pong; import com.yahoo.prelude.fastsearch.DocumentDatabase; import com.yahoo.prelude.fastsearch.VespaBackEndSearcher; import com.yahoo.processing.request.CompoundName; import com.yahoo.search.Query; import com.yahoo.search.Result; -import com.yahoo.search.cluster.ClusterMonitor; import com.yahoo.search.dispatch.Dispatcher; import com.yahoo.search.dispatch.FillInvoker; import com.yahoo.search.dispatch.InvokerFactory; import com.yahoo.search.dispatch.SearchInvoker; import com.yahoo.search.dispatch.searchcluster.Node; -import com.yahoo.search.dispatch.searchcluster.PingFactory; import com.yahoo.search.dispatch.searchcluster.SearchCluster; import java.util.Optional; -import java.util.concurrent.Callable; /** * @author ollivir */ -public class RpcInvokerFactory extends InvokerFactory implements PingFactory { +public class RpcInvokerFactory extends InvokerFactory { /** Unless turned off this will fill summaries by dispatching directly to search nodes over RPC when possible */ private final static CompoundName dispatchSummaries = new CompoundName("dispatch.summaries"); @@ -60,12 +56,4 @@ public class RpcInvokerFactory extends InvokerFactory implements PingFactory { return new RpcFillInvoker(rpcResourcePool, documentDb); } - public void release() { - rpcResourcePool.release(); - } - - @Override - public Callable<Pong> createPinger(Node node, ClusterMonitor<Node> monitor) { - return new RpcPing(node, monitor, rpcResourcePool); - } } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcPing.java b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcPing.java index e0f1dc5e675..5e04f1d7a3e 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcPing.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcPing.java @@ -10,55 +10,63 @@ import com.yahoo.search.cluster.ClusterMonitor; import com.yahoo.search.dispatch.rpc.Client.ProtobufResponse; import com.yahoo.search.dispatch.rpc.Client.ResponseOrError; import com.yahoo.search.dispatch.searchcluster.Node; +import com.yahoo.search.dispatch.searchcluster.Pinger; +import com.yahoo.search.dispatch.searchcluster.PongHandler; import com.yahoo.search.result.ErrorMessage; import com.yahoo.yolean.Exceptions; -import java.util.concurrent.Callable; -import java.util.concurrent.LinkedBlockingQueue; -import java.util.concurrent.TimeUnit; +import java.util.logging.Logger; -public class RpcPing implements Callable<Pong> { +public class RpcPing implements Pinger, Client.ResponseReceiver { + private static final Logger log = Logger.getLogger(RpcPing.class.getName()); private static final String RPC_METHOD = "vespa.searchprotocol.ping"; private static final CompressionType PING_COMPRESSION = CompressionType.NONE; private final Node node; private final RpcResourcePool resourcePool; private final ClusterMonitor<Node> clusterMonitor; + private final long pingSequenceId; + private final PongHandler pongHandler; - public RpcPing(Node node, ClusterMonitor<Node> clusterMonitor, RpcResourcePool rpcResourcePool) { + public RpcPing(Node node, ClusterMonitor<Node> clusterMonitor, RpcResourcePool rpcResourcePool, PongHandler pongHandler) { this.node = node; this.resourcePool = rpcResourcePool; this.clusterMonitor = clusterMonitor; + pingSequenceId = node.createPingSequenceId(); + this.pongHandler = pongHandler; } @Override - public Pong call() throws Exception { + public void ping() { try { - var queue = new LinkedBlockingQueue<ResponseOrError<ProtobufResponse>>(1); - - sendPing(queue); + sendPing(); + } catch (RuntimeException e) { + pongHandler.handle(new Pong(ErrorMessage.createBackendCommunicationError("Exception when pinging " + node + + ": " + Exceptions.toMessageString(e)))); + } + } - var responseOrError = queue.poll(clusterMonitor.getConfiguration().getRequestTimeout(), TimeUnit.MILLISECONDS); - if (responseOrError == null) { - return new Pong(ErrorMessage.createNoAnswerWhenPingingNode("Timed out waiting for pong from " + node)); - } else if (responseOrError.error().isPresent()) { - return new Pong(ErrorMessage.createBackendCommunicationError(responseOrError.error().get())); - } + private Pong toPong(ResponseOrError<ProtobufResponse> responseOrError) { + if (responseOrError == null) { + return new Pong(ErrorMessage.createNoAnswerWhenPingingNode("Timed out waiting for pong from " + node)); + } else if (responseOrError.error().isPresent()) { + return new Pong(ErrorMessage.createBackendCommunicationError(responseOrError.error().get())); + } + try { return decodeReply(responseOrError.response().get()); - } catch (RuntimeException e) { - return new Pong( - ErrorMessage.createBackendCommunicationError("Exception when pinging " + node + ": " + Exceptions.toMessageString(e))); + } catch (InvalidProtocolBufferException e) { + return new Pong(ErrorMessage.createBackendCommunicationError(e.getMessage())); } } - private void sendPing(LinkedBlockingQueue<ResponseOrError<ProtobufResponse>> queue) { + private void sendPing() { var connection = resourcePool.getConnection(node.key()); var ping = SearchProtocol.MonitorRequest.newBuilder().build().toByteArray(); double timeoutSeconds = ((double) clusterMonitor.getConfiguration().getRequestTimeout()) / 1000.0; Compressor.Compression compressionResult = resourcePool.compressor().compress(PING_COMPRESSION, ping); - connection.request(RPC_METHOD, compressionResult.type(), ping.length, compressionResult.data(), rsp -> queue.add(rsp), timeoutSeconds); + connection.request(RPC_METHOD, compressionResult.type(), ping.length, compressionResult.data(),this, timeoutSeconds); } private Pong decodeReply(ProtobufResponse response) throws InvalidProtocolBufferException { @@ -76,4 +84,14 @@ public class RpcPing implements Callable<Pong> { } } + @Override + public void receive(ResponseOrError<ProtobufResponse> response) { + if (node.isLastReceivedPong(pingSequenceId)) { + pongHandler.handle(toPong(response)); + } else { + //TODO Reduce to debug or remove once we have enumerated what happens here. + log.info("Pong " + pingSequenceId + " from node " + node.key() + " in group " + node.group() + + " with hostname " + node.hostname() + " received too late, latest is " + node.getLastReceivedPongId()); + } + } } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcPingFactory.java b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcPingFactory.java new file mode 100644 index 00000000000..ac8f0a59c20 --- /dev/null +++ b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcPingFactory.java @@ -0,0 +1,18 @@ +package com.yahoo.search.dispatch.rpc; + +import com.yahoo.search.cluster.ClusterMonitor; +import com.yahoo.search.dispatch.searchcluster.Node; +import com.yahoo.search.dispatch.searchcluster.PingFactory; +import com.yahoo.search.dispatch.searchcluster.Pinger; +import com.yahoo.search.dispatch.searchcluster.PongHandler; + +public class RpcPingFactory implements PingFactory { + private final RpcResourcePool rpcResourcePool; + public RpcPingFactory(RpcResourcePool rpcResourcePool) { + this.rpcResourcePool = rpcResourcePool; + } + @Override + public Pinger createPinger(Node node, ClusterMonitor<Node> monitor, PongHandler pongHandler) { + return new RpcPing(node, monitor, rpcResourcePool, pongHandler); + } +} diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcResourcePool.java b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcResourcePool.java index ca2a0c9bfb0..065489ef9a0 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcResourcePool.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcResourcePool.java @@ -2,6 +2,9 @@ package com.yahoo.search.dispatch.rpc; import com.google.common.collect.ImmutableMap; +import com.google.inject.Inject; +import com.yahoo.component.AbstractComponent; +import com.yahoo.component.ComponentId; import com.yahoo.compress.CompressionType; import com.yahoo.compress.Compressor; import com.yahoo.compress.Compressor.Compression; @@ -23,7 +26,7 @@ import java.util.Random; * * @author ollivir */ -public class RpcResourcePool { +public class RpcResourcePool extends AbstractComponent { /** The compression method which will be used with rpc dispatch. "lz4" (default) and "none" is supported. */ public final static CompoundName dispatchCompression = new CompoundName("dispatch.compression"); @@ -33,13 +36,15 @@ public class RpcResourcePool { /** Connections to the search nodes this talks to, indexed by node id ("partid") */ private final ImmutableMap<Integer, NodeConnectionPool> nodeConnectionPools; - public RpcResourcePool(Map<Integer, NodeConnection> nodeConnections) { + RpcResourcePool(Map<Integer, NodeConnection> nodeConnections) { var builder = new ImmutableMap.Builder<Integer, NodeConnectionPool>(); nodeConnections.forEach((key, connection) -> builder.put(key, new NodeConnectionPool(Collections.singletonList(connection)))); this.nodeConnectionPools = builder.build(); } + @Inject public RpcResourcePool(DispatchConfig dispatchConfig) { + super(); var client = new RpcClient(dispatchConfig.numJrtTransportThreads()); // Create rpc node connection pools indexed by the node distribution key @@ -73,7 +78,9 @@ public class RpcResourcePool { } } - public void release() { + @Override + public void deconstruct() { + super.deconstruct(); nodeConnectionPools.values().forEach(NodeConnectionPool::release); } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcSearchInvoker.java b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcSearchInvoker.java index 07d8439ff46..76240e55c98 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcSearchInvoker.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcSearchInvoker.java @@ -5,7 +5,6 @@ import com.yahoo.compress.CompressionType; import com.yahoo.compress.Compressor; import com.yahoo.prelude.fastsearch.VespaBackEndSearcher; import com.yahoo.search.Query; -import com.yahoo.search.Result; import com.yahoo.search.dispatch.InvokerResult; import com.yahoo.search.dispatch.SearchInvoker; import com.yahoo.search.dispatch.rpc.Client.ProtobufResponse; diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Group.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Group.java index 0e4e87b9a6a..ec616a18e09 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Group.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Group.java @@ -21,6 +21,7 @@ public class Group { private final AtomicBoolean hasSufficientCoverage = new AtomicBoolean(true); private final AtomicBoolean hasFullCoverage = new AtomicBoolean(true); private final AtomicLong activeDocuments = new AtomicLong(0); + private final AtomicBoolean isBlockingWrites = new AtomicBoolean(false); public Group(int id, List<Node> nodes) { this.id = id; @@ -61,21 +62,16 @@ public class Group { return nodesUp; } - void aggregateActiveDocuments() { - long activeDocumentsInGroup = 0; - for (Node node : nodes) { - if (node.isWorking() == Boolean.TRUE) { - activeDocumentsInGroup += node.getActiveDocuments(); - } - } - activeDocuments.set(activeDocumentsInGroup); - + void aggregateNodeValues() { + activeDocuments.set(nodes.stream().filter(node -> node.isWorking() == Boolean.TRUE).mapToLong(Node::getActiveDocuments).sum()); + isBlockingWrites.set(nodes.stream().anyMatch(node -> node.isBlockingWrites())); } /** Returns the active documents on this node. If unknown, 0 is returned. */ - long getActiveDocuments() { - return this.activeDocuments.get(); - } + long getActiveDocuments() { return activeDocuments.get(); } + + /** Returns whether any node in this group is currently blocking write operations */ + public boolean isBlockingWrites() { return isBlockingWrites.get(); } public boolean isFullCoverageStatusChanged(boolean hasFullCoverageNow) { boolean previousState = hasFullCoverage.getAndSet(hasFullCoverageNow); diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Node.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Node.java index 2f70c37cd48..8f465070de4 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Node.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Node.java @@ -21,6 +21,9 @@ public class Node { private final AtomicBoolean statusIsKnown = new AtomicBoolean(false); private final AtomicBoolean working = new AtomicBoolean(true); private final AtomicLong activeDocuments = new AtomicLong(0); + private final AtomicLong pingSequence = new AtomicLong(0); + private final AtomicLong lastPong = new AtomicLong(0); + private final AtomicBoolean isBlockingWrites = new AtomicBoolean(false); public Node(int key, String hostname, int group) { this.key = key; @@ -28,6 +31,18 @@ public class Node { this.group = group; } + /** Give a monotonically increasing sequence number.*/ + public long createPingSequenceId() { return pingSequence.incrementAndGet(); } + /** Checks if this pong is received in line and accepted, or out of band and should be ignored..*/ + public boolean isLastReceivedPong(long pingId ) { + long last = lastPong.get(); + while ((pingId > last) && ! lastPong.compareAndSet(last, pingId)) { + last = lastPong.get(); + } + return last < pingId; + } + public long getLastReceivedPongId() { return lastPong.get(); } + /** Returns the unique and stable distribution key of this node */ public int key() { return key; } @@ -56,14 +71,14 @@ public class Node { } /** Updates the active documents on this node */ - void setActiveDocuments(long activeDocuments) { - this.activeDocuments.set(activeDocuments); - } + void setActiveDocuments(long activeDocuments) { this.activeDocuments.set(activeDocuments); } /** Returns the active documents on this node. If unknown, 0 is returned. */ - long getActiveDocuments() { - return activeDocuments.get(); - } + long getActiveDocuments() { return activeDocuments.get(); } + + public void setBlockingWrites(boolean isBlockingWrites) { this.isBlockingWrites.set(isBlockingWrites); } + + boolean isBlockingWrites() { return isBlockingWrites.get(); } @Override public int hashCode() { return Objects.hash(hostname, key, pathIndex, group); } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/PingFactory.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/PingFactory.java index b16fa941f68..2e07d8d61e6 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/PingFactory.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/PingFactory.java @@ -1,13 +1,11 @@ // Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.dispatch.searchcluster; -import com.yahoo.prelude.Pong; import com.yahoo.search.cluster.ClusterMonitor; -import java.util.concurrent.Callable; public interface PingFactory { - Callable<Pong> createPinger(Node node, ClusterMonitor<Node> monitor); + Pinger createPinger(Node node, ClusterMonitor<Node> monitor, PongHandler pongHandler); } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Pinger.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Pinger.java new file mode 100644 index 00000000000..b4a7ccbf98c --- /dev/null +++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Pinger.java @@ -0,0 +1,12 @@ +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.search.dispatch.searchcluster; + +/** + * Send a ping and ensure that the pong is propagated to the ponghandler. + * Should not wait as this should be done in parallel on all nodes. + * + * @author baldersheim + */ +public interface Pinger { + void ping(); +} diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/PongHandler.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/PongHandler.java new file mode 100644 index 00000000000..c0579b5d36e --- /dev/null +++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/PongHandler.java @@ -0,0 +1,12 @@ +package com.yahoo.search.dispatch.searchcluster; + +import com.yahoo.prelude.Pong; + +/** + * Handle the Pong result of a Ping. + * + * @author baldersheim + */ +public interface PongHandler { + void handle(Pong pong); +} diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java index 5f211c37917..7dfc03fd2d7 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java @@ -10,7 +10,7 @@ import com.yahoo.net.HostName; import com.yahoo.prelude.Pong; import com.yahoo.search.cluster.ClusterMonitor; import com.yahoo.search.cluster.NodeManager; -import com.yahoo.search.result.ErrorMessage; +import com.yahoo.search.dispatch.TopKEstimator; import com.yahoo.vespa.config.search.DispatchConfig; import java.util.LinkedHashMap; @@ -18,13 +18,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.OptionalInt; -import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; -import java.util.concurrent.FutureTask; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; -import java.util.function.Predicate; -import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -43,9 +37,9 @@ public class SearchCluster implements NodeManager<Node> { private final ImmutableMap<Integer, Group> groups; private final ImmutableMultimap<String, Node> nodesByHost; private final ImmutableList<Group> orderedGroups; - private final ClusterMonitor<Node> clusterMonitor; private final VipStatus vipStatus; - private PingFactory pingFactory; + private final PingFactory pingFactory; + private final TopKEstimator hitEstimator; private long nextLogTime = 0; /** @@ -58,10 +52,12 @@ public class SearchCluster implements NodeManager<Node> { */ private final Optional<Node> localCorpusDispatchTarget; - public SearchCluster(String clusterId, DispatchConfig dispatchConfig, int containerClusterSize, VipStatus vipStatus) { + public SearchCluster(String clusterId, DispatchConfig dispatchConfig, int containerClusterSize, + VipStatus vipStatus, PingFactory pingFactory) { this.clusterId = clusterId; this.dispatchConfig = dispatchConfig; this.vipStatus = vipStatus; + this.pingFactory = pingFactory; List<Node> nodes = toNodes(dispatchConfig); this.size = nodes.size(); @@ -82,31 +78,28 @@ public class SearchCluster implements NodeManager<Node> { for (Node node : nodes) nodesByHostBuilder.put(node.hostname(), node); this.nodesByHost = nodesByHostBuilder.build(); + hitEstimator = new TopKEstimator(30.0, dispatchConfig.topKProbability()); this.localCorpusDispatchTarget = findLocalCorpusDispatchTarget(HostName.getLocalhost(), size, containerClusterSize, nodesByHost, groups); - - this.clusterMonitor = new ClusterMonitor<>(this); } - public void shutDown() { - clusterMonitor.shutdown(); + /* Testing only */ + public SearchCluster(String clusterId, DispatchConfig dispatchConfig, + VipStatus vipStatus, PingFactory pingFactory) { + this(clusterId, dispatchConfig, 1, vipStatus, pingFactory); } - public void startClusterMonitoring(PingFactory pingFactory) { - this.pingFactory = pingFactory; - - for (var group : orderedGroups) { + public void addMonitoring(ClusterMonitor clusterMonitor) { + for (var group : orderedGroups()) { for (var node : group.nodes()) clusterMonitor.add(node, true); } } - ClusterMonitor<Node> clusterMonitor() { return clusterMonitor; } - private static Optional<Node> findLocalCorpusDispatchTarget(String selfHostname, int searchClusterSize, int containerClusterSize, @@ -157,8 +150,8 @@ public class SearchCluster implements NodeManager<Node> { /** Returns the n'th (zero-indexed) group in the cluster if possible */ public Optional<Group> group(int n) { - if (orderedGroups.size() > n) { - return Optional.of(orderedGroups.get(n)); + if (orderedGroups().size() > n) { + return Optional.of(orderedGroups().get(n)); } else { return Optional.empty(); } @@ -166,13 +159,13 @@ public class SearchCluster implements NodeManager<Node> { /** Returns the number of nodes per group - size()/groups.size() */ public int groupSize() { - if (groups.size() == 0) return size(); - return size() / groups.size(); + if (groups().size() == 0) return size(); + return size() / groups().size(); } public int groupsWithSufficientCoverage() { int covered = 0; - for (Group g : orderedGroups) { + for (Group g : orderedGroups()) { if (g.hasSufficientCoverage()) { covered++; } @@ -188,7 +181,7 @@ public class SearchCluster implements NodeManager<Node> { if ( localCorpusDispatchTarget.isEmpty()) return Optional.empty(); // Only use direct dispatch if the local group has sufficient coverage - Group localSearchGroup = groups.get(localCorpusDispatchTarget.get().group()); + Group localSearchGroup = groups().get(localCorpusDispatchTarget.get().group()); if ( ! localSearchGroup.hasSufficientCoverage()) return Optional.empty(); // Only use direct dispatch if the local search node is not down @@ -227,7 +220,10 @@ public class SearchCluster implements NodeManager<Node> { setInRotationOnlyIf(hasWorkingNodes()); } else if (usesLocalCorpusIn(node)) { // follow the status of this node - setInRotationOnlyIf(nodeIsWorking); + // Do not take this out of rotation if we're a combined cluster of size 1, + // as that can't be helpful, and leads to a deadlock where this node is never taken back in servic e + if (nodeIsWorking || size() > 1) + setInRotationOnlyIf(nodeIsWorking); } } @@ -247,7 +243,14 @@ public class SearchCluster implements NodeManager<Node> { vipStatus.removeFromRotation(clusterId); } - private boolean hasInformationAboutAllNodes() { + public int estimateHitsToFetch(int wantedHits, int numPartitions) { + return hitEstimator.estimateK(wantedHits, numPartitions); + } + public int estimateHitsToFetch(int wantedHits, int numPartitions, double topKProbability) { + return hitEstimator.estimateK(wantedHits, numPartitions, topKProbability); + } + + public boolean hasInformationAboutAllNodes() { return nodesByHost.values().stream().allMatch(node -> node.isWorking() != null); } @@ -263,29 +266,41 @@ public class SearchCluster implements NodeManager<Node> { return localCorpusDispatchTarget.isPresent() && localCorpusDispatchTarget.get().group() == group.id(); } - /** Used by the cluster monitor to manage node status */ - @Override - public void ping(Node node, Executor executor) { - if (pingFactory == null) return; // not initialized yet + private static class PongCallback implements PongHandler { - FutureTask<Pong> futurePong = new FutureTask<>(pingFactory.createPinger(node, clusterMonitor)); - executor.execute(futurePong); - Pong pong = getPong(futurePong, node); - futurePong.cancel(true); + private final ClusterMonitor<Node> clusterMonitor; + private final Node node; - if (pong.badResponse()) { - clusterMonitor.failed(node, pong.error().get()); - } else { - if (pong.activeDocuments().isPresent()) { - node.setActiveDocuments(pong.activeDocuments().get()); + PongCallback(Node node, ClusterMonitor<Node> clusterMonitor) { + this.node = node; + this.clusterMonitor = clusterMonitor; + } + + @Override + public void handle(Pong pong) { + if (pong.badResponse()) { + clusterMonitor.failed(node, pong.error().get()); + } else { + if (pong.activeDocuments().isPresent()) { + node.setActiveDocuments(pong.activeDocuments().get()); + node.setBlockingWrites(pong.isBlockingWrites()); + } + clusterMonitor.responded(node); } - clusterMonitor.responded(node); } + + } + + /** Used by the cluster monitor to manage node status */ + @Override + public void ping(ClusterMonitor clusterMonitor, Node node, Executor executor) { + Pinger pinger = pingFactory.createPinger(node, clusterMonitor, new PongCallback(node, clusterMonitor)); + pinger.ping(); } private void pingIterationCompletedSingleGroup() { - Group group = groups.values().iterator().next(); - group.aggregateActiveDocuments(); + Group group = groups().values().iterator().next(); + group.aggregateNodeValues(); // With just one group sufficient coverage may not be the same as full coverage, as the // group will always be marked sufficient for use. updateSufficientCoverage(group, true); @@ -295,21 +310,20 @@ public class SearchCluster implements NodeManager<Node> { } private void pingIterationCompletedMultipleGroups() { - int numGroups = orderedGroups.size(); + int numGroups = orderedGroups().size(); // Update active documents per group and use it to decide if the group should be active - long[] activeDocumentsInGroup = new long[numGroups]; long sumOfActiveDocuments = 0; for(int i = 0; i < numGroups; i++) { - Group group = orderedGroups.get(i); - group.aggregateActiveDocuments(); + Group group = orderedGroups().get(i); + group.aggregateNodeValues(); activeDocumentsInGroup[i] = group.getActiveDocuments(); sumOfActiveDocuments += activeDocumentsInGroup[i]; } boolean anyGroupsSufficientCoverage = false; for (int i = 0; i < numGroups; i++) { - Group group = orderedGroups.get(i); + Group group = orderedGroups().get(i); long activeDocuments = activeDocumentsInGroup[i]; long averageDocumentsInOtherGroups = (sumOfActiveDocuments - activeDocuments) / (numGroups - 1); boolean sufficientCoverage = isGroupCoverageSufficient(group.workingNodes(), group.nodes().size(), activeDocuments, averageDocumentsInOtherGroups); @@ -326,7 +340,7 @@ public class SearchCluster implements NodeManager<Node> { */ @Override public void pingIterationCompleted() { - int numGroups = orderedGroups.size(); + int numGroups = orderedGroups().size(); if (numGroups == 1) { pingIterationCompletedSingleGroup(); } else { @@ -353,25 +367,11 @@ public class SearchCluster implements NodeManager<Node> { return workingNodes + nodesAllowedDown >= nodesInGroup; } - private Pong getPong(FutureTask<Pong> futurePong, Node node) { - try { - return futurePong.get(clusterMonitor.getConfiguration().getFailLimit(), TimeUnit.MILLISECONDS); - } catch (InterruptedException e) { - log.log(Level.WARNING, "Exception pinging " + node, e); - return new Pong(ErrorMessage.createUnspecifiedError("Ping was interrupted: " + node)); - } catch (ExecutionException e) { - log.log(Level.WARNING, "Exception pinging " + node, e); - return new Pong(ErrorMessage.createUnspecifiedError("Execution was interrupted: " + node)); - } catch (TimeoutException e) { - return new Pong(ErrorMessage.createNoAnswerWhenPingingNode("Ping thread timed out")); - } - } - /** * Calculate whether a subset of nodes in a group has enough coverage */ public boolean isPartialGroupCoverageSufficient(OptionalInt knownGroupId, List<Node> nodes) { - if (orderedGroups.size() == 1) { + if (orderedGroups().size() == 1) { boolean sufficient = nodes.size() >= groupSize() - dispatchConfig.maxNodesDownPerGroup(); return sufficient; } @@ -380,14 +380,14 @@ public class SearchCluster implements NodeManager<Node> { return false; } int groupId = knownGroupId.getAsInt(); - Group group = groups.get(groupId); + Group group = groups().get(groupId); if (group == null) { return false; } int nodesInGroup = group.nodes().size(); long sumOfActiveDocuments = 0; int otherGroups = 0; - for (Group g : orderedGroups) { + for (Group g : orderedGroups()) { if (g.id() != groupId) { sumOfActiveDocuments += g.getActiveDocuments(); otherGroups++; @@ -402,6 +402,7 @@ public class SearchCluster implements NodeManager<Node> { } private void trackGroupCoverageChanges(int index, Group group, boolean fullCoverage, long averageDocuments) { + if ( ! hasInformationAboutAllNodes()) return; // Be silent until we know what we are talking about. boolean changed = group.isFullCoverageStatusChanged(fullCoverage); if (changed || (!fullCoverage && System.currentTimeMillis() > nextLogTime)) { nextLogTime = System.currentTimeMillis() + 30 * 1000; diff --git a/container-search/src/main/java/com/yahoo/search/federation/FederationResult.java b/container-search/src/main/java/com/yahoo/search/federation/FederationResult.java index 5f1cfccf549..6243dc694c2 100644 --- a/container-search/src/main/java/com/yahoo/search/federation/FederationResult.java +++ b/container-search/src/main/java/com/yahoo/search/federation/FederationResult.java @@ -39,8 +39,8 @@ class FederationResult { } /** - * Wait on each target for that targets timeout - * On the worst case this is the same as waiting for the max target timeout, + * Wait on each target for that targets timeout. + * In the worst case this is the same as waiting for the max target timeout, * in the average case it may be much better because lower timeout sources do not get to * drive the timeout above their own timeout value. * When this completes, results can be accessed from the TargetResults with no blocking diff --git a/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java b/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java index c3ede4fe20a..60c5d42c531 100644 --- a/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java +++ b/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java @@ -78,7 +78,6 @@ public class FederationSearcher extends ForkingSearcher { /** The name of the query property containing the source name added to the query to each source by this */ public final static CompoundName SOURCENAME = new CompoundName("sourceName"); public final static CompoundName PROVIDERNAME = new CompoundName("providerName"); - /** Logging field name constants */ public static final String LOG_COUNT_PREFIX = "count_"; @@ -110,7 +109,7 @@ public class FederationSearcher extends ForkingSearcher { // for testing public FederationSearcher(ComponentId id, SearchChainResolver searchChainResolver) { - this(searchChainResolver, false, PropagateSourceProperties.ALL, null); + this(searchChainResolver, false, PropagateSourceProperties.EVERY, null); } private FederationSearcher(SearchChainResolver searchChainResolver, @@ -271,7 +270,10 @@ public class FederationSearcher extends ForkingSearcher { outgoing.setTimeout(timeout); switch (propagateSourceProperties) { - case ALL: + case EVERY: + propagatePerSourceQueryProperties(query, outgoing, window, sourceName, providerName, null); + break; + case NATIVE: case ALL: propagatePerSourceQueryProperties(query, outgoing, window, sourceName, providerName, Query.nativeProperties); break; case OFFSET_HITS: @@ -288,10 +290,21 @@ public class FederationSearcher extends ForkingSearcher { private void propagatePerSourceQueryProperties(Query original, Query outgoing, Window window, String sourceName, String providerName, List<CompoundName> queryProperties) { - for (CompoundName key : queryProperties) { - Object value = getSourceOrProviderProperty(original, key, sourceName, providerName, window.get(key)); - if (value != null) - outgoing.properties().set(key, value); + if (queryProperties == null) { + outgoing.setHits(window.hits); + outgoing.setOffset(window.offset); + original.properties().listProperties(CompoundName.fromComponents("provider", providerName)).forEach((k, v) -> + outgoing.properties().set(k, v)); + original.properties().listProperties(CompoundName.fromComponents("source", sourceName)).forEach((k, v) -> + outgoing.properties().set(k, v)); + } + else { + for (CompoundName key : queryProperties) { + Object value = getSourceOrProviderProperty(original, key, sourceName, providerName, window.get(key)); + if (value != null) + outgoing.properties().set(key, value); + if (value != null) System.out.println("Setting " + key + " = " + value); + } } } @@ -319,7 +332,7 @@ public class FederationSearcher extends ForkingSearcher { private ErrorMessage missingSearchChainsErrorMessage(List<UnresolvedSearchChainException> unresolvedSearchChainExceptions) { String message = String.join(" ", getMessagesSet(unresolvedSearchChainExceptions)) + - " Valid source refs are " + String.join(", ", allSourceRefDescriptions()) +'.'; + " Valid source refs are " + String.join(", ", allSourceRefDescriptions()) +'.'; return ErrorMessage.createInvalidQueryParameter(message); } @@ -341,7 +354,7 @@ public class FederationSearcher extends ForkingSearcher { } private void warnIfUnresolvedSearchChains(List<UnresolvedSearchChainException> missingTargets, - HitGroup errorHitGroup) { + HitGroup errorHitGroup) { if (!missingTargets.isEmpty()) { errorHitGroup.addError(missingSearchChainsErrorMessage(missingTargets)); } @@ -479,9 +492,9 @@ public class FederationSearcher extends ForkingSearcher { * TODO This is probably a dirty hack for bug 4711376. There are probably better ways. * But I will leave that to trd-processing@ * - * @param group The merging hitgroup to be updated if necessary - * @param orderer The per provider hit orderer. - * @return The hitorderer chosen + * @param group the merging hitgroup to be updated if necessary + * @param orderer the per provider hit orderer + * @return he hitorderer chosen */ private HitOrderer dirtyCopyIfModifiedOrderer(HitGroup group, HitOrderer orderer) { if (orderer != null) { diff --git a/container-search/src/main/java/com/yahoo/search/handler/HttpSearchResponse.java b/container-search/src/main/java/com/yahoo/search/handler/HttpSearchResponse.java index 3602d21f7d8..d636d3bc925 100644 --- a/container-search/src/main/java/com/yahoo/search/handler/HttpSearchResponse.java +++ b/container-search/src/main/java/com/yahoo/search/handler/HttpSearchResponse.java @@ -23,6 +23,7 @@ import com.yahoo.processing.rendering.Renderer; import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.query.context.QueryContext; +import com.yahoo.yolean.trace.TraceNode; /** * Wrap the result of a query as an HTTP response. @@ -36,8 +37,13 @@ public class HttpSearchResponse extends ExtendedResponse { private final Renderer<Result> rendererCopy; private final Timing timing; private final HitCounts hitCounts; + private final TraceNode trace; public HttpSearchResponse(int status, Result result, Query query, Renderer renderer) { + this(status, result, query, renderer, null); + } + + HttpSearchResponse(int status, Result result, Query query, Renderer renderer, TraceNode trace) { super(status); this.query = query; this.result = result; @@ -45,6 +51,7 @@ public class HttpSearchResponse extends ExtendedResponse { this.timing = SearchResponse.createTiming(query, result); this.hitCounts = SearchResponse.createHitCounts(query, result); + this.trace = trace; populateHeaders(headers(), result.getHeaders(false)); } @@ -107,6 +114,9 @@ public class HttpSearchResponse extends ExtendedResponse { @Override public void populateAccessLogEntry(final AccessLogEntry accessLogEntry) { super.populateAccessLogEntry(accessLogEntry); + if (trace != null) { + accessLogEntry.setTrace(trace); + } populateAccessLogEntry(accessLogEntry, getHitCounts()); } diff --git a/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java b/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java index 0981c6e8dad..5e3b79c1545 100644 --- a/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java +++ b/container-search/src/main/java/com/yahoo/search/handler/SearchHandler.java @@ -25,11 +25,12 @@ import com.yahoo.prelude.query.QueryException; import com.yahoo.prelude.query.parser.ParseException; import com.yahoo.processing.rendering.Renderer; import com.yahoo.processing.request.CompoundName; +import com.yahoo.search.query.context.QueryContext; import com.yahoo.search.query.ranking.SoftTimeout; import com.yahoo.search.searchchain.ExecutionFactory; import com.yahoo.slime.Inspector; import com.yahoo.slime.ObjectTraverser; -import com.yahoo.vespa.config.SlimeUtils; +import com.yahoo.slime.SlimeUtils; import com.yahoo.yolean.Exceptions; import com.yahoo.search.Query; import com.yahoo.search.Result; @@ -49,6 +50,7 @@ import com.yahoo.statistics.Handle; import com.yahoo.statistics.Statistics; import com.yahoo.statistics.Value; import com.yahoo.vespa.configdefinition.SpecialtokensConfig; +import com.yahoo.yolean.trace.TraceNode; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -58,6 +60,7 @@ import java.util.Optional; import java.util.concurrent.Executor; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; import java.util.logging.Level; import java.util.logging.Logger; @@ -98,6 +101,8 @@ public class SearchHandler extends LoggingRequestHandler { private final ExecutionFactory executionFactory; + private final AtomicLong numRequestsLeftToTrace; + private final class MeanConnections implements Callback { @Override @@ -125,6 +130,7 @@ public class SearchHandler extends LoggingRequestHandler { accessLog, QueryProfileConfigurer.createFromConfig(queryProfileConfig).compile(), executionFactory, + containerHttpConfig.numQueriesToTraceOnDebugAfterConstruction(), containerHttpConfig.hostResponseHeaderKey().equals("") ? Optional.empty() : Optional.of( containerHttpConfig.hostResponseHeaderKey())); } @@ -136,6 +142,17 @@ public class SearchHandler extends LoggingRequestHandler { CompiledQueryProfileRegistry queryProfileRegistry, ExecutionFactory executionFactory, Optional<String> hostResponseHeaderKey) { + this(statistics, metric, executor, accessLog, queryProfileRegistry, executionFactory, 0, hostResponseHeaderKey); + } + + private SearchHandler(Statistics statistics, + Metric metric, + Executor executor, + AccessLog accessLog, + CompiledQueryProfileRegistry queryProfileRegistry, + ExecutionFactory executionFactory, + long numQueriesToTraceOnDebugAfterStartup, + Optional<String> hostResponseHeaderKey) { super(executor, accessLog, metric, true); log.log(LogLevel.DEBUG, "SearchHandler.init " + System.identityHashCode(this)); this.queryProfileRegistry = queryProfileRegistry; @@ -150,6 +167,7 @@ public class SearchHandler extends LoggingRequestHandler { .setCallback(new MeanConnections())); this.hostResponseHeaderKey = hostResponseHeaderKey; + this.numRequestsLeftToTrace = new AtomicLong(numQueriesToTraceOnDebugAfterStartup); } /** @deprecated use the other constructor */ @@ -215,7 +233,6 @@ public class SearchHandler extends LoggingRequestHandler { } - @SuppressWarnings("unchecked") private HttpResponse errorResponse(HttpRequest request, ErrorMessage errorMessage) { Query query = new Query(); Result result = new Result(query, errorMessage); @@ -281,7 +298,8 @@ public class SearchHandler extends LoggingRequestHandler { // Transform result to response Renderer renderer = toRendererCopy(query.getPresentation().getRenderer()); HttpSearchResponse response = new HttpSearchResponse(getHttpResponseStatus(request, result), - result, query, renderer); + result, query, renderer, + extractTraceNode(query)); if (hostResponseHeaderKey.isPresent()) response.headers().add(hostResponseHeaderKey.get(), selfHostname); @@ -292,6 +310,19 @@ public class SearchHandler extends LoggingRequestHandler { return response; } + private static TraceNode extractTraceNode(Query query) { + if (log.isLoggable(Level.FINE)) { + QueryContext queryContext = query.getContext(false); + if (queryContext != null) { + Execution.Trace trace = queryContext.getTrace(); + if (trace != null) { + return trace.traceNode(); + } + } + } + return null; + } + private static int getErrors(Result result) { return result.hits().getErrorHit() == null ? 0 : 1; } @@ -330,7 +361,13 @@ public class SearchHandler extends LoggingRequestHandler { Execution execution = executionFactory.newExecution(searchChain); query.getModel().setExecution(execution); - execution.trace().setForceTimestamps(query.properties().getBoolean(FORCE_TIMESTAMPS, false)); + if (log.isLoggable(Level.FINE) && (numRequestsLeftToTrace.getAndDecrement() > 0)) { + query.setTraceLevel(Math.max(1, query.getTraceLevel())); + execution.trace().setForceTimestamps(true); + + } else { + execution.trace().setForceTimestamps(query.properties().getBoolean(FORCE_TIMESTAMPS, false)); + } if (query.properties().getBoolean(DETAILED_TIMING_LOGGING, false)) { // check and set (instead of set directly) to avoid overwriting stuff from prepareForBreakdownAnalysis() execution.context().setDetailedDiagnostics(true); @@ -389,7 +426,7 @@ public class SearchHandler extends LoggingRequestHandler { } catch (ParseException e) { ErrorMessage error = ErrorMessage.createIllegalQuery("Could not parse query [" + request + "]: " + Exceptions.toMessageString(e)); - log.log(LogLevel.DEBUG, () -> error.getDetailedMessage()); + log.log(LogLevel.DEBUG, error::getDetailedMessage); return new Result(query, error); } catch (IllegalArgumentException e) { if ("Comparison method violates its general contract!".equals(e.getMessage())) { @@ -401,7 +438,7 @@ public class SearchHandler extends LoggingRequestHandler { else { ErrorMessage error = ErrorMessage.createBadRequest("Invalid search request [" + request + "]: " + Exceptions.toMessageString(e)); - log.log(LogLevel.DEBUG, () -> error.getDetailedMessage()); + log.log(LogLevel.DEBUG, error::getDetailedMessage); return new Result(query, error); } } catch (LinkageError | StackOverflowError e) { diff --git a/container-search/src/main/java/com/yahoo/search/query/Properties.java b/container-search/src/main/java/com/yahoo/search/query/Properties.java index a1a70b4c3ba..a0cd4137e9f 100644 --- a/container-search/src/main/java/com/yahoo/search/query/Properties.java +++ b/container-search/src/main/java/com/yahoo/search/query/Properties.java @@ -30,8 +30,9 @@ public abstract class Properties extends com.yahoo.processing.request.Properties return (Properties)super.clone(); } - /** The query owning this property object. - * Only guaranteed to work if this instance is accessible as query.properties() + /** + * Returns the query owning this property object. + * Only guaranteed to work if this instance is accessible as query.properties() */ public Query getParentQuery() { if (chained() == null) { @@ -48,4 +49,5 @@ public abstract class Properties extends com.yahoo.processing.request.Properties if (chained() != null) chained().setParentQuery(query); } + } diff --git a/container-search/src/main/java/com/yahoo/search/query/Ranking.java b/container-search/src/main/java/com/yahoo/search/query/Ranking.java index 7444c94f491..830a3f4ef81 100644 --- a/container-search/src/main/java/com/yahoo/search/query/Ranking.java +++ b/container-search/src/main/java/com/yahoo/search/query/Ranking.java @@ -47,7 +47,7 @@ public class Ranking implements Cloneable { public static final String PROPERTIES = "properties"; static { - argumentType =new QueryProfileType(RANKING); + argumentType = new QueryProfileType(RANKING); argumentType.setStrict(true); argumentType.setBuiltin(true); argumentType.addField(new FieldDescription(LOCATION, "string", "location")); @@ -63,7 +63,7 @@ public class Ranking implements Cloneable { argumentType.addField(new FieldDescription(FEATURES, "query-profile", "rankfeature")); argumentType.addField(new FieldDescription(PROPERTIES, "query-profile", "rankproperty")); argumentType.freeze(); - argumentTypeName=new CompoundName(argumentType.getId().getName()); + argumentTypeName = new CompoundName(argumentType.getId().getName()); } public static QueryProfileType getArgumentType() { return argumentType; } diff --git a/container-search/src/main/java/com/yahoo/search/query/Select.java b/container-search/src/main/java/com/yahoo/search/query/Select.java index cb662dcd671..65ffd29efe0 100644 --- a/container-search/src/main/java/com/yahoo/search/query/Select.java +++ b/container-search/src/main/java/com/yahoo/search/query/Select.java @@ -57,12 +57,13 @@ public class Select implements Cloneable { } public Select(String where, String grouping, Query query) { - this(where, grouping, query, Collections.emptyList()); + this(where, grouping, null, query, Collections.emptyList()); } - private Select(String where, String grouping, Query query, List<GroupingRequest> groupingRequests) { + private Select(String where, String grouping, String groupingExpressionString, Query query, List<GroupingRequest> groupingRequests) { this.where = Objects.requireNonNull(where, "A Select must have a where string (possibly the empty string)"); this.grouping = Objects.requireNonNull(grouping, "A Select must have a select string (possibly the empty string)"); + this.groupingExpressionString = groupingExpressionString; this.parent = Objects.requireNonNull(query, "A Select must have a parent query"); this.groupingRequests = deepCopy(groupingRequests, this); } @@ -136,11 +137,11 @@ public class Select implements Cloneable { @Override public Object clone() { - return new Select(where, grouping, parent, groupingRequests); + return new Select(where, grouping, groupingExpressionString, parent, groupingRequests); } public Select cloneFor(Query parent) { - return new Select(where, grouping, parent, groupingRequests); + return new Select(where, grouping, groupingExpressionString, parent, groupingRequests); } } diff --git a/container-search/src/main/java/com/yahoo/search/query/SelectParser.java b/container-search/src/main/java/com/yahoo/search/query/SelectParser.java index ae50756331c..775dca7c444 100644 --- a/container-search/src/main/java/com/yahoo/search/query/SelectParser.java +++ b/container-search/src/main/java/com/yahoo/search/query/SelectParser.java @@ -45,8 +45,7 @@ import com.yahoo.search.yql.VespaGroupingStep; import com.yahoo.slime.ArrayTraverser; import com.yahoo.slime.Inspector; import com.yahoo.slime.ObjectTraverser; -import com.yahoo.slime.Type; -import com.yahoo.vespa.config.SlimeUtils; +import com.yahoo.slime.SlimeUtils; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/BackedOverridableQueryProfile.java b/container-search/src/main/java/com/yahoo/search/query/profile/BackedOverridableQueryProfile.java index 99f1e26b221..11864e60cec 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/BackedOverridableQueryProfile.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/BackedOverridableQueryProfile.java @@ -135,8 +135,8 @@ public class BackedOverridableQueryProfile extends OverridableQueryProfile imple @Override public List<String> getDimensions() { - List<String> dimensions=super.getDimensions(); - if (dimensions!=null) return dimensions; + List<String> dimensions = super.getDimensions(); + if (dimensions != null) return dimensions; return backingProfile.getDimensions(); } diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/DimensionBinding.java b/container-search/src/main/java/com/yahoo/search/query/profile/DimensionBinding.java index 50bd2c58da8..0cbfdc5dca0 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/DimensionBinding.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/DimensionBinding.java @@ -3,7 +3,6 @@ package com.yahoo.search.query.profile; import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -22,7 +21,7 @@ public class DimensionBinding { private DimensionValues values; /** The binding from those dimensions to values, and possibly other values */ - private Map<String, String> context; // TODO: This is not needed any more + private Map<String, String> context; public static final DimensionBinding nullBinding = new DimensionBinding(Collections.unmodifiableList(Collections.emptyList()), DimensionValues.empty, null); @@ -34,13 +33,13 @@ public class DimensionBinding { private boolean containsAllNulls; // NOTE: Map must be ordered - public static DimensionBinding createFrom(Map<String,String> values) { + public static DimensionBinding createFrom(Map<String, String> values) { return createFrom(new ArrayList<>(values.keySet()), values); } /** Creates a binding from a variant and a context. Any of the arguments may be null. */ // NOTE: Map must be ordered - public static DimensionBinding createFrom(List<String> dimensions, Map<String,String> context) { + public static DimensionBinding createFrom(List<String> dimensions, Map<String, String> context) { if (dimensions == null || dimensions.size() == 0) { if (context == null) return nullBinding; if (dimensions == null) return new DimensionBinding(null, DimensionValues.empty, context); // Null, but must preserve context @@ -51,7 +50,7 @@ public class DimensionBinding { /** Creates a binding from a variant and a context. Any of the arguments may be null. */ public static DimensionBinding createFrom(List<String> dimensions, DimensionValues dimensionValues) { - if (dimensionValues==null || dimensionValues == DimensionValues.empty) return nullBinding; + if (dimensionValues == null || dimensionValues == DimensionValues.empty) return nullBinding; // If null, preserve raw material for creating a context later (in createFor) if (dimensions == null) return new DimensionBinding(null, dimensionValues, null); @@ -61,14 +60,13 @@ public class DimensionBinding { /** Returns a binding for a (possibly) new set of variants. Variants may be null, but not bindings */ public DimensionBinding createFor(List<String> newDimensions) { - if (newDimensions==null) return this; // Note: Not necessarily null - if no new variants then keep the existing binding - // if (this.context==null && values.length==0) return nullBinding; // No data from which to create a non-null binding - if (this.dimensions==newDimensions) return this; // Avoid creating a new object if the dimensions are the same - - Map<String,String> context=this.context; - if (context==null) - context=this.values.asContext(this.dimensions !=null ? this.dimensions : newDimensions); - return new DimensionBinding(newDimensions,extractDimensionValues(newDimensions,context),context); + if (newDimensions == null) return this; // Note: Not necessarily null - if no new variants then keep the existing binding + if (this.dimensions == newDimensions) return this; // Avoid creating a new object if the dimensions are the same + + Map<String,String> context = this.context; + if (context == null) + context = this.values.asContext(this.dimensions != null ? this.dimensions : newDimensions); + return new DimensionBinding(newDimensions, extractDimensionValues(newDimensions, context), context); } /** @@ -76,20 +74,20 @@ public class DimensionBinding { * The array will not be modified. The context is needed in order to convert this binding to another * given another set of variant dimensions. */ - private DimensionBinding(List<String> dimensions, DimensionValues values, Map<String,String> context) { - this.dimensions=dimensions; - this.values=values; + private DimensionBinding(List<String> dimensions, DimensionValues values, Map<String, String> context) { + this.dimensions = dimensions; + this.values = values; this.context = context; - containsAllNulls=values.isEmpty(); + containsAllNulls = values.isEmpty(); } /** Returns a read-only list of the dimensions of this. This value is undefined if this isNull() */ public List<String> getDimensions() { return dimensions; } /** Returns a context created from the dimensions and values of this */ - public Map<String,String> getContext() { - if (context !=null) return context; - context =values.asContext(dimensions); + public Map<String, String> getContext() { + if (context != null) return context; + context = values.asContext(dimensions); return context; } @@ -102,7 +100,7 @@ public class DimensionBinding { public DimensionValues getValues() { return values; } /** Returns true only if this binding is null (contains no values for its dimensions (if any) */ - public boolean isNull() { return dimensions==null || containsAllNulls; } + public boolean isNull() { return dimensions == null || containsAllNulls; } /** * Returns an array of the dimension values corresponding to the dimensions of this from the given context, @@ -110,10 +108,10 @@ public class DimensionBinding { * Dimensions which are not set in this context get a null value. */ private static DimensionValues extractDimensionValues(List<String> dimensions, Map<String,String> context) { - String[] dimensionValues=new String[dimensions.size()]; - if (context==null || context.size()==0) return DimensionValues.createFrom(dimensionValues); - for (int i=0; i<dimensions.size(); i++) - dimensionValues[i]=context.get(dimensions.get(i)); + String[] dimensionValues = new String[dimensions.size()]; + if (context == null || context.size() == 0) return DimensionValues.createFrom(dimensionValues); + for (int i = 0; i < dimensions.size(); i++) + dimensionValues[i] = context.get(dimensions.get(i)); return DimensionValues.createFrom(dimensionValues); } @@ -138,16 +136,6 @@ public class DimensionBinding { return DimensionBinding.createFrom(combinedDimensions, combinedValues); } - /** Returns the binding of this (dimension->value) as a map */ - private Map<String, String> asMap() { - Map<String, String> map = new LinkedHashMap<>(); - for (int i = 0; i < Math.min(dimensions.size(), values.size()); i++) { - if (values.getValues()[i] != null) - map.put(dimensions.get(i), values.getValues()[i]); - } - return map; - } - /** * Returns a combined list of dimensions from two separate lists, * or null if they are incompatible. @@ -155,8 +143,12 @@ public class DimensionBinding { * (or return null if impossible). */ private List<String> combineDimensions(List<String> d1, List<String> d2) { + if (d1.equals(d2)) return d1; + if (d1.isEmpty()) return d2; + if (d2.isEmpty()) return d1; + List<String> combined = new ArrayList<>(); - int d1Index = 0, d2Index=0; + int d1Index = 0, d2Index = 0; while (d1Index < d1.size() && d2Index < d2.size()) { if (d1.get(d1Index).equals(d2.get(d2Index))) { // agreement on next element combined.add(d1.get(d1Index)); @@ -186,27 +178,22 @@ public class DimensionBinding { * or null if they are incompatible. */ private Map<String, String> combineValues(Map<String, String> m1, Map<String, String> m2) { - Map<String, String> combinedValues = new LinkedHashMap<>(m1); + if (m1.isEmpty()) return m2; + if (m2.isEmpty()) return m1; + Map<String, String> combinedValues = null; for (Map.Entry<String, String> m2Entry : m2.entrySet()) { if (m2Entry.getValue() == null) continue; String m1Value = m1.get(m2Entry.getKey()); if (m1Value != null && ! m1Value.equals(m2Entry.getValue())) return null; // conflicting values of a key + if (combinedValues == null) + combinedValues = new LinkedHashMap<>(m1); combinedValues.put(m2Entry.getKey(), m2Entry.getValue()); } - return combinedValues; + return combinedValues == null ? m1 : combinedValues; } - private boolean intersects(List<String> l1, List<String> l2) { - for (String l1Item : l1) - if (l2.contains(l1Item)) - return true; - return false; - } - - /** - * Returns true if <code>this == invalidBinding</code> - */ + /** Returns true if this == invalidBinding */ public boolean isInvalid() { return this == invalidBinding; } @Override @@ -226,7 +213,7 @@ public class DimensionBinding { /** Two bindings are equal if they contain the same dimensions and the same non-null values */ @Override public boolean equals(Object o) { - if (o==this) return true; + if (o == this) return true; if (! (o instanceof DimensionBinding)) return false; DimensionBinding other = (DimensionBinding)o; if ( ! this.dimensions.equals(other.dimensions)) return false; diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/ModelObjectMap.java b/container-search/src/main/java/com/yahoo/search/query/profile/ModelObjectMap.java index 4dc9ade62e5..e32d2dc226d 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/ModelObjectMap.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/ModelObjectMap.java @@ -22,8 +22,7 @@ public class ModelObjectMap extends PropertyMap { */ @Override protected boolean shouldSet(CompoundName name, Object value) { - if (value == null) return true; - return ! FieldType.isLegalFieldValue(value); + return value != null && ! FieldType.isLegalFieldValue(value); } } diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfile.java b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfile.java index e9ccdd22f98..ab0f129f1e9 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfile.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfile.java @@ -100,7 +100,7 @@ public class QueryProfile extends FreezableSimpleComponent implements Cloneable public QueryProfileType getType() { return type; } /** Sets the type of this, or set to null to not use any type checking in this profile */ - public void setType(QueryProfileType type) { this.type=type; } + public void setType(QueryProfileType type) { this.type = type; } /** Returns the virtual variants of this, or null if none */ public QueryProfileVariants getVariants() { return variants; } @@ -564,7 +564,7 @@ public class QueryProfile extends FreezableSimpleComponent implements Cloneable if (visitor.isDone()) return; if (allowContent) { - visitContent(visitor,dimensionBinding); + visitContent(visitor, dimensionBinding); if (visitor.isDone()) return; } @@ -601,7 +601,7 @@ public class QueryProfile extends FreezableSimpleComponent implements Cloneable visitor.acceptValue(contentKey, getContent(contentKey), dimensionBinding, this, null); } else { // get all content in this - for (Map.Entry<String,Object> entry : getContent().entrySet()) { + for (Map.Entry<String, Object> entry : getContent().entrySet()) { visitor.acceptValue(entry.getKey(), entry.getValue(), dimensionBinding, this, null); if (visitor.isDone()) return; } @@ -614,7 +614,7 @@ public class QueryProfile extends FreezableSimpleComponent implements Cloneable } /** Returns all the content from this as an unmodifiable map */ - protected Map<String,Object> getContent() { + protected Map<String, Object> getContent() { return content.unmodifiableMap(); } diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileCompiler.java b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileCompiler.java index cffe941b912..f1fc90dee09 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileCompiler.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileCompiler.java @@ -71,6 +71,7 @@ public class QueryProfileCompiler { */ private static Set<DimensionBindingForPath> collectVariants(CompoundName path, QueryProfile profile, DimensionBinding currentVariant) { Set<DimensionBindingForPath> variants = new HashSet<>(); + variants.addAll(collectVariantsFromValues(path, profile.getContent(), currentVariant)); variants.addAll(collectVariantsInThis(path, profile, currentVariant)); if (profile instanceof BackedOverridableQueryProfile) @@ -157,6 +158,7 @@ public class QueryProfileCompiler { if (combinedVariant.isInvalid()) continue; // values at this point in the graph are unreachable + variants.add(new DimensionBindingForPath(combinedVariant, path)); variants.addAll(collectVariantsFromValues(path, variant.values(), combinedVariant)); for (QueryProfile variantInheritedProfile : variant.inherited()) variants.addAll(collectVariants(path, variantInheritedProfile, combinedVariant)); @@ -169,9 +171,6 @@ public class QueryProfileCompiler { Map<String, Object> values, DimensionBinding currentVariant) { Set<DimensionBindingForPath> variants = new HashSet<>(); - if ( ! values.isEmpty()) - variants.add(new DimensionBindingForPath(currentVariant, path)); // there are actual values for this variant - for (Map.Entry<String, Object> entry : values.entrySet()) { if (entry.getValue() instanceof QueryProfile) variants.addAll(collectVariants(path.append(entry.getKey()), (QueryProfile)entry.getValue(), currentVariant)); @@ -202,7 +201,7 @@ public class QueryProfileCompiler { @Override public int hashCode() { - return binding.hashCode() + 17*path.hashCode(); + return binding.hashCode() + 17 * path.hashCode(); } @Override diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileProperties.java b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileProperties.java index b250560e2f3..701ea7690f4 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileProperties.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileProperties.java @@ -9,6 +9,7 @@ import com.yahoo.search.query.Properties; import com.yahoo.search.query.profile.compiled.CompiledQueryProfile; import com.yahoo.search.query.profile.compiled.DimensionalValue; import com.yahoo.search.query.profile.types.FieldDescription; +import com.yahoo.search.query.profile.types.QueryProfileFieldType; import com.yahoo.search.query.profile.types.QueryProfileType; import java.util.ArrayList; @@ -31,7 +32,12 @@ public class QueryProfileProperties extends Properties { /** Values which has been overridden at runtime, or null if none */ private Map<CompoundName, Object> values = null; - /** Query profile references which has been overridden at runtime, or null if none. Earlier values has precedence */ + + /** + * Query profile references which has been overridden at runtime, possibly to the null value to clear values, + * or null if none (i.e this is lazy). + * Earlier values has precedence + */ private List<Pair<CompoundName, CompiledQueryProfile>> references = null; /** Creates an instance from a profile, throws an exception if the given profile is null */ @@ -48,20 +54,21 @@ public class QueryProfileProperties extends Properties { public Object get(CompoundName name, Map<String, String> context, com.yahoo.processing.request.Properties substitution) { name = unalias(name, context); - Object value = null; - if (values != null) - value = values.get(name); - if (value == null) { - Pair<CompoundName, CompiledQueryProfile> reference = findReference(name); - if (reference != null) - return reference.getSecond().get(name.rest(reference.getFirst().size()), context, substitution); // yes; even if null + if (values != null && values.containsKey(name)) + return values.get(name); // Returns this value, even if null + + Pair<CompoundName, CompiledQueryProfile> reference = findReference(name); + if (reference != null) { + if (reference.getSecond() == null) + return null; // cleared + else + return reference.getSecond().get(name.rest(reference.getFirst().size()), context, substitution); // even if null } - if (value == null) - value = profile.get(name, context, substitution); - if (value == null) - value = super.get(name, context, substitution); - return value; + Object value = profile.get(name, context, substitution); + if (value != null) + return value; + return super.get(name, context, substitution); } /** @@ -70,7 +77,7 @@ public class QueryProfileProperties extends Properties { * @throws IllegalArgumentException if this property cannot be set in the wrapped query profile */ @Override - public void set(CompoundName name, Object value, Map<String,String> context) { + public void set(CompoundName name, Object value, Map<String, String> context) { // TODO: Refactor try { name = unalias(name, context); @@ -87,8 +94,10 @@ public class QueryProfileProperties extends Properties { // Check types if ( ! profile.getTypes().isEmpty()) { - for (int i = 0; i<name.size(); i++) { - QueryProfileType type = profile.getType(name.first(i), context); + QueryProfileType type = null; + for (int i = 0; i < name.size(); i++) { + if (type == null) // We're on the first iteration, or no type is explicitly specified + type = profile.getType(name.first(i), context); if (type == null) continue; String localName = name.get(i); FieldDescription fieldDescription = type.getField(localName); @@ -97,12 +106,19 @@ public class QueryProfileProperties extends Properties { // TODO: In addition to strictness, check legality along the way - if (i == name.size()-1 && fieldDescription != null) { // at the end of the path, check the assignment type - value = fieldDescription.getType().convertFrom(value, profile.getRegistry()); - if (value == null) - throw new IllegalArgumentException("'" + value + "' is not a " + - fieldDescription.getType().toInstanceDescription()); + if (fieldDescription != null) { + if (i == name.size() - 1) { // at the end of the path, check the assignment type + value = fieldDescription.getType().convertFrom(value, profile.getRegistry()); + if (value == null) + throw new IllegalArgumentException("'" + value + "' is not a " + + fieldDescription.getType().toInstanceDescription()); + } + else if (fieldDescription.getType() instanceof QueryProfileFieldType) { + // If a type is specified, use that instead of the type implied by the name + type = ((QueryProfileFieldType) fieldDescription.getType()).getQueryProfileType(); + } } + } } @@ -133,12 +149,26 @@ public class QueryProfileProperties extends Properties { } @Override - public Map<String, Object> listProperties(CompoundName path, Map<String,String> context, + public void clearAll(CompoundName name, Map<String, String> context) { + if (references == null) + references = new ArrayList<>(); + references.add(new Pair<>(name, null)); + + if (values != null) + values.keySet().removeIf(key -> key.hasPrefix(name)); + } + + @Override + public Map<String, Object> listProperties(CompoundName path, Map<String, String> context, com.yahoo.processing.request.Properties substitution) { path = unalias(path, context); if (context == null) context = Collections.emptyMap(); - Map<String, Object> properties = profile.listValues(path, context, substitution); + Map<String, Object> properties = new HashMap<>(); + for (var entry : profile.listValues(path, context, substitution).entrySet()) { + if (references != null && containsNullParentOf(path, references)) continue; + properties.put(entry.getKey(), entry.getValue()); + } properties.putAll(super.listProperties(path, context, substitution)); if (references != null) { @@ -155,8 +185,14 @@ public class QueryProfileProperties extends Properties { pathInReference = path.rest(refEntry.getFirst().size()); prefixToReferenceKeys = CompoundName.empty; } - for (Map.Entry<String, Object> valueEntry : refEntry.getSecond().listValues(pathInReference, context, substitution).entrySet()) { - properties.put(prefixToReferenceKeys.append(new CompoundName(valueEntry.getKey())).toString(), valueEntry.getValue()); + if (refEntry.getSecond() == null) { + if (refEntry.getFirst().hasPrefix(path)) + properties.put(prefixToReferenceKeys.toString(), null); + } + else { + for (Map.Entry<String, Object> valueEntry : refEntry.getSecond().listValues(pathInReference, context, substitution).entrySet()) { + properties.put(prefixToReferenceKeys.append(new CompoundName(valueEntry.getKey())).toString(), valueEntry.getValue()); + } } } @@ -231,6 +267,12 @@ public class QueryProfileProperties extends Properties { return null; } + private boolean containsNullParentOf(CompoundName path, List<Pair<CompoundName, CompiledQueryProfile>> properties) { + if (properties.contains(new Pair<>(path, (CompiledQueryProfile)null))) return true; + if (path.size() > 0 && containsNullParentOf(path.first(path.size() - 1), properties)) return true; + return false; + } + CompoundName unalias(CompoundName name, Map<String,String> context) { if (profile.getTypes().isEmpty()) return name; diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/compiled/Binding.java b/container-search/src/main/java/com/yahoo/search/query/profile/compiled/Binding.java index 2774bd4ebf2..88014eef46d 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/compiled/Binding.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/compiled/Binding.java @@ -106,7 +106,7 @@ public class Binding implements Comparable<Binding> { * Returns true if all the dimension values in this have the same values * in the given context. */ - public boolean matches(Map<String,String> context) { + public boolean matches(Map<String, String> context) { for (int i = 0; i < dimensions.length; i++) { if ( ! dimensionValues[i].equals(context.get(dimensions[i]))) return false; } diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/compiled/CompiledQueryProfile.java b/container-search/src/main/java/com/yahoo/search/query/profile/compiled/CompiledQueryProfile.java index 644d366e7d0..1c7a7cf3e97 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/compiled/CompiledQueryProfile.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/compiled/CompiledQueryProfile.java @@ -102,7 +102,7 @@ public class CompiledQueryProfile extends AbstractComponent implements Cloneable * For example, if {a.d => "a.d-value" ,a.e => "a.e-value", b.d => "b.d-value", then calling listValues("a") * will return {"d" => "a.d-value","e" => "a.e-value"} */ - public final Map<String, Object> listValues(CompoundName prefix) { return listValues(prefix, Collections.<String,String>emptyMap()); } + public final Map<String, Object> listValues(CompoundName prefix) { return listValues(prefix, Collections.emptyMap()); } public final Map<String, Object> listValues(String prefix) { return listValues(new CompoundName(prefix)); } /** @@ -134,7 +134,6 @@ public class CompiledQueryProfile extends AbstractComponent implements Cloneable public Map<String, Object> listValues(CompoundName prefix, Map<String, String> context, Properties substitution) { Map<String, Object> values = new HashMap<>(); for (Map.Entry<CompoundName, DimensionalValue<ValueWithSource>> entry : entries.entrySet()) { - if ( entry.getKey().size() <= prefix.size()) continue; if ( ! entry.getKey().hasPrefix(prefix)) continue; ValueWithSource valueWithSource = entry.getValue().get(context); diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/compiled/DimensionalValue.java b/container-search/src/main/java/com/yahoo/search/query/profile/compiled/DimensionalValue.java index b5481059ac0..0137b848dac 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/compiled/DimensionalValue.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/compiled/DimensionalValue.java @@ -96,7 +96,7 @@ public class DimensionalValue<VALUE> { private VALUE value = null; /** The minimal binding this holds for */ - private Binding binding = null; + private Binding binding; public Value(VALUE value, Binding binding) { this.value = value; diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/config/QueryProfileXMLReader.java b/container-search/src/main/java/com/yahoo/search/query/profile/config/QueryProfileXMLReader.java index 33f07a58195..1b1cdce5890 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/config/QueryProfileXMLReader.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/config/QueryProfileXMLReader.java @@ -33,7 +33,7 @@ public class QueryProfileXMLReader { * Reads all query profile xml files in a given directory, * and all type xml files from the immediate subdirectory "types/" (if any) * - * @throws RuntimeException if <code>directory</code> is not a readable directory, or if there is some error in the XML + * @throws IllegalArgumentException if the directory is not readable, or if there is some error in the XML */ public QueryProfileRegistry read(String directory) { List<NamedReader> queryProfileReaders = new ArrayList<>(); @@ -58,7 +58,7 @@ public class QueryProfileXMLReader { return read(queryProfileTypeReaders,queryProfileReaders); } catch (IOException e) { - throw new IllegalArgumentException("Could not read query profiles from '" + directory + "'",e); + throw new IllegalArgumentException("Could not read query profiles from '" + directory + "'", e); } finally { closeAll(queryProfileReaders); @@ -105,14 +105,14 @@ public class QueryProfileXMLReader { "' must be 'query-profile-type', not '" + root.getNodeName() + "'"); } - String idString=root.getAttribute("id"); + String idString = root.getAttribute("id"); if (idString == null || idString.equals("")) throw new IllegalArgumentException("'" + reader.getName() + "' has no 'id' attribute in the root element"); ComponentId id = new ComponentId(idString); - validateFileNameToId(reader.getName(),id,"query profile type"); + validateFileNameToId(reader.getName(), id,"query profile type"); QueryProfileType type = new QueryProfileType(id); - type.setMatchAsPath(XML.getChild(root,"match") != null); - type.setStrict(XML.getChild(root,"strict") != null); + type.setMatchAsPath(XML.getChild(root, "match") != null); + type.setStrict(XML.getChild(root, "strict") != null); registry.register(type); queryProfileTypeElements.add(root); } @@ -145,7 +145,7 @@ public class QueryProfileXMLReader { queryProfile.setType(type); } - Element dimensions = XML.getChild(root,"dimensions"); + Element dimensions = XML.getChild(root, "dimensions"); if (dimensions != null) queryProfile.setDimensions(toArray(XML.getValue(dimensions))); @@ -215,7 +215,7 @@ public class QueryProfileXMLReader { try { String fieldTypeName = field.getAttribute("type"); if (fieldTypeName == null) throw new IllegalArgumentException("Field '" + field + "' has no 'type' attribute"); - FieldType fieldType=FieldType.fromString(fieldTypeName,registry); + FieldType fieldType = FieldType.fromString(fieldTypeName, registry); type.addField(new FieldDescription(name, fieldType, field.getAttribute("alias"), diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/types/FieldDescription.java b/container-search/src/main/java/com/yahoo/search/query/profile/types/FieldDescription.java index b8290fa092b..6c30f1a8b05 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/types/FieldDescription.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/types/FieldDescription.java @@ -97,7 +97,7 @@ public class FieldDescription implements Comparable<FieldDescription> { this.type = type; // Forbidden until we can figure out the right semantics - if (name.isCompound() && ! aliases.isEmpty()) throw new IllegalArgumentException("Aliases is not allowed with compound names"); + if (name.isCompound() && ! aliases.isEmpty()) throw new IllegalArgumentException("Aliases are not allowed with compound names"); this.aliases = ImmutableList.copyOf(aliases); this.mandatory = mandatory; diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/types/QueryProfileType.java b/container-search/src/main/java/com/yahoo/search/query/profile/types/QueryProfileType.java index 07c9e4475ec..c02aada2062 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/types/QueryProfileType.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/types/QueryProfileType.java @@ -6,6 +6,7 @@ import com.google.common.collect.ImmutableMap; import com.yahoo.component.ComponentId; import com.yahoo.component.provider.FreezableSimpleComponent; import com.yahoo.processing.request.CompoundName; +import com.yahoo.search.query.profile.OverridableQueryProfile; import com.yahoo.search.query.profile.QueryProfile; import java.util.ArrayList; @@ -24,6 +25,7 @@ import static com.yahoo.text.Lowercase.toLowerCase; public class QueryProfileType extends FreezableSimpleComponent { private final CompoundName componentIdAsCompoundName; + /** The fields of this query profile type */ private Map<String, FieldDescription> fields; @@ -217,25 +219,38 @@ public class QueryProfileType extends FreezableSimpleComponent { /** Returns the type of the given query profile type declared as a field in this */ public QueryProfileType getType(String localName) { - FieldDescription fieldDescription=getField(localName); - if (fieldDescription ==null) return null; + FieldDescription fieldDescription = getField(localName); + if (fieldDescription == null) return null; if ( ! (fieldDescription.getType() instanceof QueryProfileFieldType)) return null; return ((QueryProfileFieldType) fieldDescription.getType()).getQueryProfileType(); } + /** Returns the field type of the given name under this, of null if none */ + public FieldType getFieldType(CompoundName name) { + FieldDescription field = getField(name.first()); + if (field == null) return null; + + FieldType fieldType = field.getType(); + if (name.size() == 1) return fieldType; + + if ( ! (fieldType instanceof QueryProfileFieldType)) return null; + + return ((QueryProfileFieldType)fieldType).getQueryProfileType().getFieldType(name.rest()); + } + /** * Returns the description of the field with the given name in this type or an inherited type * (depth first left to right search). Returns null if the field is not defined in this or an inherited profile. */ public FieldDescription getField(String name) { - FieldDescription field=fields.get(name); - if ( field!=null ) return field; + FieldDescription field = fields.get(name); + if ( field != null ) return field; if ( isFrozen() ) return null; // Inherited are collapsed into this for (QueryProfileType inheritedType : this.inherited() ) { - field=inheritedType.getField(name); - if (field!=null) return field; + field = inheritedType.getField(name); + if (field != null) return field; } return null; @@ -276,7 +291,7 @@ public class QueryProfileType extends FreezableSimpleComponent { // Add (/to) a query profile type containing the rest of the name. // (we do not need the field description settings for intermediate query profile types // as the leaf entry will enforce them) - QueryProfileType type = getOrCreateQueryProfileType(name.first(), registry); + QueryProfileType type = extendOrCreateQueryProfileType(name.first(), registry); type.addField(fieldDescription.withName(name.rest()), registry); } else { @@ -288,27 +303,42 @@ public class QueryProfileType extends FreezableSimpleComponent { addAlias(alias, fieldDescription.getName()); } - private QueryProfileType getOrCreateQueryProfileType(String name, QueryProfileTypeRegistry registry) { + private QueryProfileType extendOrCreateQueryProfileType(String name, QueryProfileTypeRegistry registry) { + QueryProfileType type = null; FieldDescription fieldDescription = getField(name); if (fieldDescription != null) { - if ( ! ( fieldDescription.getType() instanceof QueryProfileFieldType)) + if ( ! (fieldDescription.getType() instanceof QueryProfileFieldType)) throw new IllegalArgumentException("Cannot use name '" + name + "' as a prefix because it is " + "already a " + fieldDescription.getType()); QueryProfileFieldType fieldType = (QueryProfileFieldType) fieldDescription.getType(); - QueryProfileType type = fieldType.getQueryProfileType(); - if (type == null) { // an as-yet untyped reference; add type - type = new QueryProfileType(name); - registry.register(type.getId(), type); - fields.put(name, fieldDescription.withType(new QueryProfileFieldType(type))); - } - return type; + type = fieldType.getQueryProfileType(); + } + + if (type == null) { + type = registry.getComponent(name); + } + + // found in registry but not already added in *this* type (getField also checks parents): extend it + if (type != null && ! fields.containsKey(name)) { + type = new QueryProfileType(ComponentId.createAnonymousComponentId(type.getIdString()), + new HashMap<>(), + List.of(type)); + } + + if (type == null) { // create it + type = new QueryProfileType(ComponentId.createAnonymousComponentId(name)); + } + + if (fieldDescription == null) { + fieldDescription = new FieldDescription(name, new QueryProfileFieldType(type)); } else { - QueryProfileType type = new QueryProfileType(name); - registry.register(type.getId(), type); - fields.put(name, new FieldDescription(name, new QueryProfileFieldType(type))); - return type; + fieldDescription = fieldDescription.withType(new QueryProfileFieldType(type)); } + + registry.register(type); + fields.put(name, fieldDescription); + return type; } private void addAlias(String alias, String field) { @@ -362,6 +392,7 @@ public class QueryProfileType extends FreezableSimpleComponent { return other.getId().equals(this.getId()); } + @Override public String toString() { return "query profile type '" + getId() + "'"; } diff --git a/container-search/src/main/java/com/yahoo/search/query/properties/PropertyAliases.java b/container-search/src/main/java/com/yahoo/search/query/properties/PropertyAliases.java index a4a82d27f8e..83e8dd530ad 100644 --- a/container-search/src/main/java/com/yahoo/search/query/properties/PropertyAliases.java +++ b/container-search/src/main/java/com/yahoo/search/query/properties/PropertyAliases.java @@ -37,6 +37,8 @@ public class PropertyAliases extends Properties { * @return the real name if an alias or the input name itself */ protected CompoundName unalias(CompoundName nameOrAlias) { + if (aliases.isEmpty()) return nameOrAlias; + if (nameOrAlias.size() > 1) return nameOrAlias; // aliases are simple names CompoundName properName = aliases.get(nameOrAlias.getLowerCasedName()); return (properName != null) ? properName : nameOrAlias; } diff --git a/container-search/src/main/java/com/yahoo/search/query/properties/PropertyMap.java b/container-search/src/main/java/com/yahoo/search/query/properties/PropertyMap.java index 30fc98ac6b1..643e215daef 100644 --- a/container-search/src/main/java/com/yahoo/search/query/properties/PropertyMap.java +++ b/container-search/src/main/java/com/yahoo/search/query/properties/PropertyMap.java @@ -26,7 +26,10 @@ public class PropertyMap extends Properties { /** The properties of this */ private Map<CompoundName, Object> properties = new LinkedHashMap<>(); - public void set(CompoundName name, Object value, Map<String,String> context) { + public void set(CompoundName name, Object value, Map<String, String> context) { + if (value == null) // Both clear and forward + properties.remove(name); + if (shouldSet(name, value)) properties.put(name, value); else @@ -37,7 +40,7 @@ public class PropertyMap extends Properties { * Return true if this value should be set in this map, false if the set should be propagated instead * This default implementation always returns true. */ - protected boolean shouldSet(CompoundName name,Object value) { return true; } + protected boolean shouldSet(CompoundName name, Object value) { return true; } @Override public Object get(CompoundName name, Map<String,String> context, diff --git a/container-search/src/main/java/com/yahoo/search/query/properties/QueryProperties.java b/container-search/src/main/java/com/yahoo/search/query/properties/QueryProperties.java index a4c150b606e..96f73e925af 100644 --- a/container-search/src/main/java/com/yahoo/search/query/properties/QueryProperties.java +++ b/container-search/src/main/java/com/yahoo/search/query/properties/QueryProperties.java @@ -44,7 +44,7 @@ public class QueryProperties extends Properties { @Override public Object get(CompoundName key, - Map<String,String> context, + Map<String, String> context, com.yahoo.processing.request.Properties substitution) { if (key.size() == 2 && key.first().equals(Model.MODEL)) { Model model = query.getModel(); @@ -294,7 +294,7 @@ public class QueryProperties extends Properties { super.set(key,value,context); } catch (Exception e) { // Make sure error messages are informative. This should be moved out of this properties implementation - if (e.getMessage().startsWith("Could not set")) + if (e.getMessage() != null && e.getMessage().startsWith("Could not set")) throw e; else throw new IllegalArgumentException("Could not set '" + key + "' to '" + value + "'", e); diff --git a/container-search/src/main/java/com/yahoo/search/query/properties/RequestContextProperties.java b/container-search/src/main/java/com/yahoo/search/query/properties/RequestContextProperties.java index ee09521fa74..6cf27fc9a3e 100644 --- a/container-search/src/main/java/com/yahoo/search/query/properties/RequestContextProperties.java +++ b/container-search/src/main/java/com/yahoo/search/query/properties/RequestContextProperties.java @@ -7,7 +7,7 @@ import com.yahoo.search.query.Properties; import java.util.Map; /** - * Turns get(name) into get(name,request) using the request given at construction time. + * Turns get(name) into get(name, request) using the request given at construction time. * This is used to allow the query's request to be supplied to all property requests * without forcing users of the query.properties() to supply this explicitly. * @@ -22,18 +22,18 @@ public class RequestContextProperties extends Properties { } @Override - public Object get(CompoundName name,Map<String,String> context, + public Object get(CompoundName name, Map<String,String> context, com.yahoo.processing.request.Properties substitution) { return super.get(name, context == null ? requestMap : context, substitution); } @Override - public void set(CompoundName name,Object value,Map<String,String> context) { + public void set(CompoundName name, Object value, Map<String,String> context) { super.set(name, value, context == null ? requestMap : context); } @Override - public Map<String, Object> listProperties(CompoundName path,Map<String,String> context, + public Map<String, Object> listProperties(CompoundName path, Map<String,String> context, com.yahoo.processing.request.Properties substitution) { return super.listProperties(path, context == null ? requestMap : context, substitution); } diff --git a/container-search/src/main/java/com/yahoo/search/querytransform/VespaLowercasingSearcher.java b/container-search/src/main/java/com/yahoo/search/querytransform/VespaLowercasingSearcher.java index 1e8f436a05a..25488aa7bbc 100644 --- a/container-search/src/main/java/com/yahoo/search/querytransform/VespaLowercasingSearcher.java +++ b/container-search/src/main/java/com/yahoo/search/querytransform/VespaLowercasingSearcher.java @@ -44,4 +44,5 @@ public class VespaLowercasingSearcher extends LowercasingSearcher { Index index = indexFacts.getIndex(sb.toString()); return index.isLowercase() || index.isAttribute(); } + } diff --git a/container-search/src/main/java/com/yahoo/search/rendering/JsonRenderer.java b/container-search/src/main/java/com/yahoo/search/rendering/JsonRenderer.java index af453983f89..31f8194b3b7 100644 --- a/container-search/src/main/java/com/yahoo/search/rendering/JsonRenderer.java +++ b/container-search/src/main/java/com/yahoo/search/rendering/JsonRenderer.java @@ -7,6 +7,7 @@ import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.core.TreeNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.base.Preconditions; +import com.yahoo.container.logging.TraceRenderer; import com.yahoo.data.JsonProducer; import com.yahoo.data.access.Inspectable; import com.yahoo.data.access.Inspector; @@ -44,8 +45,6 @@ import com.yahoo.search.result.Hit; import com.yahoo.search.result.HitGroup; import com.yahoo.search.result.NanNumber; import com.yahoo.tensor.Tensor; -import com.yahoo.yolean.trace.TraceNode; -import com.yahoo.yolean.trace.TraceVisitor; import org.json.JSONArray; import org.json.JSONObject; @@ -111,10 +110,6 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> { private static final String ROOT = "root"; private static final String SOURCE = "source"; private static final String TOTAL_COUNT = "totalCount"; - private static final String TRACE = "trace"; - private static final String TRACE_CHILDREN = "children"; - private static final String TRACE_MESSAGE = "message"; - private static final String TRACE_TIMESTAMP = "timestamp"; private static final String TIMING = "timing"; private static final String QUERY_TIME = "querytime"; private static final String SUMMARY_FETCH_TIME = "summaryfetchtime"; @@ -132,145 +127,6 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> { private LongSupplier timeSource; private OutputStream stream; - private class TraceRenderer extends TraceVisitor { - private final long basetime; - private boolean hasFieldName = false; - int emittedChildNesting = 0; - int currentChildNesting = 0; - private boolean insideOpenObject = false; - - TraceRenderer(long basetime) { - this.basetime = basetime; - } - - @Override - public void entering(TraceNode node) { - ++currentChildNesting; - } - - @Override - public void leaving(TraceNode node) { - conditionalEndObject(); - if (currentChildNesting == emittedChildNesting) { - try { - generator.writeEndArray(); - generator.writeEndObject(); - } catch (IOException e) { - throw new TraceRenderWrapper(e); - } - --emittedChildNesting; - } - --currentChildNesting; - } - - @Override - public void visit(TraceNode node) { - try { - doVisit(node.timestamp(), node.payload(), node.children().iterator().hasNext()); - } catch (IOException e) { - throw new TraceRenderWrapper(e); - } - } - - private void doVisit(long timestamp, Object payload, boolean hasChildren) throws IOException { - boolean dirty = false; - if (timestamp != 0L) { - header(); - generator.writeStartObject(); - generator.writeNumberField(TRACE_TIMESTAMP, timestamp - basetime); - dirty = true; - } - if (payload != null) { - if (!dirty) { - header(); - generator.writeStartObject(); - } - generator.writeFieldName(TRACE_MESSAGE); - fieldConsumer.renderFieldContentsDirect(payload); - dirty = true; - } - if (dirty) { - if (!hasChildren) { - generator.writeEndObject(); - } else { - setInsideOpenObject(true); - } - } - } - - private void header() { - fieldName(); - for (int i = 0; i < (currentChildNesting - emittedChildNesting); ++i) { - startChildArray(); - } - emittedChildNesting = currentChildNesting; - } - - private void startChildArray() { - try { - conditionalStartObject(); - generator.writeArrayFieldStart(TRACE_CHILDREN); - } catch (IOException e) { - throw new TraceRenderWrapper(e); - } - } - - private void conditionalStartObject() throws IOException { - if (!isInsideOpenObject()) { - generator.writeStartObject(); - } else { - setInsideOpenObject(false); - } - } - - private void conditionalEndObject() { - if (isInsideOpenObject()) { - // This triggers if we were inside a data node with payload and - // subnodes, but none of the subnodes contained data - try { - generator.writeEndObject(); - setInsideOpenObject(false); - } catch (IOException e) { - throw new TraceRenderWrapper(e); - } - } - } - - private void fieldName() { - if (hasFieldName) { - return; - } - - try { - generator.writeFieldName(TRACE); - } catch (IOException e) { - throw new TraceRenderWrapper(e); - } - hasFieldName = true; - } - - boolean isInsideOpenObject() { - return insideOpenObject; - } - - void setInsideOpenObject(boolean insideOpenObject) { - this.insideOpenObject = insideOpenObject; - } - } - - private static final class TraceRenderWrapper extends RuntimeException { - - /** - * Should never be serialized, but this is still needed. - */ - private static final long serialVersionUID = 2L; - - TraceRenderWrapper(IOException wrapped) { - super(wrapped); - } - - } - public JsonRenderer() { this(null); } @@ -352,8 +208,8 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> { long basetime = trace.traceNode().timestamp(); if (basetime == 0L) basetime = getResult().getElapsedTime().first(); - trace.accept(new TraceRenderer(basetime)); - } catch (TraceRenderWrapper e) { + trace.accept(new TraceRenderer(generator, fieldConsumer, basetime)); + } catch (TraceRenderer.TraceRenderWrapper e) { throw new IOException(e); } } @@ -641,11 +497,9 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> { private String getJsonCallback() { Result result = getResult(); - if (result != null) { - Query query = result.getQuery(); - if (query != null) { - return query.properties().getString(JSON_CALLBACK, null); - } + Query query = result.getQuery(); + if (query != null) { + return query.properties().getString(JSON_CALLBACK, null); } return null; } @@ -671,7 +525,7 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> { * This instance is reused for all hits of a Result since we are in a single-threaded context * and want to limit object creation. */ - public static class FieldConsumer implements Hit.RawUtf8Consumer { + public static class FieldConsumer implements Hit.RawUtf8Consumer, TraceRenderer.FieldConsumer { private final JsonGenerator generator; private final boolean debugRendering; @@ -788,11 +642,12 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> { if (field instanceof Inspectable && ! (field instanceof FeatureData)) { renderInspector(((Inspectable)field).inspect()); } else { - renderFieldContentsDirect(field); + accept(field); } } - private void renderFieldContentsDirect(Object field) throws IOException { + @Override + public void accept(Object field) throws IOException { if (field == null) { generator.writeNull(); } else if (field instanceof Boolean) { diff --git a/container-search/src/main/java/com/yahoo/search/result/Hit.java b/container-search/src/main/java/com/yahoo/search/result/Hit.java index fc416c0d930..c14b3f39bc1 100644 --- a/container-search/src/main/java/com/yahoo/search/result/Hit.java +++ b/container-search/src/main/java/com/yahoo/search/result/Hit.java @@ -473,7 +473,7 @@ public class Hit extends ListenableFreezableClass implements Data, Comparable<Hi private Map<String, Object> getFieldMap(int minSize) { if (fields == null) { // Compensate for loadfactor and then some, rounded up.... - fields = new LinkedHashMap<>(2*minSize); + fields = new LinkedHashMap<>(2 * minSize); } return fields; } @@ -505,7 +505,7 @@ public class Hit extends ListenableFreezableClass implements Data, Comparable<Hi } /** Returns the types of this as a modifiable set. Modifications to this set are directly reflected in this hit */ - //TODO This shoudld not be exposed as a modifiable set + // TODO: This should not be exposed as a modifiable set public Set<String> types() { if (types == null) types = new ArraySet<>(1); diff --git a/container-search/src/main/java/com/yahoo/search/searchchain/Execution.java b/container-search/src/main/java/com/yahoo/search/searchchain/Execution.java index 5cc34ff5b28..84fe88d0292 100644 --- a/container-search/src/main/java/com/yahoo/search/searchchain/Execution.java +++ b/container-search/src/main/java/com/yahoo/search/searchchain/Execution.java @@ -3,7 +3,6 @@ package com.yahoo.search.searchchain; import com.yahoo.component.chain.Chain; import com.yahoo.language.Linguistics; -import com.yahoo.log.LogLevel; import com.yahoo.prelude.IndexFacts; import com.yahoo.prelude.Ping; import com.yahoo.prelude.Pong; @@ -11,7 +10,6 @@ import com.yahoo.prelude.query.parser.SpecialTokenRegistry; import com.yahoo.processing.Processor; import com.yahoo.processing.Request; import com.yahoo.processing.Response; -import com.yahoo.protect.Validator; import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.Searcher; diff --git a/container-search/src/main/java/com/yahoo/search/searchchain/testutil/DocumentSourceSearcher.java b/container-search/src/main/java/com/yahoo/search/searchchain/testutil/DocumentSourceSearcher.java index d39a488626b..e346a766738 100644 --- a/container-search/src/main/java/com/yahoo/search/searchchain/testutil/DocumentSourceSearcher.java +++ b/container-search/src/main/java/com/yahoo/search/searchchain/testutil/DocumentSourceSearcher.java @@ -97,11 +97,10 @@ public class DocumentSourceSearcher extends Searcher { public Result search(Query query, Execution execution) { queryCount++; Result r = unFilledResults.get(getQueryKeyClone(query)); - if (r == null) { + if (r == null) r = defaultFilledResult.clone(); - } else { + else r = r.clone(); - } r.setQuery(query); r.hits().trim(query.getOffset(), query.getHits()); @@ -182,11 +181,8 @@ public class DocumentSourceSearcher extends Searcher { * reset. For testing - not reliable if multiple threads makes * queries simultaneously */ - public int getQueryCount() { - return queryCount; - } + public int getQueryCount() { return queryCount; } + + public void resetQueryCount() { queryCount = 0; } - public void resetQueryCount() { - queryCount=0; - } } diff --git a/container-search/src/main/java/com/yahoo/search/searchers/ValidateNearestNeighborSearcher.java b/container-search/src/main/java/com/yahoo/search/searchers/ValidateNearestNeighborSearcher.java index 8cae081cada..76b8c1ef8a2 100644 --- a/container-search/src/main/java/com/yahoo/search/searchers/ValidateNearestNeighborSearcher.java +++ b/container-search/src/main/java/com/yahoo/search/searchers/ValidateNearestNeighborSearcher.java @@ -4,37 +4,33 @@ package com.yahoo.search.searchers; import com.google.common.annotations.Beta; -import com.yahoo.container.QrSearchersConfig; import com.yahoo.prelude.query.Item; import com.yahoo.prelude.query.NearestNeighborItem; -import com.yahoo.prelude.query.QueryCanonicalizer; import com.yahoo.prelude.query.ToolBox; import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.Searcher; +import com.yahoo.search.grouping.vespa.GroupingExecutor; import com.yahoo.search.query.ranking.RankProperties; import com.yahoo.search.result.ErrorMessage; import com.yahoo.search.searchchain.Execution; import com.yahoo.tensor.Tensor; import com.yahoo.tensor.TensorType; import com.yahoo.vespa.config.search.AttributesConfig; -import com.yahoo.yolean.chain.After; +import com.yahoo.yolean.chain.Before; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; -// This depends on tensors in query.getRanking which are moved to rank.properties during query.prepare() -// Query.prepare is done at the same time as canonicalization (by GroupingExecutor), so use that constraint. -@After(QueryCanonicalizer.queryCanonicalization) - /** * Validates any NearestNeighborItem query items. * * @author arnej */ @Beta +@Before(GroupingExecutor.COMPONENT_NAME) // Must happen before query.prepare() public class ValidateNearestNeighborSearcher extends Searcher { private Map<String, TensorType> validAttributes = new HashMap<>(); @@ -56,7 +52,7 @@ public class ValidateNearestNeighborSearcher extends Searcher { } private Optional<ErrorMessage> validate(Query query) { - NNVisitor visitor = new NNVisitor(query.getRanking().getProperties(), validAttributes); + NNVisitor visitor = new NNVisitor(query.getRanking().getProperties(), validAttributes, query); ToolBox.visit(visitor, query.getModel().getQueryTree().getRoot()); return visitor.errorMessage; } @@ -65,26 +61,26 @@ public class ValidateNearestNeighborSearcher extends Searcher { public Optional<ErrorMessage> errorMessage = Optional.empty(); - private RankProperties rankProperties; - private Map<String, TensorType> validAttributes; + private final RankProperties rankProperties; + private final Map<String, TensorType> validAttributes; + private final Query query; - public NNVisitor(RankProperties rankProperties, Map<String, TensorType> validAttributes) { + public NNVisitor(RankProperties rankProperties, Map<String, TensorType> validAttributes, Query query) { this.rankProperties = rankProperties; this.validAttributes = validAttributes; + this.query = query; } @Override public boolean visit(Item item) { if (item instanceof NearestNeighborItem) { - validate((NearestNeighborItem) item); + String error = validate((NearestNeighborItem)item); + if (error != null) + errorMessage = Optional.of(ErrorMessage.createIllegalQuery(error)); } return true; } - private void setError(String description) { - errorMessage = Optional.of(ErrorMessage.createIllegalQuery(description)); - } - private static boolean isCompatible(TensorType lhs, TensorType rhs) { return lhs.dimensions().equals(rhs.dimensions()); } @@ -98,50 +94,27 @@ public class ValidateNearestNeighborSearcher extends Searcher { return true; } - private void validate(NearestNeighborItem item) { - int target = item.getTargetNumHits(); - if (target < 1) { - setError(item.toString() + " has invalid targetNumHits"); - return; - } - String qprop = item.getQueryTensorName(); - List<Object> rankPropValList = rankProperties.asMap().get(qprop); - if (rankPropValList == null) { - setError(item.toString() + " query tensor not found"); - return; - } - if (rankPropValList.size() != 1) { - setError(item.toString() + " query tensor does not have a single value"); - return; - } - Object rankPropValue = rankPropValList.get(0); - if (! (rankPropValue instanceof Tensor)) { - setError(item.toString() + " query tensor should be a tensor, was: "+ - (rankPropValue == null ? "null" : rankPropValue.getClass().toString())); - return; - } - Tensor qTensor = (Tensor)rankPropValue; - TensorType qTensorType = qTensor.type(); - - String field = item.getIndexName(); - if (validAttributes.containsKey(field)) { - TensorType fTensorType = validAttributes.get(field); - if (fTensorType == null) { - setError(item.toString() + " field is not a tensor"); - return; - } - if (! isCompatible(fTensorType, qTensorType)) { - setError(item.toString() + " field type "+fTensorType+" does not match query tensor type "+qTensorType); - return; - } - if (! isDenseVector(fTensorType)) { - setError(item.toString() + " tensor type "+fTensorType+" is not a dense vector"); - return; - } - } else { - setError(item.toString() + " field is not an attribute"); - return; - } + /** Returns an error message if this is invalid, or null if it is valid */ + private String validate(NearestNeighborItem item) { + if (item.getTargetNumHits() < 1) + return item + " has invalid targetNumHits " + item.getTargetNumHits() + ": Must be >= 1"; + + String queryFeatureName = "query(" + item.getQueryTensorName() + ")"; + Optional<Tensor> queryTensor = query.getRanking().getFeatures().getTensor(queryFeatureName); + if (queryTensor.isEmpty()) + return item + " requires a tensor rank feature " + queryFeatureName + " but this is not present"; + + if ( ! validAttributes.containsKey(item.getIndexName())) + return item + " field is not an attribute"; + TensorType fieldType = validAttributes.get(item.getIndexName()); + if (fieldType == null) return item + " field is not a tensor"; + if ( ! isDenseVector(fieldType)) + return item + " tensor type " + fieldType + " is not a dense vector"; + + if ( ! isCompatible(fieldType, queryTensor.get().type())) + return item + " field type " + fieldType + " does not match query type " + queryTensor.get().type(); + + return null; } @Override diff --git a/container-search/src/main/java/com/yahoo/search/yql/VespaSerializer.java b/container-search/src/main/java/com/yahoo/search/yql/VespaSerializer.java index 6eef1252998..dd52b9e19b8 100644 --- a/container-search/src/main/java/com/yahoo/search/yql/VespaSerializer.java +++ b/container-search/src/main/java/com/yahoo/search/yql/VespaSerializer.java @@ -701,7 +701,18 @@ public class VespaSerializer { destination.append(leafAnnotations(item)); comma(destination, initLen); int targetNumHits = item.getTargetNumHits(); - destination.append("\"targetNumHits\": ").append(targetNumHits); + annotationKey(destination, "targetNumHits").append(targetNumHits); + int explore = item.getHnswExploreAdditionalHits(); + if (explore != 0) { + comma(destination, initLen); + String key = YqlParser.HNSW_EXPLORE_ADDITIONAL_HITS; + annotationKey(destination, key).append(explore); + } + boolean allow_approx = item.getAllowApproximate(); + if (! allow_approx) { + comma(destination, initLen); + annotationKey(destination, "approximate").append(allow_approx); + } destination.append("}]"); destination.append(NEAREST_NEIGHBOR).append('('); destination.append(item.getIndexName()).append(", "); @@ -1347,6 +1358,11 @@ public class VespaSerializer { } } + private static StringBuilder annotationKey(StringBuilder annotation, String val) { + annotation.append("\"").append(val).append("\": "); + return annotation; + } + private static void comma(StringBuilder annotation, int initLen) { if (annotation.length() > initLen) { annotation.append(", "); diff --git a/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java b/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java index 8d013e501e8..f4560806dd2 100644 --- a/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java +++ b/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java @@ -137,6 +137,7 @@ public class YqlParser implements Parser { static final String ACCENT_DROP = "accentDrop"; static final String ALTERNATIVES = "alternatives"; static final String AND_SEGMENTING = "andSegmenting"; + static final String APPROXIMATE = "approximate"; static final String BOUNDS = "bounds"; static final String BOUNDS_LEFT_OPEN = "leftOpen"; static final String BOUNDS_OPEN = "open"; @@ -149,6 +150,7 @@ public class YqlParser implements Parser { static final String EQUIV = "equiv"; static final String FILTER = "filter"; static final String HIT_LIMIT = "hitLimit"; + static final String HNSW_EXPLORE_ADDITIONAL_HITS = "hnsw.exploreAdditionalHits"; static final String IMPLICIT_TRANSFORMS = "implicitTransforms"; static final String LABEL = "label"; static final String NEAR = "near"; @@ -421,6 +423,14 @@ public class YqlParser implements Parser { if (targetNumHits != null) { item.setTargetNumHits(targetNumHits); } + Integer hnswExploreAdditionalHits = getAnnotation(ast, HNSW_EXPLORE_ADDITIONAL_HITS, + Integer.class, null, "number of extra hits to explore for HNSW algorithm"); + if (hnswExploreAdditionalHits != null) { + item.setHnswExploreAdditionalHits(hnswExploreAdditionalHits); + } + Boolean allowApproximate = getAnnotation(ast, APPROXIMATE, + Boolean.class, Boolean.TRUE, "allow approximate nearest neighbor search"); + item.setAllowApproximate(allowApproximate); String label = getAnnotation(ast, LABEL, String.class, null, "item label"); if (label != null) { item.setLabel(label); |