From e6cddf5989e565593edf087958a1c1a5349b7459 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Mon, 7 Mar 2022 21:55:13 +0100 Subject: Add FlatteningSearcher --- .../prelude/searcher/MultipleResultsSearcher.java | 18 +-- .../src/main/java/com/yahoo/search/Query.java | 3 +- .../search/dispatch/InterleavedSearchInvoker.java | 31 ----- .../search/grouping/result/FlatteningSearcher.java | 50 ++++++++ .../java/com/yahoo/search/result/HitGroup.java | 4 +- .../result/FlatteningSearcherTestCase.java | 126 +++++++++++++++++++++ .../grouping/result/HitRendererTestCase.java | 1 - .../search/rendering/JsonRendererTestCase.java | 5 +- 8 files changed, 189 insertions(+), 49 deletions(-) create mode 100644 container-search/src/main/java/com/yahoo/search/grouping/result/FlatteningSearcher.java create mode 100644 container-search/src/test/java/com/yahoo/search/grouping/result/FlatteningSearcherTestCase.java (limited to 'container-search') diff --git a/container-search/src/main/java/com/yahoo/prelude/searcher/MultipleResultsSearcher.java b/container-search/src/main/java/com/yahoo/prelude/searcher/MultipleResultsSearcher.java index d017cce0d44..3c61a361cbb 100644 --- a/container-search/src/main/java/com/yahoo/prelude/searcher/MultipleResultsSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/searcher/MultipleResultsSearcher.java @@ -13,9 +13,8 @@ import com.yahoo.search.searchchain.Execution; import java.util.*; /** - *

Groups hits according to sddocname.

- * - *

For each group, the desired number of hits can be specified.

+ * Groups hits according to document type. + * For each group, the desired number of hits can be specified. * * @author Tony Vaagenes */ @@ -48,7 +47,7 @@ public class MultipleResultsSearcher extends Searcher { } } - private class HitsRetriever { + private static class HitsRetriever { PartitionedResult partitionedResult; @@ -58,12 +57,12 @@ public class MultipleResultsSearcher extends Searcher { private final Parameters parameters; private final int hits; private final int offset; - private Execution execution; - private Result initialResult; + private final Execution execution; + private final Result initialResult; HitsRetriever(Query query, Execution execution, Parameters parameters) throws ParameterException { - this.offset=query.getOffset(); - this.hits=query.getHits(); + this.offset = query.getOffset(); + this.hits = query.getHits(); this.nextOffset = query.getOffset() + query.getHits(); this.query = query; this.parameters = parameters; @@ -362,13 +361,14 @@ public class MultipleResultsSearcher extends Searcher { } } - @SuppressWarnings("serial") private static class ParameterException extends Exception { + String msg; ParameterException(String msg) { this.msg = msg; } + } } 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 fb7281e1f24..83fa18d847f 100644 --- a/container-search/src/main/java/com/yahoo/search/Query.java +++ b/container-search/src/main/java/com/yahoo/search/Query.java @@ -726,8 +726,7 @@ public class Query extends com.yahoo.processing.Request implements Cloneable { * if the trace level of the query is sufficiently high. * * @param message the message to add - * @param includeQuery true to append the query root stringValue - * at the end of the message + * @param includeQuery true to append the query root stringValue at the end of the message * @param traceLevel the context level of the message, this method will do nothing * if the traceLevel of the query is lower than this value */ diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java b/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java index d7c9f1dce53..5e38f6b4bdd 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java @@ -116,8 +116,6 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM InvokerResult result = new InvokerResult(query, query.getHits()); List merged = Collections.emptyList(); long nextTimeout = query.getTimeLeft(); - boolean extraDebug = (query.getOffset() == 0) && (query.getHits() == 7) && log.isLoggable(java.util.logging.Level.FINE); - List processed = new ArrayList<>(); var groupingResultAggregator = new GroupingResultAggregator(); try { while (!invokers.isEmpty() && nextTimeout >= 0) { @@ -127,9 +125,6 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM break; } else { InvokerResult toMerge = invoker.getSearchResult(execution); - if (extraDebug) { - processed.add(toMerge); - } merged = mergeResult(result.getResult(), toMerge, merged, groupingResultAggregator); ejectInvoker(invoker); } @@ -143,32 +138,6 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM insertNetworkErrors(result.getResult()); result.getResult().setCoverage(createCoverage()); - if (extraDebug && merged.size() > 0) { - int firstPartId = merged.get(0).getPartId(); - for (int index = 1; index < merged.size(); index++) { - if (merged.get(index).getPartId() != firstPartId) { - extraDebug = false; - log.fine("merged["+index+"/"+merged.size()+"] from partId "+merged.get(index).getPartId()+", first "+firstPartId); - break; - } - } - } - if (extraDebug) { - log.fine("Interleaved "+processed.size()+" results"); - for (int pIdx = 0; pIdx < processed.size(); ++pIdx) { - var p = processed.get(pIdx); - log.fine("InvokerResult "+pIdx+" total hits "+p.getResult().getTotalHitCount()); - var lean = p.getLeanHits(); - for (int idx = 0; idx < lean.size(); ++idx) { - var hit = lean.get(idx); - log.fine("lean hit "+idx+" relevance "+hit.getRelevance()+" partid "+hit.getPartId()); - } - } - for (int mIdx = 0; mIdx < merged.size(); ++mIdx) { - var hit = merged.get(mIdx); - log.fine("merged hit "+mIdx+" relevance "+hit.getRelevance()+" partid "+hit.getPartId()); - } - } int needed = query.getOffset() + query.getHits(); for (int index = query.getOffset(); (index < merged.size()) && (index < needed); index++) { result.getLeanHits().add(merged.get(index)); diff --git a/container-search/src/main/java/com/yahoo/search/grouping/result/FlatteningSearcher.java b/container-search/src/main/java/com/yahoo/search/grouping/result/FlatteningSearcher.java new file mode 100644 index 00000000000..321f86facd0 --- /dev/null +++ b/container-search/src/main/java/com/yahoo/search/grouping/result/FlatteningSearcher.java @@ -0,0 +1,50 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.search.grouping.result; + +import com.yahoo.component.chain.dependencies.Before; +import com.yahoo.search.Query; +import com.yahoo.search.Result; +import com.yahoo.search.Searcher; +import com.yahoo.search.grouping.vespa.GroupingExecutor; +import com.yahoo.search.result.Hit; +import com.yahoo.search.result.HitGroup; +import com.yahoo.search.searchchain.Execution; + +import java.util.Iterator; + +/** + * Flattens a grouping result into a flat list of hits on the top level in the returned result. + * Useful when using grouping to create hits with diversity and similar. + * + * @author bratseth + */ +@Before(GroupingExecutor.COMPONENT_NAME) +public class FlatteningSearcher extends Searcher { + + @Override + public Result search(Query query, Execution execution) { + if ( ! query.properties().getBoolean("flatten", true)) return execution.search(query); + + query.trace("Flattening groups", 2); + int originalHits = query.getHits(); + query.setHits(0); + Result result = execution.search(query); + query.setHits(originalHits); + flatten(result.hits(), result); + return result; + } + + public void flatten(HitGroup hits, Result result) { + int hitsLeft = hits.size(); // Iterate only through the initial size + for (Iterator i = hits.iterator(); i.hasNext() && hitsLeft-- > 0;) { + Hit hit = i.next(); + if (hit instanceof HitGroup) { + flatten((HitGroup)hit, result); + i.remove(); + } else { + result.hits().add(hit); + } + } + } + +} diff --git a/container-search/src/main/java/com/yahoo/search/result/HitGroup.java b/container-search/src/main/java/com/yahoo/search/result/HitGroup.java index 6d09bf66175..efe25e04f2e 100644 --- a/container-search/src/main/java/com/yahoo/search/result/HitGroup.java +++ b/container-search/src/main/java/com/yahoo/search/result/HitGroup.java @@ -249,9 +249,7 @@ public class HitGroup extends Hit implements DataList, Cloneable, Iterable< return hit; } - /** - * Adds a list of hits to this group, the same - */ + /** Adds a list of hits to this group, the same as calling add for each item in the list. */ public void addAll(List hits) { for (Hit hit : hits) add(hit); diff --git a/container-search/src/test/java/com/yahoo/search/grouping/result/FlatteningSearcherTestCase.java b/container-search/src/test/java/com/yahoo/search/grouping/result/FlatteningSearcherTestCase.java new file mode 100644 index 00000000000..9e5bfb4a9f5 --- /dev/null +++ b/container-search/src/test/java/com/yahoo/search/grouping/result/FlatteningSearcherTestCase.java @@ -0,0 +1,126 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.search.grouping.result; + +import com.yahoo.component.ComponentId; +import com.yahoo.component.chain.dependencies.After; +import com.yahoo.document.GlobalId; +import com.yahoo.prelude.fastsearch.FastHit; +import com.yahoo.prelude.fastsearch.GroupingListHit; +import com.yahoo.search.Query; +import com.yahoo.search.Result; +import com.yahoo.search.Searcher; +import com.yahoo.search.grouping.GroupingRequest; +import com.yahoo.search.grouping.request.GroupingOperation; +import com.yahoo.search.grouping.vespa.GroupingExecutor; +import com.yahoo.search.result.Hit; +import com.yahoo.search.result.HitGroup; +import com.yahoo.search.searchchain.Execution; +import com.yahoo.search.searchchain.SearchChain; +import com.yahoo.searchlib.aggregation.FS4Hit; +import com.yahoo.searchlib.aggregation.Group; +import com.yahoo.searchlib.aggregation.Grouping; +import com.yahoo.searchlib.aggregation.HitsAggregationResult; +import com.yahoo.searchlib.expression.StringResultNode; +import org.junit.Test; + +import java.util.Arrays; +import java.util.LinkedList; +import java.util.List; +import java.util.Queue; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +/** + * @author bratseth + */ +public class FlatteningSearcherTestCase { + + @Test + public void testFlatteningSearcher() { + Query query = new Query("?query=test"); + GroupingRequest req = GroupingRequest.newInstance(query); + req.setRootOperation(GroupingOperation.fromString("all(group(foo) each(each(output(summary(bar)))))")); + + Grouping group0 = new Grouping(0); + group0.setRoot(new com.yahoo.searchlib.aggregation.Group() + .addChild(new Group().setId(new StringResultNode("unique1")) + .addAggregationResult(new HitsAggregationResult(3, "bar") + ) + ) + .addChild(new Group().setId(new StringResultNode("unique2")) + .addAggregationResult(new HitsAggregationResult(3, "bar") + ) + )); + Grouping group1 = new Grouping(0); + group1.setRoot(new com.yahoo.searchlib.aggregation.Group() + .addChild(new Group().setId(new StringResultNode("unique1")) + .addAggregationResult(new HitsAggregationResult(3, "bar") + .addHit(fs4Hit(0.7)) + .addHit(fs4Hit(0.6)) + .addHit(fs4Hit(0.3)) + ) + ) + .addChild(new Group().setId(new StringResultNode("unique2")) + .addAggregationResult(new HitsAggregationResult(3, "bar") + .addHit(fs4Hit(0.5)) + .addHit(fs4Hit(0.4)) + ) + )); + Execution execution = newExecution(new FlatteningSearcher(), + new GroupingExecutor(ComponentId.fromString("grouping")), + new ResultProvider(Arrays.asList( + new GroupingListHit(List.of(group0), null), + new GroupingListHit(List.of(group1), null)))); + Result result = execution.search(query); + assertEquals(5, result.hits().size()); + assertFlat(result); + } + + private void assertFlat(Result result) { + for (var hit : result.hits()) + assertTrue(hit instanceof FastHit); + } + + private FS4Hit fs4Hit(double relevance) { + return new FS4Hit(0, new GlobalId(new byte[GlobalId.LENGTH]), relevance); + } + + private void dump(Hit hit, String indent) { + System.out.println(indent + hit); + if (hit instanceof HitGroup) { + for (var child : (HitGroup)hit) + dump(child, indent + " "); + } + } + + private static Execution newExecution(Searcher... searchers) { + return new Execution(new SearchChain(new ComponentId("foo"), Arrays.asList(searchers)), + Execution.Context.createContextStub()); + } + + @After (GroupingExecutor.COMPONENT_NAME) + private static class ResultProvider extends Searcher { + + final Queue hits = new LinkedList<>(); + int pass = 0; + + ResultProvider(List hits) { + this.hits.addAll(hits); + } + + @Override + public Result search(Query query, Execution exec) { + GroupingListHit hit = hits.poll(); + for (Grouping group : hit.getGroupingList()) { + group.setFirstLevel(pass); + group.setLastLevel(pass); + } + ++pass; + Result result = exec.search(query); + result.hits().add(hit); + return result; + } + } + +} diff --git a/container-search/src/test/java/com/yahoo/search/grouping/result/HitRendererTestCase.java b/container-search/src/test/java/com/yahoo/search/grouping/result/HitRendererTestCase.java index 7f70178bcf7..1035c9d9284 100644 --- a/container-search/src/test/java/com/yahoo/search/grouping/result/HitRendererTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/grouping/result/HitRendererTestCase.java @@ -151,7 +151,6 @@ public class HitRendererTestCase { return new Group(id, new Relevance(1)); } - @SuppressWarnings("deprecation") private static void assertRender(HitGroup hit, String expectedXml) { StringWriter str = new StringWriter(); XMLWriter out = new XMLWriter(str, 0, -1); diff --git a/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java b/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java index 637666a6a19..1d2e3c240b9 100644 --- a/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java @@ -948,7 +948,7 @@ public class JsonRendererTestCase { } }); Group g = new Group(new StringId("Jones"), new Relevance(1.0)); - g.setField("count()", Integer.valueOf(7)); + g.setField("count()", 7); gl.add(g); rg.add(gl); r.hits().add(rg); @@ -1443,8 +1443,7 @@ public class JsonRendererTestCase { ByteArrayOutputStream bs = new ByteArrayOutputStream(); ListenableFuture f = renderer.render(bs, r, execution, null); assertTrue(f.get()); - String summary = Utf8.toString(bs.toByteArray()); - return summary; + return Utf8.toString(bs.toByteArray()); } @SuppressWarnings("unchecked") -- cgit v1.2.3 From d9c17c053470b5abcaeab06b66bde914c2f9457b Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Tue, 8 Mar 2022 18:08:57 +0100 Subject: Update ABI spec --- container-search/abi-spec.json | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'container-search') diff --git a/container-search/abi-spec.json b/container-search/abi-spec.json index 9cc519ed9e2..0e45aa59421 100644 --- a/container-search/abi-spec.json +++ b/container-search/abi-spec.json @@ -4121,6 +4121,19 @@ ], "fields": [] }, + "com.yahoo.search.grouping.result.FlatteningSearcher": { + "superClass": "com.yahoo.search.Searcher", + "interfaces": [], + "attributes": [ + "public" + ], + "methods": [ + "public void ()", + "public com.yahoo.search.Result search(com.yahoo.search.Query, com.yahoo.search.searchchain.Execution)", + "public void flatten(com.yahoo.search.result.HitGroup, com.yahoo.search.Result)" + ], + "fields": [] + }, "com.yahoo.search.grouping.result.Group": { "superClass": "com.yahoo.search.result.HitGroup", "interfaces": [], -- cgit v1.2.3 From e9884be39bf30b8e00aecf439bfe8f3fb7911cf7 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 9 Mar 2022 11:18:16 +0100 Subject: Cleanup --- .../com/yahoo/search/dispatch/searchcluster/SearchCluster.java | 8 ++------ .../vespa/rankingexpression/importer/xgboost/XGBoostParser.java | 4 ++-- 2 files changed, 4 insertions(+), 8 deletions(-) (limited to 'container-search') 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 36d7e7a85a9..e5b8d799bbf 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 @@ -293,19 +293,15 @@ public class SearchCluster implements NodeManager { // With just one group sufficient coverage may not be the same as full coverage, as the // group will always be marked sufficient for use. updateSufficientCoverage(group, true); - boolean sufficientCoverage = isGroupCoverageSufficient(group.activeDocuments(), - group.activeDocuments()); + boolean sufficientCoverage = isGroupCoverageSufficient(group.activeDocuments(), group.activeDocuments()); trackGroupCoverageChanges(group, sufficientCoverage, group.activeDocuments()); } private void pingIterationCompletedMultipleGroups() { orderedGroups().forEach(Group::aggregateNodeValues); long medianDocuments = medianDocumentsPerGroup(); - boolean anyGroupsSufficientCoverage = false; for (Group group : orderedGroups()) { - boolean sufficientCoverage = isGroupCoverageSufficient(group.activeDocuments(), - medianDocuments); - anyGroupsSufficientCoverage = anyGroupsSufficientCoverage || sufficientCoverage; + boolean sufficientCoverage = isGroupCoverageSufficient(group.activeDocuments(), medianDocuments); updateSufficientCoverage(group, sufficientCoverage); trackGroupCoverageChanges(group, sufficientCoverage, medianDocuments); } diff --git a/model-integration/src/main/java/ai/vespa/rankingexpression/importer/xgboost/XGBoostParser.java b/model-integration/src/main/java/ai/vespa/rankingexpression/importer/xgboost/XGBoostParser.java index c149683d5e6..caa7129ad34 100644 --- a/model-integration/src/main/java/ai/vespa/rankingexpression/importer/xgboost/XGBoostParser.java +++ b/model-integration/src/main/java/ai/vespa/rankingexpression/importer/xgboost/XGBoostParser.java @@ -15,12 +15,12 @@ import com.fasterxml.jackson.databind.ObjectMapper; */ class XGBoostParser { - private List xgboostTrees; + private final List xgboostTrees; /** * Constructor stores parsed JSON trees. * - * @param filePath XGBoost JSON output file. + * @param filePath XGBoost JSON intput file. * @throws JsonProcessingException Fails JSON parsing. * @throws IOException Fails file reading. */ -- cgit v1.2.3 From b6206314db3250df9adb1c64432f3ffc9ab6c0b5 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 9 Mar 2022 11:23:13 +0100 Subject: Cleanup --- .../dispatch/searchcluster/SearchCluster.java | 50 +++++++++++----------- 1 file changed, 25 insertions(+), 25 deletions(-) (limited to 'container-search') 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 e5b8d799bbf..0df7fb0c154 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 @@ -255,31 +255,6 @@ public class SearchCluster implements NodeManager { return localCorpusDispatchTarget.isPresent() && localCorpusDispatchTarget.get().group() == group.id(); } - private static class PongCallback implements PongHandler { - - private final ClusterMonitor clusterMonitor; - private final Node node; - - PongCallback(Node node, ClusterMonitor clusterMonitor) { - this.node = node; - this.clusterMonitor = clusterMonitor; - } - - @Override - public void handle(Pong pong) { - if (pong.badResponse()) { - clusterMonitor.failed(node, pong.error().get()); - } else { - if (pong.activeDocuments().isPresent()) { - node.setActiveDocuments(pong.activeDocuments().get()); - node.setBlockingWrites(pong.isBlockingWrites()); - } - clusterMonitor.responded(node); - } - } - - } - /** Used by the cluster monitor to manage node status */ @Override public void ping(ClusterMonitor clusterMonitor, Node node, Executor executor) { @@ -368,4 +343,29 @@ public class SearchCluster implements NodeManager { } } + private static class PongCallback implements PongHandler { + + private final ClusterMonitor clusterMonitor; + private final Node node; + + PongCallback(Node node, ClusterMonitor clusterMonitor) { + this.node = node; + this.clusterMonitor = clusterMonitor; + } + + @Override + public void handle(Pong pong) { + if (pong.badResponse()) { + clusterMonitor.failed(node, pong.error().get()); + } else { + if (pong.activeDocuments().isPresent()) { + node.setActiveDocuments(pong.activeDocuments().get()); + node.setBlockingWrites(pong.isBlockingWrites()); + } + clusterMonitor.responded(node); + } + } + + } + } -- cgit v1.2.3 From a91fe34bdf10232b3a48ee2c83d380c0a02abf9a Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 9 Mar 2022 11:30:40 +0100 Subject: Cleanup --- .../com/yahoo/search/dispatch/searchcluster/SearchCluster.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) (limited to 'container-search') 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 0df7fb0c154..50e120a1c5e 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 @@ -153,13 +153,7 @@ public class SearchCluster implements NodeManager { } public int groupsWithSufficientCoverage() { - int covered = 0; - for (Group g : orderedGroups()) { - if (g.hasSufficientCoverage()) { - covered++; - } - } - return covered; + return (int)groups.values().stream().filter(g -> g.hasSufficientCoverage()).count(); } /** -- cgit v1.2.3 From 67939802b486126d5b07ee3e1ab1b2795939e575 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 9 Mar 2022 11:41:36 +0100 Subject: Don't assume equal-sized groups --- .../src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java | 2 +- .../com/yahoo/search/dispatch/searchcluster/SearchCluster.java | 9 ++------- .../test/java/com/yahoo/search/dispatch/MockSearchCluster.java | 4 +--- 3 files changed, 4 insertions(+), 11 deletions(-) (limited to 'container-search') diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java index 7f411bbc80f..27a45753bb5 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java @@ -81,7 +81,7 @@ public class FastSearcher extends VespaBackEndSearcher { @Override public Result doSearch2(Query query, Execution execution) { - if (dispatcher.searchCluster().wantedGroupSize() == 1) + if (dispatcher.searchCluster().allGroupsHaveSize1()) forceSinglePassGrouping(query); try (SearchInvoker invoker = getSearchInvoker(query)) { Result result = invoker.search(query, execution); 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 50e120a1c5e..a048dfd31d6 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 @@ -143,13 +143,8 @@ public class SearchCluster implements NodeManager { } } - /** - * Returns the wanted number of nodes per group - size()/groups.size(). - * The actual node count for a given group may differ due to node retirements. - */ - public int wantedGroupSize() { - if (groups().size() == 0) return size(); - return size() / groups().size(); + public boolean allGroupsHaveSize1() { + return size() == groups().size(); } public int groupsWithSufficientCoverage() { 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 54c8c1e0522..6996bd3f738 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 @@ -71,9 +71,7 @@ public class MockSearchCluster extends SearchCluster { } @Override - public int wantedGroupSize() { - return numNodesPerGroup; - } + public boolean allGroupsHaveSize1() { return numNodesPerGroup == 1;} @Override public int groupsWithSufficientCoverage() { -- cgit v1.2.3 From a74577a73d3a1c48ddd11cbe123a0ff2aa8addf0 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 9 Mar 2022 11:43:59 +0100 Subject: Cleanup --- .../src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java | 2 -- 1 file changed, 2 deletions(-) (limited to 'container-search') 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 6996bd3f738..e3f1d58e065 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 @@ -23,7 +23,6 @@ public class MockSearchCluster extends SearchCluster { private final int numNodesPerGroup; private final ImmutableList orderedGroups; private final ImmutableMap groups; - private final ImmutableMultimap nodesByHost; public MockSearchCluster(String clusterId, int groups, int nodesPerGroup) { this(clusterId, createDispatchConfig(), groups, nodesPerGroup); @@ -50,7 +49,6 @@ public class MockSearchCluster extends SearchCluster { } this.orderedGroups = orderedGroupBuilder.build(); this.groups = groupBuilder.build(); - this.nodesByHost = hostBuilder.build(); this.numGroups = groups; this.numNodesPerGroup = nodesPerGroup; } -- cgit v1.2.3 From cae22d9c6e4e0ca3aa3f75a1d7b340c76d83f30c Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 9 Mar 2022 12:00:14 +0100 Subject: Simplify --- .../dispatch/searchcluster/SearchCluster.java | 46 +++++++++------------- .../yahoo/search/dispatch/MockSearchCluster.java | 19 ++++----- 2 files changed, 29 insertions(+), 36 deletions(-) (limited to 'container-search') 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 a048dfd31d6..223a2a2da22 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 @@ -1,10 +1,8 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.dispatch.searchcluster; -import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableMultimap; import com.google.common.math.Quantiles; import com.yahoo.container.handler.VipStatus; import com.yahoo.net.HostName; @@ -32,11 +30,10 @@ public class SearchCluster implements NodeManager { private static final Logger log = Logger.getLogger(SearchCluster.class.getName()); private final DispatchConfig dispatchConfig; - private final int size; private final String clusterId; - private final ImmutableMap groups; - private final ImmutableMultimap nodesByHost; - private final ImmutableList orderedGroups; + private final Map groups; + private final List orderedGroups; + private final List nodes; private final VipStatus vipStatus; private final PingFactory pingFactory; private final TopKEstimator hitEstimator; @@ -60,8 +57,7 @@ public class SearchCluster implements NodeManager { this.vipStatus = vipStatus; this.pingFactory = pingFactory; - List nodes = toNodes(dispatchConfig); - this.size = nodes.size(); + this.nodes = toNodes(dispatchConfig); // Create groups ImmutableMap.Builder groupsBuilder = new ImmutableMap.Builder<>(); @@ -74,14 +70,8 @@ public class SearchCluster implements NodeManager { nodes.forEach(node -> groupIntroductionOrder.put(node.group(), groups.get(node.group()))); this.orderedGroups = ImmutableList.builder().addAll(groupIntroductionOrder.values()).build(); - // Index nodes by host - ImmutableMultimap.Builder nodesByHostBuilder = new ImmutableMultimap.Builder<>(); - for (Node node : nodes) - nodesByHostBuilder.put(node.hostname(), node); - this.nodesByHost = nodesByHostBuilder.build(); hitEstimator = new TopKEstimator(30.0, dispatchConfig.topKProbability(), SKEW_FACTOR); - - this.localCorpusDispatchTarget = findLocalCorpusDispatchTarget(HostName.getLocalhost(), nodesByHost, groups); + this.localCorpusDispatchTarget = findLocalCorpusDispatchTarget(HostName.getLocalhost(), nodes, groups); } @Override @@ -95,13 +85,15 @@ public class SearchCluster implements NodeManager { } private static Optional findLocalCorpusDispatchTarget(String selfHostname, - ImmutableMultimap nodesByHost, - ImmutableMap groups) { + List nodes, + Map groups) { // A search node in the search cluster in question is configured on the same host as the currently running container. // It has all the data <==> No other nodes in the search cluster have the same group id as this node. // That local search node responds. // The search cluster to be searched has at least as many nodes as the container cluster we're running in. - ImmutableCollection localSearchNodes = nodesByHost.get(selfHostname); + List localSearchNodes = nodes.stream() + .filter(node -> node.hostname().equals(selfHostname)) + .collect(Collectors.toList()); // Only use direct dispatch if we have exactly 1 search node on the same machine: if (localSearchNodes.size() != 1) return Optional.empty(); @@ -125,14 +117,14 @@ public class SearchCluster implements NodeManager { return dispatchConfig; } - /** Returns the number of nodes in this cluster (across all groups) */ - public int size() { return size; } + /** Returns an immutable list of all nodes in this. */ + public List nodes() { return nodes; } /** Returns the groups of this cluster as an immutable map indexed by group id */ - public ImmutableMap groups() { return groups; } + public Map groups() { return groups; } /** Returns the groups of this cluster as an immutable list in introduction order */ - public ImmutableList orderedGroups() { return orderedGroups; } + public List orderedGroups() { return orderedGroups; } /** Returns the n'th (zero-indexed) group in the cluster if possible */ public Optional group(int n) { @@ -144,7 +136,7 @@ public class SearchCluster implements NodeManager { } public boolean allGroupsHaveSize1() { - return size() == groups().size(); + return nodes.size() == groups.size(); } public int groupsWithSufficientCoverage() { @@ -199,8 +191,8 @@ public class SearchCluster implements NodeManager { } else if (usesLocalCorpusIn(node)) { // follow the status of this node // Do not take this out of rotation if we're a combined cluster of size 1, - // as that can't be helpful, and leads to a deadlock where this node is never taken back in servic e - if (nodeIsWorking || size() > 1) + // as that can't be helpful, and leads to a deadlock where this node is never set back in service + if (nodeIsWorking || nodes.size() > 1) setInRotationOnlyIf(nodeIsWorking); } } @@ -229,11 +221,11 @@ public class SearchCluster implements NodeManager { } public boolean hasInformationAboutAllNodes() { - return nodesByHost.values().stream().allMatch(node -> node.isWorking() != null); + return nodes.stream().allMatch(node -> node.isWorking() != null); } private boolean hasWorkingNodes() { - return nodesByHost.values().stream().anyMatch(node -> node.isWorking() != Boolean.FALSE ); + return nodes.stream().anyMatch(node -> node.isWorking() != Boolean.FALSE ); } private boolean usesLocalCorpusIn(Node node) { 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 e3f1d58e065..abd7267bb04 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 @@ -23,6 +23,7 @@ public class MockSearchCluster extends SearchCluster { private final int numNodesPerGroup; private final ImmutableList orderedGroups; private final ImmutableMap groups; + private final List nodes; public MockSearchCluster(String clusterId, int groups, int nodesPerGroup) { this(clusterId, createDispatchConfig(), groups, nodesPerGroup); @@ -35,15 +36,17 @@ public class MockSearchCluster extends SearchCluster { ImmutableMap.Builder groupBuilder = ImmutableMap.builder(); ImmutableMultimap.Builder hostBuilder = ImmutableMultimap.builder(); int distributionKey = 0; + this.nodes = new ArrayList<>(); for (int group = 0; group < groups; group++) { - List nodes = new ArrayList<>(); - for (int node = 0; node < nodesPerGroup; node++) { - Node n = new Node(distributionKey, "host" + distributionKey, group); - nodes.add(n); - hostBuilder.put(n.hostname(), n); + List groupNodes = new ArrayList<>(); + for (int i = 0; i < nodesPerGroup; i++) { + Node node = new Node(distributionKey, "host" + distributionKey, group); + nodes.add(node); + groupNodes.add(node); + hostBuilder.put(node.hostname(), node); distributionKey++; } - Group g = new Group(group, nodes); + Group g = new Group(group, groupNodes); groupBuilder.put(group, g); orderedGroupBuilder.add(g); } @@ -59,9 +62,7 @@ public class MockSearchCluster extends SearchCluster { } @Override - public int size() { - return numGroups * numNodesPerGroup; - } + public List nodes() { return nodes; } @Override public ImmutableMap groups() { -- cgit v1.2.3 From d7fe7b4d8d265665f7dbfb1f3054a5e5ec797992 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 9 Mar 2022 12:03:46 +0100 Subject: Simplify --- .../yahoo/search/dispatch/searchcluster/SearchCluster.java | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'container-search') 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 223a2a2da22..f8c4627473d 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 @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.dispatch.searchcluster; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.math.Quantiles; import com.yahoo.container.handler.VipStatus; @@ -68,7 +67,7 @@ public class SearchCluster implements NodeManager { this.groups = groupsBuilder.build(); LinkedHashMap groupIntroductionOrder = new LinkedHashMap<>(); nodes.forEach(node -> groupIntroductionOrder.put(node.group(), groups.get(node.group()))); - this.orderedGroups = ImmutableList.builder().addAll(groupIntroductionOrder.values()).build(); + this.orderedGroups = List.copyOf(groupIntroductionOrder.values()); hitEstimator = new TopKEstimator(30.0, dispatchConfig.topKProbability(), SKEW_FACTOR); this.localCorpusDispatchTarget = findLocalCorpusDispatchTarget(HostName.getLocalhost(), nodes, groups); @@ -106,11 +105,10 @@ public class SearchCluster implements NodeManager { return Optional.of(localSearchNode); } - private static ImmutableList toNodes(DispatchConfig dispatchConfig) { - ImmutableList.Builder nodesBuilder = new ImmutableList.Builder<>(); - for (DispatchConfig.Node node : dispatchConfig.node()) - nodesBuilder.add(new Node(node.key(), node.host(), node.group())); - return nodesBuilder.build(); + private static List toNodes(DispatchConfig dispatchConfig) { + return dispatchConfig.node().stream() + .map(n -> new Node(n.key(), n.host(), n.group())) + .collect(Collectors.toUnmodifiableList()); } public DispatchConfig dispatchConfig() { -- cgit v1.2.3 From b303b3a8c674c9f3f514a1747dc7abbf99907627 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 9 Mar 2022 12:47:54 +0100 Subject: Simplify --- .../com/yahoo/search/dispatch/LoadBalancer.java | 29 ++++++---------------- .../yahoo/search/dispatch/searchcluster/Group.java | 7 +++--- 2 files changed, 12 insertions(+), 24 deletions(-) (limited to 'container-search') diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/LoadBalancer.java b/container-search/src/main/java/com/yahoo/search/dispatch/LoadBalancer.java index 7f4d8fc4739..7934a9e25f6 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/LoadBalancer.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/LoadBalancer.java @@ -12,13 +12,14 @@ import java.util.Set; import java.util.logging.Logger; /** - * LoadBalancer determines which group of content nodes should be accessed next for each search query when the internal java dispatcher is - * used. + * LoadBalancer determines which group of content nodes should be accessed next for each search query when the + * internal java dispatcher is used. + * + * The implementation here is a simplistic least queries in flight + round-robin load balancer * * @author ollivir */ public class LoadBalancer { - // The implementation here is a simplistic least queries in flight + round-robin load balancer private static final Logger log = Logger.getLogger(LoadBalancer.class.getName()); @@ -174,24 +175,10 @@ public class LoadBalancer { * @return the better of the two */ private static GroupStatus betterGroup(GroupStatus first, GroupStatus second) { - if (second == null) { - return first; - } - if (first == null) { - return second; - } - - // different coverage - if (first.group.hasSufficientCoverage() != second.group.hasSufficientCoverage()) { - if (!first.group.hasSufficientCoverage()) { - // first doesn't have coverage, second does - return second; - } else { - // second doesn't have coverage, first does - return first; - } - } - + if (second == null) return first; + if (first == null) return second; + if (first.group.hasSufficientCoverage() != second.group.hasSufficientCoverage()) + return first.group.hasSufficientCoverage() ? first : second; return first; } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Group.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Group.java index e99c1d5ad32..8f6e1b1ee2f 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Group.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Group.java @@ -17,7 +17,9 @@ import java.util.logging.Logger; public class Group { private static final Logger log = Logger.getLogger(Group.class.getName()); - private final static double maxContentSkew = 0.10; // If documents on a node is more than 10% off from the average the group is unbalanced + + // If documents on a node is more than 10% off from the average the group is unbalanced + private final static double maxContentSkew = 0.10; private final static int minDocsPerNodeToRequireLowSkew = 100; private final int id; @@ -41,8 +43,7 @@ public class Group { /** * Returns the unique identity of this group. - * NOTE: This is a contiguous index from 0, NOT necessarily the group id assigned - * by the user or node repo. + * NOTE: This is a contiguous index from 0, NOT necessarily the group id assigned by the user or node repo. */ public int id() { return id; } -- cgit v1.2.3 From f035afd9460684334860eac91698bc45a6558fbd Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 9 Mar 2022 12:53:02 +0100 Subject: Cleanup --- .../src/main/java/com/yahoo/search/dispatch/LoadBalancer.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'container-search') diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/LoadBalancer.java b/container-search/src/main/java/com/yahoo/search/dispatch/LoadBalancer.java index 7934a9e25f6..4c0bcb38d15 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/LoadBalancer.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/LoadBalancer.java @@ -85,6 +85,7 @@ public class LoadBalancer { } static class GroupStatus { + private final Group group; private int allocations = 0; private long queries = 0; @@ -233,11 +234,8 @@ public class LoadBalancer { public Optional takeNextGroup(Set rejectedGroups) { double needle = random.nextDouble(); Optional gs = selectGroup(needle, true, rejectedGroups); - if (gs.isPresent()) { - return gs; - } - // fallback - any coverage better than none - return selectGroup(needle, false, rejectedGroups); + if (gs.isPresent()) return gs; + return selectGroup(needle, false, rejectedGroups); // any coverage better than none } } -- cgit v1.2.3