diff options
Diffstat (limited to 'container-search/src')
71 files changed, 879 insertions, 662 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..365ee299ca4 100644 --- a/container-search/src/main/java/com/yahoo/prelude/Index.java +++ b/container-search/src/main/java/com/yahoo/prelude/Index.java @@ -26,6 +26,7 @@ import java.util.Set; public class Index { public static class Attribute { + private boolean tokenizedContent = false; public final String name; @@ -207,20 +208,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; } 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/NearItem.java b/container-search/src/main/java/com/yahoo/prelude/query/NearItem.java index 69131b6c690..56554e14d01 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/NearItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/NearItem.java @@ -28,17 +28,15 @@ public class NearItem extends CompositeItem { /** * Creates a <i>near</i> item with a limit to the distance between the words. * - * @param distance the number of word position which may separate - * the words for this near item to match + * @param distance the maximum position difference between the words which should be counted as a match */ public NearItem(int distance) { setDistance(distance); } public void setDistance(int distance) { - if (distance < 0) { - throw new IllegalArgumentException("Can not use negative distance '" + distance + "'."); - } + if (distance < 0) + throw new IllegalArgumentException("Can not use negative distance " + distance); this.distance = distance; } 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..5e292a06b0f 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 @@ -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,37 @@ abstract class StructuredParser extends AbstractParser { ((PhraseSegmentItem) word).setExplicit(true); } - if (phrase != null) { - phrase.addItem(word); + if (composite != null) { + composite.addItem(word); } 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); } else if (word instanceof PhraseItem) { - phrase = (PhraseItem) word; + composite = (PhraseItem)word; } else { firstWord = word; starAfterFirst = tokens.skipNoIgnore(STAR); @@ -609,29 +612,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)) { 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/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..1e3f11f4f78 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 * 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..2d05168731a 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); } 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..21e5fe3bc7f 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 @@ -15,22 +15,10 @@ public class MonitorConfiguration { private long checkInterval=1000; /** - * The number of times a failed node must respond before getting - * traffic again - */ - private int responseAfterFailLimit=3; - - /** - * The number of ms a node is allowed to stay idle before it is - * pinged - */ - private long idleLimit=3000; - - /** * The number of milliseconds to attempt to complete a request * before giving up */ - private long requestTimeout = 5000; + private final long requestTimeout = 980; /** * The number of milliseconds a node is allowed to fail before we @@ -39,17 +27,6 @@ public class MonitorConfiguration { private long failLimit=5000; /** - * The number of times a node is allowed to fail in one hour - * before it is quarantined for an hour - */ - private int failQuarantineLimit=3; - - /** - * The number of ms to quarantine an unstable node - */ - private long quarantineTime=1000*60*60; - - /** * Sets the interval between each ping of idle or failing nodes * Default is 1000ms */ @@ -68,25 +45,27 @@ public class MonitorConfiguration { /** * 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 */ - public void setResponseAfterFailLimit(int responseAfterFailLimit) { - this.responseAfterFailLimit=responseAfterFailLimit; - } + @Deprecated + public void setResponseAfterFailLimit(int responseAfterFailLimit) { } /** * 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 */ - public void setIdleLimit(int idleLimit) { - this.idleLimit=idleLimit; - } + @Deprecated + public void setIdleLimit(int idleLimit) { } /** * 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 public long getIdleLimit() { - return idleLimit; + return 3000; } /** @@ -112,28 +91,24 @@ 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 + public void setFailQuarantineLimit(int failQuarantineLimit) { } /** * 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 + 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 + + " 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..481f1e1b5a5 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 @@ -19,9 +19,21 @@ 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 + 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/dispatch/Dispatcher.java b/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java index 03b51fbaf70..423de07f8c4 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 @@ -11,12 +11,13 @@ 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; @@ -58,6 +59,7 @@ public class Dispatcher extends AbstractComponent { /** A model of the search cluster this dispatches to */ private final SearchCluster searchCluster; + private final ClusterMonitor clusterMonitor; private final LoadBalancer loadBalancer; @@ -82,49 +84,53 @@ public class Dispatcher extends AbstractComponent { 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.startClusterMonitoring(pingFactory); + searchCluster.addMonitoring(clusterMonitor); + try { + while ( ! searchCluster.hasInformationAboutAllNodes()) { + Thread.sleep(1); + } + } 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(); } /** Returns the search cluster this dispatches to */ @@ -134,8 +140,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(); } 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 05ce6d50493..a2821892358 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 @@ -22,6 +22,7 @@ import java.util.List; * @author bratseth */ class RpcClient implements Client { + private final Supervisor supervisor; public RpcClient(int transportThreads) { @@ -66,7 +67,7 @@ class RpcClient implements Client { @Override public void request(String rpcMethod, CompressionType compression, int uncompressedLength, byte[] compressedPayload, - ResponseReceiver responseReceiver, double timeoutSeconds) { + ResponseReceiver responseReceiver, double timeoutSeconds) { Request request = new Request(rpcMethod); request.parameters().add(new Int8Value(compression.getCode())); request.parameters().add(new Int32Value(uncompressedLength)); 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/Node.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Node.java index 2f70c37cd48..e93b633f09d 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,8 @@ 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); public Node(int key, String hostname, int group) { this.key = key; @@ -28,6 +30,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; } 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..7619cb34b77 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,6 @@ 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.vespa.config.search.DispatchConfig; import java.util.LinkedHashMap; @@ -18,13 +17,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 +36,8 @@ 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 long nextLogTime = 0; /** @@ -58,10 +50,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(); @@ -84,29 +78,25 @@ public class SearchCluster implements NodeManager<Node> { this.nodesByHost = nodesByHostBuilder.build(); this.localCorpusDispatchTarget = findLocalCorpusDispatchTarget(HostName.getLocalhost(), - size, - containerClusterSize, - nodesByHost, - groups); - - this.clusterMonitor = new ClusterMonitor<>(this); + size, + containerClusterSize, + nodesByHost, + groups); } - 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; - + 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, @@ -247,7 +237,7 @@ public class SearchCluster implements NodeManager<Node> { vipStatus.removeFromRotation(clusterId); } - private boolean hasInformationAboutAllNodes() { + public boolean hasInformationAboutAllNodes() { return nodesByHost.values().stream().allMatch(node -> node.isWorking() != null); } @@ -263,26 +253,33 @@ 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 - - FutureTask<Pong> futurePong = new FutureTask<>(pingFactory.createPinger(node, clusterMonitor)); - executor.execute(futurePong); - Pong pong = getPong(futurePong, node); - futurePong.cancel(true); - - if (pong.badResponse()) { - clusterMonitor.failed(node, pong.error().get()); - } else { - if (pong.activeDocuments().isPresent()) { - node.setActiveDocuments(pong.activeDocuments().get()); + private static class PongCallback implements PongHandler { + private final ClusterMonitor<Node> clusterMonitor; + private final Node node; + 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()); + } + 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(); @@ -353,20 +350,6 @@ 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 */ @@ -402,6 +385,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/FederationSearcher.java b/container-search/src/main/java/com/yahoo/search/federation/FederationSearcher.java index d6918675e87..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,13 +270,14 @@ public class FederationSearcher extends ForkingSearcher { outgoing.setTimeout(timeout); switch (propagateSourceProperties) { - case ALL: - propagatePerSourceQueryProperties(query, outgoing, window, sourceName, providerName, - Query.nativeProperties); + 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: - propagatePerSourceQueryProperties(query, outgoing, window, sourceName, providerName, - queryAndHits); + propagatePerSourceQueryProperties(query, outgoing, window, sourceName, providerName, queryAndHits); break; } @@ -290,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); + } } } @@ -321,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); } 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..dad106570ab 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 @@ -29,7 +29,7 @@ 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; @@ -58,6 +58,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 +99,8 @@ public class SearchHandler extends LoggingRequestHandler { private final ExecutionFactory executionFactory; + private final AtomicLong numRequestsLeftToTrace; + private final class MeanConnections implements Callback { @Override @@ -125,6 +128,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 +140,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 +165,7 @@ public class SearchHandler extends LoggingRequestHandler { .setCallback(new MeanConnections())); this.hostResponseHeaderKey = hostResponseHeaderKey; + this.numRequestsLeftToTrace = new AtomicLong(numQueriesToTraceOnDebugAfterStartup); } /** @deprecated use the other constructor */ @@ -215,7 +231,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 +296,10 @@ 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, + log.isLoggable(Level.FINE) + ? query.getContext(false).getTrace().traceNode() + : null); if (hostResponseHeaderKey.isPresent()) response.headers().add(hostResponseHeaderKey.get(), selfHostname); @@ -330,7 +348,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 +413,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 +425,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/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/SelectParser.java b/container-search/src/main/java/com/yahoo/search/query/SelectParser.java index c654edda6f5..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,7 +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.vespa.config.SlimeUtils; +import com.yahoo.slime.SlimeUtils; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; @@ -159,7 +159,7 @@ public class SelectParser implements Parser { return new QueryTree(root); } - private Item walkJson(Inspector inspector){ + private Item walkJson(Inspector inspector) { Item[] item = {null}; inspector.traverse((ObjectTraverser) (key, value) -> { String type = (FUNCTION_CALLS.contains(key)) ? CALL : key; @@ -197,8 +197,8 @@ public class SelectParser implements Parser { public List<VespaGroupingStep> getGroupingSteps(String grouping){ List<VespaGroupingStep> groupingSteps = new ArrayList<>(); - List<String> groupingOperations = getOperations(grouping); - for (String groupingString : groupingOperations){ + List<String> groupingOperations = toGroupingRequests(grouping); + for (String groupingString : groupingOperations) { GroupingOperation groupingOperation = GroupingOperation.fromString(groupingString); VespaGroupingStep groupingStep = new VespaGroupingStep(groupingOperation); groupingSteps.add(groupingStep); @@ -206,24 +206,51 @@ public class SelectParser implements Parser { return groupingSteps; } - private List<String> getOperations(String grouping) { - List<String> operations = new ArrayList<>(); - Inspector inspector = SlimeUtils.jsonToSlime(grouping.getBytes()).get(); - if (inspector.field("error_message").valid()){ + /** Translates a list of grouping requests on JSON form to a list in the grouping language form */ + private List<String> toGroupingRequests(String groupingJson) { + Inspector inspector = SlimeUtils.jsonToSlime(groupingJson.getBytes()).get(); + if (inspector.field("error_message").valid()) { throw new QueryException("Illegal query: " + inspector.field("error_message").asString() + " at: '" + new String(inspector.field("offending_input").asData(), StandardCharsets.UTF_8) + "'"); } - inspector.traverse( (ArrayTraverser) (key, value) -> { - String groupingString = value.toString(); - groupingString = groupingString.replace(" ", "").replace("\"", "").replace("\'", "").replace(":{", "(").replace(":", "(").replace("}", ")").replace(",", ")"); - groupingString = groupingString.substring(1, groupingString.length()); - operations.add(groupingString); - }); - + List<String> operations = new ArrayList<>(); + inspector.traverse((ArrayTraverser) (__, item) -> operations.add(toGroupingRequest(item))); return operations; } + private String toGroupingRequest(Inspector groupingJson) { + StringBuilder b = new StringBuilder(); + toGroupingRequest(groupingJson, b); + return b.toString(); + } + + private void toGroupingRequest(Inspector groupingJson, StringBuilder b) { + switch (groupingJson.type()) { + case ARRAY: + groupingJson.traverse((ArrayTraverser) (index, item) -> { + toGroupingRequest(item, b); + if (index + 1 < groupingJson.entries()) + b.append(","); + }); + break; + case OBJECT: + groupingJson.traverse((ObjectTraverser) (name, object) -> { + b.append(name); + b.append("("); + toGroupingRequest(object, b); + b.append(") "); + }); + break; + case STRING: + b.append(groupingJson.asString()); + break; + default: + b.append(groupingJson.toString()); + break; + } + } + private Item buildFunctionCall(String key, Inspector value) { switch (key) { case WAND: @@ -250,7 +277,7 @@ public class SelectParser implements Parser { }); } else if (inspector.type() == OBJECT){ - if (inspector.field("children").valid()){ + if (inspector.field("children").valid()) { inspector.field("children").traverse((ArrayTraverser) (index, new_value) -> { item.addItem(walkJson(new_value)); }); @@ -259,22 +286,22 @@ public class SelectParser implements Parser { } } - private Inspector getChildren(Inspector inspector){ + private Inspector getChildren(Inspector inspector) { if (inspector.type() == ARRAY){ return inspector; } else if (inspector.type() == OBJECT){ - if (inspector.field("children").valid()){ + if (inspector.field("children").valid()) { return inspector.field("children"); } - if (inspector.field(1).valid()){ + if (inspector.field(1).valid()) { return inspector.field(1); } } return null; } - private HashMap<Integer, Inspector> childMap(Inspector inspector){ + private HashMap<Integer, Inspector> childMap(Inspector inspector) { HashMap<Integer, Inspector> children = new HashMap<>(); if (inspector.type() == ARRAY){ inspector.traverse((ArrayTraverser) (index, new_value) -> { @@ -291,14 +318,14 @@ public class SelectParser implements Parser { return children; } - private Inspector getAnnotations(Inspector inspector){ + private Inspector getAnnotations(Inspector inspector) { if (inspector.type() == OBJECT && inspector.field("attributes").valid()){ return inspector.field("attributes"); } return null; } - private HashMap<String, Inspector> getAnnotationMapFromAnnotationInspector(Inspector annotation){ + private HashMap<String, Inspector> getAnnotationMapFromAnnotationInspector(Inspector annotation) { HashMap<String, Inspector> attributes = new HashMap<>(); if (annotation.type() == OBJECT){ annotation.traverse((ObjectTraverser) (index, new_value) -> { @@ -308,7 +335,7 @@ public class SelectParser implements Parser { return attributes; } - private HashMap<String, Inspector> getAnnotationMap(Inspector inspector){ + private HashMap<String, Inspector> getAnnotationMap(Inspector inspector) { HashMap<String, Inspector> attributes = new HashMap<>(); if (inspector.type() == OBJECT && inspector.field("attributes").valid()){ inspector.field("attributes").traverse((ObjectTraverser) (index, new_value) -> { 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/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..7ae18f96d86 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; } 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 ea79b10d779..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(); + } } + } } @@ -128,17 +144,31 @@ public class QueryProfileProperties extends Properties { } } catch (IllegalArgumentException e) { - throw new IllegalArgumentException("Could not set '" + name + "' to '" + value + "': " + e.getMessage()); // TODO: Nest instead + throw new IllegalArgumentException("Could not set '" + name + "' to '" + value + "'", e); } } @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/CompiledQueryProfile.java b/container-search/src/main/java/com/yahoo/search/query/profile/compiled/CompiledQueryProfile.java index 644d366e7d0..94f4e4747a0 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 @@ -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/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/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/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/resources/configdefinitions/strict-contracts.def b/container-search/src/main/resources/configdefinitions/strict-contracts.def index f9dd788c054..5ceb37db8d1 100644 --- a/container-search/src/main/resources/configdefinitions/strict-contracts.def +++ b/container-search/src/main/resources/configdefinitions/strict-contracts.def @@ -1,6 +1,7 @@ # Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. namespace=search.federation +## DEPRECATED: This config will be removed on Vespa 8 ## A config to control whether to activate strict adherence to public contracts ## in the container. Usually, the container tries to do a best effort of hiding ## some undesirable effects of the the public contracts. Modifying this config @@ -11,11 +12,9 @@ namespace=search.federation ## can be construed to be unnecessary. searchchains bool default=false -# WARNING: Beta feature, might be removed soon. -# Propagate source.(sourceName).{QueryProperties.PER_SOURCE_QUERY_PROPERTIES} and -# provider.(providerName).{QueryProperties.PER_SOURCE_QUERY_PROPERTIES} -# to the outgoing query. -# All means all in QueryProperties.PER_SOURCE_QUERY_PROPERTIES -# OFFSET_HITS means {Query.HITS, Query.OFFSET} -# NONE means {} -propagateSourceProperties enum {ALL, OFFSET_HITS, NONE} default=ALL +# EVERY, // Propagate any property starting by source.[sourceName] and provider.[providerName] +# NATIVE, // Propagate native properties only +# ALL, // Deprecated synonym of NATIVE +# OFFSET_HITS, // Propagate offset ands hits only +# NONE // propagate no properties +propagateSourceProperties enum {EVERY, NATIVE, ALL, OFFSET_HITS, NONE} default=EVERY diff --git a/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java b/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java index 5b6a4b68930..8b7a57c38e7 100644 --- a/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java @@ -22,6 +22,7 @@ import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.config.ClusterConfig; import com.yahoo.search.dispatch.Dispatcher; +import com.yahoo.search.dispatch.rpc.RpcResourcePool; import com.yahoo.search.result.Hit; import com.yahoo.search.searchchain.Execution; import com.yahoo.vespa.config.search.DispatchConfig; @@ -514,8 +515,10 @@ public class ClusterSearcherTestCase { DocumentdbInfoConfig.Builder documentDbConfig = new DocumentdbInfoConfig.Builder(); documentDbConfig.documentdb(new DocumentdbInfoConfig.Documentdb.Builder().name("type1")); - Dispatcher dispatcher = new Dispatcher(new ComponentId("test-id"), - new DispatchConfig.Builder().build(), + DispatchConfig dispatchConfig = new DispatchConfig.Builder().build(); + Dispatcher dispatcher = new Dispatcher(new RpcResourcePool(dispatchConfig), + ComponentId.createAnonymousComponentId("test-id"), + dispatchConfig, createClusterInfoConfig(), vipStatus, new MockMetric()); diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java index b5e54b46e5a..63475c9c189 100644 --- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java @@ -14,7 +14,6 @@ import com.yahoo.prelude.fastsearch.SummaryParameters; import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.Searcher; -import com.yahoo.search.dispatch.Dispatcher; import com.yahoo.search.dispatch.rpc.RpcResourcePool; import com.yahoo.search.dispatch.searchcluster.Node; import com.yahoo.search.grouping.GroupingRequest; @@ -151,7 +150,8 @@ public class FastSearcherTestCase { VipStatus vipStatus = new VipStatus(b.build()); List<Node> nodes_1 = ImmutableList.of(new Node(0, "host0", 0)); RpcResourcePool rpcPool_1 = new RpcResourcePool(MockDispatcher.toDispatchConfig(nodes_1)); - Dispatcher dispatch_1 = MockDispatcher.create(nodes_1, rpcPool_1, 1, vipStatus); + MockDispatcher dispatch_1 = MockDispatcher.create(nodes_1, rpcPool_1, vipStatus); + dispatch_1.clusterMonitor.shutdown(); vipStatus.addToRotation(clusterName); assertTrue(vipStatus.isInRotation()); dispatch_1.deconstruct(); diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/MockDispatcher.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/MockDispatcher.java index 440e3b8d78f..2a2c8410b2c 100644 --- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/MockDispatcher.java +++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/MockDispatcher.java @@ -2,8 +2,10 @@ package com.yahoo.prelude.fastsearch.test; import com.yahoo.container.handler.VipStatus; +import com.yahoo.search.cluster.ClusterMonitor; import com.yahoo.search.dispatch.Dispatcher; 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.Node; import com.yahoo.search.dispatch.searchcluster.SearchCluster; @@ -13,25 +15,27 @@ import java.util.List; class MockDispatcher extends Dispatcher { + public final ClusterMonitor clusterMonitor; + public static MockDispatcher create(List<Node> nodes) { var rpcResourcePool = new RpcResourcePool(toDispatchConfig(nodes)); - return create(nodes, rpcResourcePool, 1, new VipStatus()); + return create(nodes, rpcResourcePool, new VipStatus()); } - public static MockDispatcher create(List<Node> nodes, RpcResourcePool rpcResourcePool, - int containerClusterSize, VipStatus vipStatus) { + public static MockDispatcher create(List<Node> nodes, RpcResourcePool rpcResourcePool, VipStatus vipStatus) { var dispatchConfig = toDispatchConfig(nodes); - var searchCluster = new SearchCluster("a", dispatchConfig, containerClusterSize, vipStatus); - return new MockDispatcher(searchCluster, dispatchConfig, rpcResourcePool); + var searchCluster = new SearchCluster("a", dispatchConfig, vipStatus, new RpcPingFactory(rpcResourcePool)); + return new MockDispatcher(new ClusterMonitor<>(searchCluster, true), searchCluster, dispatchConfig, rpcResourcePool); } - private MockDispatcher(SearchCluster searchCluster, DispatchConfig dispatchConfig, RpcResourcePool rpcResourcePool) { - this(searchCluster, dispatchConfig, new RpcInvokerFactory(rpcResourcePool, searchCluster)); + private MockDispatcher(ClusterMonitor clusterMonitor, SearchCluster searchCluster, DispatchConfig dispatchConfig, RpcResourcePool rpcResourcePool) { + this(clusterMonitor, searchCluster, dispatchConfig, new RpcInvokerFactory(rpcResourcePool, searchCluster)); } - private MockDispatcher(SearchCluster searchCluster, DispatchConfig dispatchConfig, RpcInvokerFactory invokerFactory) { - super(searchCluster, dispatchConfig, invokerFactory, new MockMetric()); + private MockDispatcher(ClusterMonitor clusterMonitor, SearchCluster searchCluster, DispatchConfig dispatchConfig, RpcInvokerFactory invokerFactory) { + super(clusterMonitor, searchCluster, dispatchConfig, invokerFactory, new MockMetric()); + this.clusterMonitor = clusterMonitor; } static DispatchConfig toDispatchConfig(List<Node> nodes) { diff --git a/container-search/src/test/java/com/yahoo/prelude/querytransform/test/QueryRewriteTestCase.java b/container-search/src/test/java/com/yahoo/prelude/querytransform/test/QueryRewriteTestCase.java index 11922cf640a..36137abd9b8 100644 --- a/container-search/src/test/java/com/yahoo/prelude/querytransform/test/QueryRewriteTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/querytransform/test/QueryRewriteTestCase.java @@ -1,15 +1,22 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.prelude.querytransform.test; +import com.yahoo.prelude.IndexFacts; import com.yahoo.prelude.query.AndItem; import com.yahoo.prelude.query.NotItem; import com.yahoo.prelude.query.OrItem; +import com.yahoo.prelude.query.QueryCanonicalizer; import com.yahoo.prelude.query.WordItem; import com.yahoo.prelude.querytransform.QueryRewrite; +import com.yahoo.prelude.querytransform.RecallSearcher; import com.yahoo.search.Query; +import com.yahoo.search.Result; +import com.yahoo.search.searchchain.Execution; +import com.yahoo.search.test.QueryTestCase; import org.junit.Test; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; /** @@ -38,9 +45,17 @@ public class QueryRewriteTestCase { assertRewritten(query, "OR sddocname:per foo bar"); ((OrItem)query.getModel().getQueryTree().getRoot()).getItem(2).setRanked(false); // set 'bar' unranked assertRewritten(query, "OR sddocname:per foo"); - assertRewritten("sddocname:per OR foo OR (bar AND fuz)", "per", "OR sddocname:per foo (AND bar fuz)"); + } + @Test + public void testRankContributingTermsAreNotRemovedOnFullRecall() { + Query query = new Query(QueryTestCase.httpEncode("?query=default:term1 OR default:term2 OR default:term3 OR sddocname:per&type=adv&recall=+id:1&restrict=per")); + RecallSearcher searcher = new RecallSearcher(); + Result result = new Execution(searcher, Execution.Context.createContextStub(new IndexFacts())).search(query); + assertNull(result.hits().getError()); + assertNull(QueryCanonicalizer.canonicalize(query)); + assertRewritten(query, "AND (OR default:term1 default:term2 default:term3 sddocname:per) |id:1"); } @Test @@ -88,6 +103,7 @@ public class QueryRewriteTestCase { private static void assertRewritten(Query query, String expectedOptimizedQuery) { QueryRewrite.optimizeByRestrict(query); + QueryRewrite.optimizeAndNot(query); QueryRewrite.collapseSingleComposites(query); assertEquals(expectedOptimizedQuery, query.getModel().getQueryTree().toString()); } diff --git a/container-search/src/test/java/com/yahoo/prelude/searcher/test/BlendingSearcherTestCase.java b/container-search/src/test/java/com/yahoo/prelude/searcher/test/BlendingSearcherTestCase.java index 9da1c184505..0086f1b3571 100644 --- a/container-search/src/test/java/com/yahoo/prelude/searcher/test/BlendingSearcherTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/searcher/test/BlendingSearcherTestCase.java @@ -108,7 +108,7 @@ public class BlendingSearcherTestCase { entry.getValue())); } - StrictContractsConfig contracts = new StrictContractsConfig(new StrictContractsConfig.Builder()); + StrictContractsConfig contracts = new StrictContractsConfig.Builder().build(); FederationSearcher fedSearcher = new FederationSearcher(new FederationConfig(builder), contracts, new ComponentRegistry<>()); @@ -124,7 +124,6 @@ public class BlendingSearcherTestCase { @Test public void testitTwoPhase() { - DocumentSourceSearcher chain1 = new DocumentSourceSearcher(); DocumentSourceSearcher chain2 = new DocumentSourceSearcher(); DocumentSourceSearcher chain3 = new DocumentSourceSearcher(); diff --git a/container-search/src/test/java/com/yahoo/prelude/searcher/test/ValidateSortingSearcherTestCase.java b/container-search/src/test/java/com/yahoo/prelude/searcher/test/ValidateSortingSearcherTestCase.java index 65011ffb562..f4bf957e29a 100644 --- a/container-search/src/test/java/com/yahoo/prelude/searcher/test/ValidateSortingSearcherTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/searcher/test/ValidateSortingSearcherTestCase.java @@ -52,6 +52,7 @@ public class ValidateSortingSearcherTestCase { assertEquals("[ASCENDING:[rank]]", quoteAndTransform("+[rank]")); assertEquals("[ASCENDING:[docid]]", quoteAndTransform("+[docid]")); assertEquals("[ASCENDING:[rank]]", quoteAndTransform("+[relevancy]")); + assertEquals("[ASCENDING:[rank]]", quoteAndTransform("+[relevance]")); } @Test diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/DispatcherTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/DispatcherTest.java index de6bafa267a..5433a28dd6e 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/DispatcherTest.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/DispatcherTest.java @@ -1,22 +1,21 @@ // Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.dispatch; -import com.yahoo.prelude.Pong; import com.yahoo.prelude.fastsearch.VespaBackEndSearcher; import com.yahoo.prelude.fastsearch.test.MockMetric; -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.searchcluster.Node; import com.yahoo.search.dispatch.searchcluster.PingFactory; +import com.yahoo.search.dispatch.searchcluster.Pinger; +import com.yahoo.search.dispatch.searchcluster.PongHandler; import com.yahoo.search.dispatch.searchcluster.SearchCluster; import org.junit.Test; import java.util.List; import java.util.Optional; import java.util.OptionalInt; -import java.util.concurrent.Callable; import static com.yahoo.search.dispatch.MockSearchCluster.createDispatchConfig; import static org.junit.Assert.assertEquals; @@ -39,9 +38,10 @@ public class DispatcherTest { assertEquals(2, nodes.get(0).key()); return true; }); - Dispatcher disp = new Dispatcher(cl, createDispatchConfig(), invokerFactory, invokerFactory, new MockMetric()); + Dispatcher disp = new Dispatcher(new ClusterMonitor(cl, false), cl, createDispatchConfig(), invokerFactory, new MockMetric()); SearchInvoker invoker = disp.getSearchInvoker(q, null); invokerFactory.verifyAllEventsProcessed(); + disp.deconstruct(); } @Test @@ -53,9 +53,10 @@ public class DispatcherTest { } }; MockInvokerFactory invokerFactory = new MockInvokerFactory(cl, (n, a) -> true); - Dispatcher disp = new Dispatcher(cl, createDispatchConfig(), invokerFactory, invokerFactory, new MockMetric()); + Dispatcher disp = new Dispatcher(new ClusterMonitor(cl, false), cl, createDispatchConfig(), invokerFactory, new MockMetric()); SearchInvoker invoker = disp.getSearchInvoker(new Query(), null); invokerFactory.verifyAllEventsProcessed(); + disp.deconstruct(); } @Test @@ -69,9 +70,10 @@ public class DispatcherTest { assertTrue(acceptIncompleteCoverage); return true; }); - Dispatcher disp = new Dispatcher(cl, createDispatchConfig(), invokerFactory, invokerFactory, new MockMetric()); + Dispatcher disp = new Dispatcher(new ClusterMonitor(cl, false), cl, createDispatchConfig(), invokerFactory, new MockMetric()); SearchInvoker invoker = disp.getSearchInvoker(new Query(), null); invokerFactory.verifyAllEventsProcessed(); + disp.deconstruct(); } @Test @@ -80,8 +82,9 @@ public class DispatcherTest { SearchCluster cl = new MockSearchCluster("1", 2, 1); MockInvokerFactory invokerFactory = new MockInvokerFactory(cl, (n, a) -> false, (n, a) -> false); - Dispatcher disp = new Dispatcher(cl, createDispatchConfig(), invokerFactory, invokerFactory, new MockMetric()); + Dispatcher disp = new Dispatcher(new ClusterMonitor(cl, false), cl, createDispatchConfig(), invokerFactory, new MockMetric()); disp.getSearchInvoker(new Query(), null); + disp.deconstruct(); fail("Expected exception"); } catch (IllegalStateException e) { @@ -142,7 +145,7 @@ public class DispatcherTest { } @Override - public Callable<Pong> createPinger(Node node, ClusterMonitor<Node> monitor) { + public Pinger createPinger(Node node, ClusterMonitor<Node> monitor, PongHandler pongHandler) { fail("Unexpected call to createPinger"); return null; } diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/LoadBalancerTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/LoadBalancerTest.java index 0496194f8ed..36b476e2936 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/LoadBalancerTest.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/LoadBalancerTest.java @@ -29,7 +29,7 @@ public class LoadBalancerTest { @Test public void requireThatLoadBalancerServesSingleNodeSetups() { Node n1 = new Node(0, "test-node1", 0); - SearchCluster cluster = new SearchCluster("a", createDispatchConfig(n1), 1, null); + SearchCluster cluster = new SearchCluster("a", createDispatchConfig(n1), null, null); LoadBalancer lb = new LoadBalancer(cluster, true); Optional<Group> grp = lb.takeGroup(null); @@ -43,7 +43,7 @@ public class LoadBalancerTest { public void requireThatLoadBalancerServesMultiGroupSetups() { Node n1 = new Node(0, "test-node1", 0); Node n2 = new Node(1, "test-node2", 1); - SearchCluster cluster = new SearchCluster("a", createDispatchConfig(n1, n2), 1, null); + SearchCluster cluster = new SearchCluster("a", createDispatchConfig(n1, n2), null, null); LoadBalancer lb = new LoadBalancer(cluster, true); Optional<Group> grp = lb.takeGroup(null); @@ -59,7 +59,7 @@ public class LoadBalancerTest { Node n2 = new Node(1, "test-node2", 0); Node n3 = new Node(0, "test-node3", 1); Node n4 = new Node(1, "test-node4", 1); - SearchCluster cluster = new SearchCluster("a", createDispatchConfig(n1, n2, n3, n4), 2, null); + SearchCluster cluster = new SearchCluster("a", createDispatchConfig(n1, n2, n3, n4), null, null); LoadBalancer lb = new LoadBalancer(cluster, true); Optional<Group> grp = lb.takeGroup(null); @@ -70,7 +70,7 @@ public class LoadBalancerTest { public void requireThatLoadBalancerReturnsDifferentGroups() { Node n1 = new Node(0, "test-node1", 0); Node n2 = new Node(1, "test-node2", 1); - SearchCluster cluster = new SearchCluster("a", createDispatchConfig(n1, n2), 1, null); + SearchCluster cluster = new SearchCluster("a", createDispatchConfig(n1, n2), null,null); LoadBalancer lb = new LoadBalancer(cluster, true); // get first group diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java b/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java index a976b287f63..32c6738fc3b 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java @@ -30,7 +30,7 @@ public class MockSearchCluster extends SearchCluster { } public MockSearchCluster(String clusterId, DispatchConfig dispatchConfig, int groups, int nodesPerGroup) { - super(clusterId, dispatchConfig, 1, null); + super(clusterId, dispatchConfig, null, null); ImmutableList.Builder<Group> orderedGroupBuilder = ImmutableList.builder(); ImmutableMap.Builder<Integer, Group> groupBuilder = ImmutableMap.builder(); diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTest.java index 10a579b0e4f..cf90a1c6d81 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTest.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTest.java @@ -14,12 +14,12 @@ import org.junit.Test; import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import java.util.concurrent.Callable; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -34,6 +34,7 @@ public class SearchClusterTest { final int nodesPerGroup; final VipStatus vipStatus; final SearchCluster searchCluster; + final ClusterMonitor clusterMonitor; final List<AtomicInteger> numDocsPerNode; List<AtomicInteger> pingCounts; @@ -57,74 +58,76 @@ public class SearchClusterTest { numDocsPerNode.add(new AtomicInteger(1)); pingCounts.add(new AtomicInteger(0)); } - searchCluster = new SearchCluster(clusterId, MockSearchCluster.createDispatchConfig(nodes), nodes.size() / nodesPerGroup, vipStatus); + searchCluster = new SearchCluster(clusterId, MockSearchCluster.createDispatchConfig(nodes), nodes.size() / nodesPerGroup, + vipStatus, new Factory(nodesPerGroup, numDocsPerNode, pingCounts)); + clusterMonitor = new ClusterMonitor(searchCluster, false); + searchCluster.addMonitoring(clusterMonitor); } - void startMonitoring() { - searchCluster.startClusterMonitoring(new Factory(nodesPerGroup, numDocsPerNode, pingCounts)); - } - - static private int maxFrom(List<AtomicInteger> list) { - int max = list.get(0).get(); - for (AtomicInteger v : list) { - if (v.get() > max) { - max = v.get(); + private int maxPingCount() { + int max = pingCounts.get(0).get(); + for (AtomicInteger count : pingCounts) { + if (count.get() > max) { + max = count.get(); } } return max; } - private static int minFrom(List<AtomicInteger> list) { - int min = list.get(0).get(); - for (AtomicInteger v : list) { - if (v.get() < min) { - min = v.get(); + private int minPingCount() { + int min = pingCounts.get(0).get(); + for (AtomicInteger count : pingCounts) { + if (count.get() < min) { + min = count.get(); } } return min; } - private void waitAtLeast(int atLeast, List<AtomicInteger> list) { - while (minFrom(list) < atLeast) { + void waitOneFullPingRound() { + int minPingCount = minPingCount(); + int atLeast = maxPingCount() + 1; + while (minPingCount < atLeast) { ExecutorService executor = Executors.newCachedThreadPool(); - searchCluster.clusterMonitor().ping(executor); + clusterMonitor.ping(executor); executor.shutdown(); try { boolean completed = executor.awaitTermination(120, TimeUnit.SECONDS); if ( ! completed ) throw new IllegalStateException("Ping thread timed out"); + // Since a separate thread will be modifying values in pingCounts, we need to wait for the thread to + // finish before re-reading the minimum value + minPingCount = minPingCount(); } catch (InterruptedException e) { - System.out.println("Ping thread interrupted"); + throw new RuntimeException(e); } } } - void waitOneFullPingRound() { - waitAtLeast(maxFrom(pingCounts) + 1, pingCounts); - } - @Override public void close() { - searchCluster.shutDown(); + clusterMonitor.shutdown(); } static class Factory implements PingFactory { - static class Pinger implements Callable<Pong> { + static class PingJob implements Pinger { private final AtomicInteger numDocs; private final AtomicInteger pingCount; - Pinger(AtomicInteger numDocs, AtomicInteger pingCount) { + private final PongHandler pongHandler; + PingJob(AtomicInteger numDocs, AtomicInteger pingCount, PongHandler pongHandler) { this.numDocs = numDocs; this.pingCount = pingCount; + this.pongHandler = pongHandler; } @Override - public Pong call() { + public void ping() { int docs = numDocs.get(); - pingCount.incrementAndGet(); - return (docs < 0) + pongHandler.handle ((docs < 0) ? new Pong(ErrorMessage.createBackendCommunicationError("Negative numDocs = " + docs)) - : new Pong(docs); + : new Pong(docs)); + pingCount.incrementAndGet(); } } @@ -139,9 +142,9 @@ public class SearchClusterTest { } @Override - public Callable<Pong> createPinger(Node node, ClusterMonitor<Node> monitor) { + public Pinger createPinger(Node node, ClusterMonitor<Node> monitor, PongHandler pongHandler) { int index = node.group() * numPerGroup + node.key(); - return new Pinger(activeDocs.get(index), pingCounts.get(index)); + return new PingJob(activeDocs.get(index), pingCounts.get(index), pongHandler); } } @@ -153,7 +156,6 @@ public class SearchClusterTest { assertTrue(test.searchCluster.localCorpusDispatchTarget().isEmpty()); assertFalse(test.vipStatus.isInRotation()); - test.startMonitoring(); test.waitOneFullPingRound(); assertTrue(test.vipStatus.isInRotation()); } @@ -162,7 +164,6 @@ public class SearchClusterTest { @Test public void requireThatZeroDocsAreFine() { try (State test = new State("cluster.1", 2, "a", "b")) { - test.startMonitoring(); test.waitOneFullPingRound(); assertTrue(test.vipStatus.isInRotation()); @@ -184,7 +185,6 @@ public class SearchClusterTest { assertTrue(test.searchCluster.localCorpusDispatchTarget().isPresent()); assertFalse(test.vipStatus.isInRotation()); - test.startMonitoring(); test.waitOneFullPingRound(); assertTrue(test.vipStatus.isInRotation()); } @@ -196,7 +196,6 @@ public class SearchClusterTest { assertTrue(test.searchCluster.localCorpusDispatchTarget().isPresent()); assertFalse(test.vipStatus.isInRotation()); - test.startMonitoring(); test.waitOneFullPingRound(); assertTrue(test.vipStatus.isInRotation()); test.numDocsPerNode.get(0).set(-1); @@ -209,7 +208,6 @@ public class SearchClusterTest { public void requireThatVipStatusDownWhenLocalIsDown() { try (State test = new State("cluster.1",1,HostName.getLocalhost(), "b")) { - test.startMonitoring(); test.waitOneFullPingRound(); assertTrue(test.vipStatus.isInRotation()); assertTrue(test.searchCluster.localCorpusDispatchTarget().isPresent()); @@ -245,7 +243,6 @@ public class SearchClusterTest { List<String> nodeNames = generateNodeNames(numGroups, nodesPerGroup); try (State test = new State("cluster.1", nodesPerGroup, nodeNames)) { - test.startMonitoring(); test.waitOneFullPingRound(); assertTrue(test.vipStatus.isInRotation()); assertTrue(test.searchCluster.localCorpusDispatchTarget().isEmpty()); @@ -273,8 +270,8 @@ public class SearchClusterTest { static private List<String> generateNodeNames(int numGroups, int nodesPerGroup) { List<String> nodeNames = new ArrayList<>(numGroups*nodesPerGroup); for (int g = 0; g < numGroups; g++) { - for (int n=0; n < nodesPerGroup; n++) { - nodeNames.add(new StringBuilder("node.").append(g).append('.').append(n).toString()); + for (int n = 0; n < nodesPerGroup; n++) { + nodeNames.add("node." + g + '.' + n); } } return nodeNames; @@ -284,7 +281,6 @@ public class SearchClusterTest { List<String> nodeNames = generateNodeNames(numGroups, nodesPerGroup); try (State test = new State("cluster.1", nodesPerGroup, nodeNames)) { - test.startMonitoring(); test.waitOneFullPingRound(); assertTrue(test.vipStatus.isInRotation()); assertTrue(test.searchCluster.localCorpusDispatchTarget().isEmpty()); @@ -310,4 +306,18 @@ public class SearchClusterTest { verifyThatVipStatusUpRequireOnlyOneOnlineNode(3, 3); } + @Test + public void requireThatPingSequenceIsUpHeld() { + Node node = new Node(1, "n", 1); + assertEquals(1, node.createPingSequenceId()); + assertEquals(2, node.createPingSequenceId()); + assertEquals(0, node.getLastReceivedPongId()); + assertTrue(node.isLastReceivedPong(2)); + assertEquals(2, node.getLastReceivedPongId()); + assertFalse(node.isLastReceivedPong(1)); + assertFalse(node.isLastReceivedPong(2)); + assertTrue(node.isLastReceivedPong(3)); + assertEquals(3, node.getLastReceivedPongId()); + } + } diff --git a/container-search/src/test/java/com/yahoo/search/federation/test/FederationSearcherTestCase.java b/container-search/src/test/java/com/yahoo/search/federation/test/FederationSearcherTestCase.java index 111b7a8eb69..65cb4dff1f8 100644 --- a/container-search/src/test/java/com/yahoo/search/federation/test/FederationSearcherTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/federation/test/FederationSearcherTestCase.java @@ -201,13 +201,33 @@ public class FederationSearcherTestCase { } @Test - public void testPropertyPropagation() { - Result result = searchWithPropertyPropagation(PropagateSourceProperties.ALL); + public void testPropertyPropagation_native() { + Result result = searchWithPropertyPropagation(PropagateSourceProperties.NATIVE); assertEquals("source:mySource1", result.hits().get(0).getId().stringValue()); assertEquals("source:mySource2", result.hits().get(1).getId().stringValue()); assertEquals("nalle", result.hits().get(0).getQuery().getPresentation().getSummary()); assertNull(result.hits().get(1).getQuery().getPresentation().getSummary()); + assertEquals(null, result.hits().get(0).getQuery().properties().get("custom")); + } + + @Test + public void testPropertyPropagation_every() { + Result result = searchWithPropertyPropagation(PropagateSourceProperties.EVERY); + + assertEquals("source:mySource1", result.hits().get(0).getId().stringValue()); + assertEquals("source:mySource2", result.hits().get(1).getId().stringValue()); + assertEquals("nalle", result.hits().get(0).getQuery().getPresentation().getSummary()); + assertEquals("foo", result.hits().get(0).getQuery().properties().get("customSourceProperty")); + assertEquals(null, result.hits().get(1).getQuery().properties().get("customSourceProperty")); + assertEquals(null, result.hits().get(0).getQuery().properties().get("custom.source.property")); + assertEquals("bar", result.hits().get(1).getQuery().properties().get("custom.source.property")); + assertEquals(13, result.hits().get(0).getQuery().properties().get("hits")); + assertEquals(1, result.hits().get(0).getQuery().properties().get("offset")); + assertEquals(10, result.hits().get(1).getQuery().properties().get("hits")); + assertEquals(0, result.hits().get(1).getQuery().properties().get("offset")); + + assertNull(result.hits().get(1).getQuery().getPresentation().getSummary()); } private Result searchWithPropertyPropagation(PropagateSourceProperties.Enum propagateSourceProperties) { @@ -215,7 +235,7 @@ public class FederationSearcherTestCase { addChained(new MockSearcher(), "mySource2"); Chain<Searcher> mainChain = new Chain<>("default", createFederationSearcher(propagateSourceProperties)); - Query q = new Query(QueryTestCase.httpEncode("?query=test&source.mySource1.presentation.summary=nalle")); + Query q = new Query(QueryTestCase.httpEncode("?query=test&source.mySource1.presentation.summary=nalle&source.mySource1.customSourceProperty=foo&source.mySource2.custom.source.property=bar&source.mySource1.hits=13&source.mySource1.offset=1")); Result result = new Execution(mainChain, Execution.Context.createContextStub(chainRegistry, null)).search(q); assertNull(result.hits().getError()); diff --git a/container-search/src/test/java/com/yahoo/search/grouping/request/parser/GroupingParserBenchmarkTest.java b/container-search/src/test/java/com/yahoo/search/grouping/request/parser/GroupingParserBenchmarkTest.java index 326e37ede38..6d9c2218022 100644 --- a/container-search/src/test/java/com/yahoo/search/grouping/request/parser/GroupingParserBenchmarkTest.java +++ b/container-search/src/test/java/com/yahoo/search/grouping/request/parser/GroupingParserBenchmarkTest.java @@ -15,7 +15,7 @@ import java.util.concurrent.TimeUnit; */ public class GroupingParserBenchmarkTest { - private static final int NUM_RUNS = 10;//000; + private static final int NUM_RUNS = 10; private static final Map<String, Long> PREV_RESULTS = new LinkedHashMap<>(); static { diff --git a/container-search/src/test/java/com/yahoo/search/grouping/request/parser/GroupingParserTestCase.java b/container-search/src/test/java/com/yahoo/search/grouping/request/parser/GroupingParserTestCase.java index cd080405a7d..c6686471dc8 100644 --- a/container-search/src/test/java/com/yahoo/search/grouping/request/parser/GroupingParserTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/grouping/request/parser/GroupingParserTestCase.java @@ -247,6 +247,7 @@ public class GroupingParserTestCase { assertParse("all(group(predefined(foo, bucket(1, 2), bucket(3, 4), bucket(5, 6))))"); assertParse("all(group(predefined(foo, bucket(1, 2), bucket(2, 3), bucket(3, 4))))"); assertParse("all(group(predefined(foo, bucket(-100, 0), bucket(0), bucket<0, 100))))"); + assertParse("all(group(predefined(foo, bucket[1, 2>, bucket[3, 4>)))"); assertParse("all(group(predefined(foo, bucket[1, 2>)))"); assertParse("all(group(predefined(foo, bucket[-1, 2>)))"); diff --git a/container-search/src/test/java/com/yahoo/search/handler/test/JSONSearchHandlerTestCase.java b/container-search/src/test/java/com/yahoo/search/handler/test/JSONSearchHandlerTestCase.java index 02e2152d7c9..272092b6fc0 100644 --- a/container-search/src/test/java/com/yahoo/search/handler/test/JSONSearchHandlerTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/handler/test/JSONSearchHandlerTestCase.java @@ -12,7 +12,7 @@ import com.yahoo.net.HostName; import com.yahoo.search.handler.SearchHandler; import com.yahoo.search.searchchain.config.test.SearchChainConfigurerTestCase; import com.yahoo.slime.Inspector; -import com.yahoo.vespa.config.SlimeUtils; +import com.yahoo.slime.SlimeUtils; import org.json.JSONArray; import org.json.JSONObject; import org.junit.After; diff --git a/container-search/src/test/java/com/yahoo/search/query/profile/config/test/XmlReadingTestCase.java b/container-search/src/test/java/com/yahoo/search/query/profile/config/test/XmlReadingTestCase.java index 0f4a22ef368..e9f7ff24d42 100644 --- a/container-search/src/test/java/com/yahoo/search/query/profile/config/test/XmlReadingTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/query/profile/config/test/XmlReadingTestCase.java @@ -384,43 +384,64 @@ public class XmlReadingTestCase { assertNull(query.properties().get("profileRef.myProfile1Only")); // later assignment - query.properties().set("profileRef.name","newName"); - assertEquals("newName",query.properties().get("profileRef.name")); + query.properties().set("profileRef.name", "newName"); + assertEquals("newName", query.properties().get("profileRef.name")); // ...will not impact others - query=new Query(HttpRequest.createTestRequest("?query=test&profileRef=ref:MyProfile2", Method.GET),registry.getComponent("default")); - assertEquals("MyProfile2",query.properties().get("profileRef.name")); + query=new Query(HttpRequest.createTestRequest("?query=test&profileRef=ref:MyProfile2", Method.GET), registry.getComponent("default")); + assertEquals("MyProfile2", query.properties().get("profileRef.name")); } } @Test public void testRefOverrideTyped() { - CompiledQueryProfileRegistry registry=new QueryProfileXMLReader().read("src/test/java/com/yahoo/search/query/profile/config/test/refoverridetyped").compile(); + CompiledQueryProfileRegistry registry = new QueryProfileXMLReader().read("src/test/java/com/yahoo/search/query/profile/config/test/refoverridetyped").compile(); { // Original reference - Query query=new Query(HttpRequest.createTestRequest("?query=test", Method.GET),registry.getComponent("default")); - assertEquals(null,query.properties().get("profileRef")); - assertEquals("MyProfile1",query.properties().get("profileRef.name")); - assertEquals("myProfile1Only",query.properties().get("profileRef.myProfile1Only")); + Query query = new Query(HttpRequest.createTestRequest("?query=test", Method.GET), registry.getComponent("default")); + assertEquals(null, query.properties().get("profileRef")); + assertEquals("MyProfile1", query.properties().get("profileRef.name")); + assertEquals("myProfile1Only", query.properties().get("profileRef.myProfile1Only")); assertNull(query.properties().get("profileRef.myProfile2Only")); } { // Overridden reference - Query query=new Query(HttpRequest.createTestRequest("?query=test&profileRef=MyProfile2", Method.GET),registry.getComponent("default")); - assertEquals(null,query.properties().get("profileRef")); - assertEquals("MyProfile2",query.properties().get("profileRef.name")); - assertEquals("myProfile2Only",query.properties().get("profileRef.myProfile2Only")); + Query query = new Query(HttpRequest.createTestRequest("?query=test&profileRef=MyProfile2", Method.GET), registry.getComponent("default")); + assertEquals(null, query.properties().get("profileRef")); + assertEquals("MyProfile2", query.properties().get("profileRef.name")); + assertEquals("myProfile2Only", query.properties().get("profileRef.myProfile2Only")); assertNull(query.properties().get("profileRef.myProfile1Only")); // later assignment - query.properties().set("profileRef.name","newName"); - assertEquals("newName",query.properties().get("profileRef.name")); + query.properties().set("profileRef.name", "newName"); + assertEquals("newName", query.properties().get("profileRef.name")); // ...will not impact others - query=new Query(HttpRequest.createTestRequest("?query=test&profileRef=ref:MyProfile2", Method.GET),registry.getComponent("default")); - assertEquals("MyProfile2",query.properties().get("profileRef.name")); + query = new Query(HttpRequest.createTestRequest("?query=test&profileRef=ref:MyProfile2", Method.GET), registry.getComponent("default")); + assertEquals("MyProfile2", query.properties().get("profileRef.name")); } } + @Test + public void testTensorTypes() { + CompiledQueryProfileRegistry registry = new QueryProfileXMLReader().read("src/test/java/com/yahoo/search/query/profile/config/test/tensortypes").compile(); + + QueryProfileType type1 = registry.getTypeRegistry().getComponent("type1"); + assertEquals("tensor<float>(x[1])", type1.getFieldType(new CompoundName("ranking.features.query(tensor_1)")).stringValue()); + assertNull(type1.getFieldType(new CompoundName("ranking.features.query(tensor_2)"))); + assertNull(type1.getFieldType(new CompoundName("ranking.features.query(tensor_3)"))); + + QueryProfileType type2 = registry.getTypeRegistry().getComponent("type2"); + assertNull(type2.getFieldType(new CompoundName("ranking.features.query(tensor_1)"))); + assertEquals("tensor<float>(x[2])", type2.getFieldType(new CompoundName("ranking.features.query(tensor_2)")).stringValue()); + assertEquals("tensor<float>(x[3])", type2.getFieldType(new CompoundName("ranking.features.query(tensor_3)")).stringValue()); + + Query queryProfile1 = new Query("?query=test&ranking.features.query(tensor_1)=[1.200]", registry.getComponent("profile1")); + assertEquals("Is received as a tensor tensor", "tensor<float>(x[1]):[1.2]", queryProfile1.properties().get("ranking.features.query(tensor_1)").toString()); + + Query queryProfile2 = new Query("?query=test&ranking.features.query(tensor_1)=[1.200]", registry.getComponent("profile2")); + assertEquals("Is received as a string", "[1.200]", queryProfile2.properties().get("ranking.features.query(tensor_1)").toString()); + } + } diff --git a/container-search/src/test/java/com/yahoo/search/query/profile/config/test/tensortypes/profile1.xml b/container-search/src/test/java/com/yahoo/search/query/profile/config/test/tensortypes/profile1.xml new file mode 100644 index 00000000000..000fd3e1c5b --- /dev/null +++ b/container-search/src/test/java/com/yahoo/search/query/profile/config/test/tensortypes/profile1.xml @@ -0,0 +1,2 @@ +<query-profile id="profile1" type="type1"> +</query-profile> diff --git a/container-search/src/test/java/com/yahoo/search/query/profile/config/test/tensortypes/profile2.xml b/container-search/src/test/java/com/yahoo/search/query/profile/config/test/tensortypes/profile2.xml new file mode 100644 index 00000000000..f6539da23e8 --- /dev/null +++ b/container-search/src/test/java/com/yahoo/search/query/profile/config/test/tensortypes/profile2.xml @@ -0,0 +1,2 @@ +<query-profile id="profile2" type="type2"> +</query-profile> diff --git a/container-search/src/test/java/com/yahoo/search/query/profile/config/test/tensortypes/types/type1.xml b/container-search/src/test/java/com/yahoo/search/query/profile/config/test/tensortypes/types/type1.xml new file mode 100644 index 00000000000..3dfaab9c5f2 --- /dev/null +++ b/container-search/src/test/java/com/yahoo/search/query/profile/config/test/tensortypes/types/type1.xml @@ -0,0 +1,3 @@ +<query-profile-type id="type1"> + <field name="ranking.features.query(tensor_1)" type="tensor<float>(x[1])" /> +</query-profile-type> diff --git a/container-search/src/test/java/com/yahoo/search/query/profile/config/test/tensortypes/types/type2.xml b/container-search/src/test/java/com/yahoo/search/query/profile/config/test/tensortypes/types/type2.xml new file mode 100644 index 00000000000..ed7cf23e464 --- /dev/null +++ b/container-search/src/test/java/com/yahoo/search/query/profile/config/test/tensortypes/types/type2.xml @@ -0,0 +1,4 @@ +<query-profile-type id="type2"> + <field name="ranking.features.query(tensor_2)" type="tensor<float>(x[2])" /> + <field name="ranking.features.query(tensor_3)" type="tensor<float>(x[3])" /> +</query-profile-type> diff --git a/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileTestCase.java b/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileTestCase.java index 46efb736918..eb1584efe84 100644 --- a/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileTestCase.java @@ -326,14 +326,14 @@ public class QueryProfileTestCase { assertEquals("mormor-model.b", annetBarnMap.get("venn.model.b")); } - /** Tests that dots are followed when setting overridability */ + /** Dots are followed when setting overridability */ @Test public void testInstanceOverridable() { QueryProfile profile = new QueryProfile("root/unoverridableIndex"); profile.set("model.defaultIndex","default", null); profile.setOverridable("model.defaultIndex", false,null); - assertFalse(profile.isDeclaredOverridable("model.defaultIndex",null).booleanValue()); + assertFalse(profile.isDeclaredOverridable("model.defaultIndex",null)); // Parameters should be ignored Query query = new Query(HttpRequest.createTestRequest("?model.defaultIndex=title", Method.GET), profile.compile(null)); @@ -345,7 +345,7 @@ public class QueryProfileTestCase { assertEquals("de", query.getModel().getLanguage().languageCode()); } - /** Tests that dots are followed when setting overridability...also with variants */ + /** Dots are followed when setting overridability, also with variants */ @Test public void testInstanceOverridableWithVariants() { QueryProfile profile = new QueryProfile("root/unoverridableIndex"); @@ -504,7 +504,8 @@ public class QueryProfileTestCase { p.set("a","a-value", null); p.set("a.b","a.b-value", null); Map<String, Object> values = p.compile(null).listValues("a"); - assertEquals(1, values.size()); + assertEquals(2, values.size()); + p.set("a","a-value", null); assertEquals("a.b-value", values.get("b")); } diff --git a/container-search/src/test/java/com/yahoo/search/query/profile/types/test/QueryProfileTypeTestCase.java b/container-search/src/test/java/com/yahoo/search/query/profile/types/test/QueryProfileTypeTestCase.java index c05c3589a30..3c200debcaf 100644 --- a/container-search/src/test/java/com/yahoo/search/query/profile/types/test/QueryProfileTypeTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/query/profile/types/test/QueryProfileTypeTestCase.java @@ -397,7 +397,7 @@ public class QueryProfileTypeTestCase { @Test public void testTensorRankFeatureInRequest() throws UnsupportedEncodingException { - QueryProfile profile=new QueryProfile("test"); + QueryProfile profile = new QueryProfile("test"); profile.setType(type); registry.register(profile); @@ -447,25 +447,25 @@ public class QueryProfileTypeTestCase { */ @Test public void testTypedOverridingOfQueryProfileReferencesNonStrictThroughQueryNestedInAnUntypedProfile() { - QueryProfile topMap=new QueryProfile("topMap"); + QueryProfile topMap = new QueryProfile("topMap"); - QueryProfile subMap=new QueryProfile("topSubMap"); - topMap.set("subMap",subMap, registry); + QueryProfile subMap = new QueryProfile("topSubMap"); + topMap.set("subMap", subMap, registry); - QueryProfile test=new QueryProfile("test"); + QueryProfile test = new QueryProfile("test"); test.setType(type); - subMap.set("typeProfile",test, registry); + subMap.set("typeProfile", test, registry); - QueryProfile myUser=new QueryProfile("myUser"); + QueryProfile myUser = new QueryProfile("myUser"); myUser.setType(user); - myUser.set("myUserString","userValue1", registry); - myUser.set("myUserInteger",442, registry); - test.set("myUserQueryProfile",myUser, registry); + myUser.set("myUserString", "userValue1", registry); + myUser.set("myUserInteger", 442, registry); + test.set("myUserQueryProfile", myUser, registry); - QueryProfile newUser=new QueryProfile("newUser"); + QueryProfile newUser = new QueryProfile("newUser"); newUser.setType(user); - newUser.set("myUserString","newUserValue1", registry); - newUser.set("myUserInteger",845, registry); + newUser.set("myUserString", "newUserValue1", registry); + newUser.set("myUserInteger", 845, registry); registry.register(topMap); registry.register(subMap); diff --git a/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java b/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java index 84565472820..34c3da395b7 100644 --- a/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.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.search.test; -import com.google.common.collect.ImmutableList; import com.yahoo.component.chain.Chain; import com.yahoo.language.Language; import com.yahoo.language.Linguistics; @@ -31,12 +30,10 @@ import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.Searcher; import com.yahoo.search.grouping.GroupingQueryParser; -import com.yahoo.search.grouping.GroupingRequest; import com.yahoo.search.query.QueryTree; import com.yahoo.search.query.SessionId; import com.yahoo.search.query.profile.QueryProfile; import com.yahoo.search.query.profile.QueryProfileRegistry; -import com.yahoo.search.query.profile.compiled.CompiledQueryProfile; import com.yahoo.search.query.profile.compiled.CompiledQueryProfileRegistry; import com.yahoo.search.query.profile.types.QueryProfileType; import com.yahoo.search.result.Hit; @@ -49,23 +46,19 @@ import java.io.UnsupportedEncodingException; import java.net.URLEncoder; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; -import java.util.HashMap; +import java.util.Collection; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; import java.util.stream.Collectors; -import static org.hamcrest.CoreMatchers.containsString; -import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -240,10 +233,10 @@ public class QueryTestCase { @Test public void test_that_cloning_preserves_timeout() { Query original = new Query(); - original.setTimeout(9876l); + original.setTimeout(9876L); Query clone = original.clone(); - assertThat(clone.getTimeout(), is(9876l)); + assertEquals(9876L, clone.getTimeout()); } @Test @@ -297,9 +290,7 @@ public class QueryTestCase { fail("Above statement should throw"); } catch (QueryException e) { // As expected. - assertThat( - Exceptions.toMessageString(e), - containsString("Could not set 'timeout' to 'nalle': Error parsing 'nalle': Invalid number 'nalle'")); + assertTrue(Exceptions.toMessageString(e).contains("Could not set 'timeout' to 'nalle': Error parsing 'nalle': Invalid number 'nalle'")); } } @@ -356,6 +347,61 @@ public class QueryTestCase { } @Test + public void testQueryProfileClearAndSet() { + QueryProfile profile = new QueryProfile("myProfile"); + profile.set("b", "b-value", null); + Query q = new Query(QueryTestCase.httpEncode("/search?queryProfile=myProfile"), profile.compile(null)); + assertEquals("b-value", q.properties().get("b")); + assertContains(q.properties().listProperties("b"), "b-value"); + + q.properties().set("b", null, null); + assertContains(q.properties().listProperties("b"), (Object)null); + + q.properties().set("b", "b-value", null); + assertEquals("b-value", q.properties().get("b")); + assertContains(q.properties().listProperties("b"), "b-value"); + } + + @Test + public void testQueryProfileClearValue() { + QueryProfile profile = new QueryProfile("myProfile"); + profile.set("a", "a-value", null); + profile.set("b", "b-value", null); + profile.set("b.c", "b.c-value", null); + profile.set("b.d", "b.d-value", null); + Query q = new Query(QueryTestCase.httpEncode("/search?queryProfile=myProfile"), profile.compile(null)); + assertEquals("a-value", q.properties().get("a")); + assertEquals("b-value", q.properties().get("b")); + assertEquals("b.c-value", q.properties().get("b.c")); + assertEquals("b.d-value", q.properties().get("b.d")); + assertContains(q.properties().listProperties("b"), "b-value", "b.c-value", "b.d-value"); + + q.properties().set("a", null, null); + assertEquals(null, q.properties().get("a")); + + q.properties().set("b", null, null); + assertEquals(null, q.properties().get("b")); + assertEquals("b.c-value", q.properties().get("b.c")); + assertEquals("b.d-value", q.properties().get("b.d")); + assertContains(q.properties().listProperties("b"), null, "b.c-value", "b.d-value"); + + q.properties().set("b", "b-value", null); + q.properties().set("b.e", "b.e-value", null); + q.properties().set("b.f", "b.f-value", null); + assertEquals("b-value", q.properties().get("b")); + assertEquals("b.e-value", q.properties().get("b.e")); + assertContains(q.properties().listProperties("b"), "b-value", "b.c-value", "b.d-value", "b.e-value", "b.f-value"); + + q.properties().clearAll("b"); + assertEquals(null, q.properties().get("b")); + assertEquals(null, q.properties().get("b.c")); + assertEquals(null, q.properties().get("b.d")); + assertEquals(null, q.properties().get("b.e")); + assertEquals(null, q.properties().get("b.f")); + assertContains(q.properties().listProperties("b"), (Object)null); + } + + @Test public void testNotEqual() { Query q = new Query("/?query=something+test&nocache"); Query p = new Query("/?query=something+test"); @@ -900,7 +946,7 @@ public class QueryTestCase { @Test public void testImplicitPhrase() { - Query query = new Query(httpEncode("?query=myfield:it's myfield:fine")); + Query query = new Query(httpEncode("?query=myfield:it's myfield:a-b myfield:c")); SearchDefinition test = new SearchDefinition("test"); Index myField = new Index("myfield"); @@ -910,12 +956,12 @@ public class QueryTestCase { IndexModel indexModel = new IndexModel(test); query.getModel().setExecution(new Execution(Execution.Context.createContextStub(new IndexFacts(indexModel)))); - assertEquals("AND myfield:'it s' myfield:fine", query.getModel().getQueryTree().toString()); + assertEquals("AND myfield:'it s' myfield:\"a b\" myfield:c", query.getModel().getQueryTree().toString()); } @Test public void testImplicitAnd() { - Query query = new Query(httpEncode("?query=myfield:it's myfield:fine")); + Query query = new Query(httpEncode("?query=myfield:it's myfield:a-b myfield:c")); SearchDefinition test = new SearchDefinition("test"); Index myField = new Index("myfield"); @@ -925,7 +971,7 @@ public class QueryTestCase { IndexModel indexModel = new IndexModel(test); query.getModel().setExecution(new Execution(Execution.Context.createContextStub(new IndexFacts(indexModel)))); - assertEquals("AND (SAND myfield:it myfield:s) myfield:fine", query.getModel().getQueryTree().toString()); + assertEquals("AND (SAND myfield:it myfield:s) myfield:a myfield:b myfield:c", query.getModel().getQueryTree().toString()); } @Test @@ -996,6 +1042,19 @@ public class QueryTestCase { assertEquals(expectedDetectionText, mockLinguistics.detector.lastDetectionText); } + private void assertContains(Map<String, Object> properties, Object ... expectedValues) { + if (expectedValues == null) { + assertEquals(1, properties.size()); + assertTrue("Contains value null", properties.containsValue(null)); + } + else { + assertEquals(properties + " contains values " + Arrays.toString(expectedValues), + expectedValues.length, properties.size()); + for (Object expectedValue : expectedValues) + assertTrue("Contains value " + expectedValue, properties.containsValue(expectedValue)); + } + } + /** A linguistics instance which records the last language detection text passed to it */ private static class MockLinguistics extends SimpleLinguistics { diff --git a/container-search/src/test/java/com/yahoo/select/SelectTestCase.java b/container-search/src/test/java/com/yahoo/select/SelectTestCase.java index 261069ea1c3..7b1b4fe6362 100644 --- a/container-search/src/test/java/com/yahoo/select/SelectTestCase.java +++ b/container-search/src/test/java/com/yahoo/select/SelectTestCase.java @@ -652,7 +652,6 @@ public class SelectTestCase { assertGrouping(expected, parseGrouping(grouping)); } - @Test public void testMultipleGroupings() { String grouping = "[ { \"all\" : { \"group\" : \"a\", \"each\" : { \"output\" : \"count()\"}}}, { \"all\" : { \"group\" : \"b\", \"each\" : { \"output\" : \"count()\"}}} ]"; @@ -661,6 +660,20 @@ public class SelectTestCase { assertGrouping(expected, parseGrouping(grouping)); } + @Test + public void testGroupingWithPredefinedBuckets() { + String grouping = "[ { \"all\" : { \"group\" : { \"predefined\" : [ \"foo\", { \"bucket\": [1,2]}, { \"bucket\": [3,4]} ] } } } ]"; + String expected = "[[]all(group(predefined(foo, bucket[1, 2>, bucket[3, 4>)))]"; + assertGrouping(expected, parseGrouping(grouping)); + } + + @Test + public void testMultipleOutputs() { + String grouping = "[ { \"all\" : { \"group\" : \"b\", \"each\" : {\"output\": [ \"count()\", \"avg(foo)\" ] } } } ]"; + String expected = "[[]all(group(b) each(output(count(), avg(foo))))]"; + assertGrouping(expected, parseGrouping(grouping)); + } + //------------------------------------------------------------------- Other tests @Test @@ -763,7 +776,6 @@ public class SelectTestCase { } private List<VespaGroupingStep> parseGrouping(String grouping) { - return parser.getGroupingSteps(grouping); } |