diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2021-07-02 12:05:35 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-07-02 12:05:35 +0200 |
commit | 130e036704c2e46717cffc56a0af3ddcbef932ac (patch) | |
tree | 9ab7917e2ff3eec439eae333bd610881da2dd1af /container-search | |
parent | 93ea6deb8ead9360335fc4fad949d45f03994c05 (diff) | |
parent | 3a3330ccd0106e355f7d8a26dbf1b15db5ab9f7b (diff) |
Merge pull request #18511 from vespa-engine/bratseth/unbalanced-with-few-docs
Bratseth/unbalanced with few docs
Diffstat (limited to 'container-search')
11 files changed, 138 insertions, 111 deletions
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 9b92a78a7c9..af731e3ade0 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 @@ -186,8 +186,8 @@ public class Dispatcher extends AbstractComponent { if (nodes.isEmpty()) return Optional.empty(); query.trace(false, 2, "Dispatching with search path ", searchPath); - return invokerFactory.createSearchInvoker(searcher, query, - OptionalInt.empty(), + return invokerFactory.createSearchInvoker(searcher, + query, nodes, true, maxHitsPerNode); @@ -203,7 +203,6 @@ public class Dispatcher extends AbstractComponent { query.trace(false, 2, "Dispatching to ", node); return invokerFactory.createSearchInvoker(searcher, query, - OptionalInt.empty(), List.of(node), true, maxHitsPerNode) @@ -222,7 +221,6 @@ public class Dispatcher extends AbstractComponent { boolean acceptIncompleteCoverage = (i == max - 1); Optional<SearchInvoker> invoker = invokerFactory.createSearchInvoker(searcher, query, - OptionalInt.of(group.id()), group.nodes(), acceptIncompleteCoverage, maxHitsPerNode); 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 adf7368faa2..fb04c8299e9 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 @@ -3,6 +3,7 @@ package com.yahoo.search.dispatch; import com.yahoo.search.Query; import com.yahoo.search.Result; +import com.yahoo.search.dispatch.searchcluster.Group; import com.yahoo.search.dispatch.searchcluster.SearchCluster; import com.yahoo.search.result.Coverage; import com.yahoo.search.result.ErrorMessage; @@ -41,9 +42,9 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM private final Set<SearchInvoker> invokers; private final SearchCluster searchCluster; + private final Group group; private final LinkedBlockingQueue<SearchInvoker> availableForProcessing; private final Set<Integer> alreadyFailedNodes; - private final boolean isContentWellBalanced; private Query query; private boolean adaptiveTimeoutCalculated = false; @@ -60,14 +61,17 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM private boolean timedOut = false; private boolean degradedByMatchPhase = false; - public InterleavedSearchInvoker(Collection<SearchInvoker> invokers, boolean isContentWellBalanced, SearchCluster searchCluster, Set<Integer> alreadyFailedNodes) { + public InterleavedSearchInvoker(Collection<SearchInvoker> invokers, + SearchCluster searchCluster, + Group group, + Set<Integer> alreadyFailedNodes) { super(Optional.empty()); this.invokers = Collections.newSetFromMap(new IdentityHashMap<>()); this.invokers.addAll(invokers); this.searchCluster = searchCluster; + this.group = group; this.availableForProcessing = newQueue(); this.alreadyFailedNodes = alreadyFailedNodes; - this.isContentWellBalanced = isContentWellBalanced; } /** @@ -85,7 +89,7 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM int originalOffset = query.getOffset(); int neededHits = originalHits + originalOffset; int q = neededHits; - if (isContentWellBalanced) { + if (group.isBalanced() && !group.isSparse()) { Double topkProbabilityOverrride = query.properties().getDouble(Dispatcher.topKProbability); q = (topkProbabilityOverrride != null) ? searchCluster.estimateHitsToFetch(neededHits, invokers.size(), topkProbabilityOverrride) diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java b/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java index 1de274ce6cf..e602afadcfb 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java @@ -4,6 +4,7 @@ package com.yahoo.search.dispatch; import com.yahoo.prelude.fastsearch.VespaBackEndSearcher; import com.yahoo.search.Query; import com.yahoo.search.Result; +import com.yahoo.search.dispatch.searchcluster.Group; import com.yahoo.search.dispatch.searchcluster.Node; import com.yahoo.search.dispatch.searchcluster.SearchCluster; import com.yahoo.search.result.Coverage; @@ -13,7 +14,6 @@ import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Optional; -import java.util.OptionalInt; import java.util.Set; /** @@ -39,8 +39,7 @@ public abstract class InvokerFactory { * * @param searcher the searcher processing the query * @param query the search query being processed - * @param groupId the id of the node group to which the nodes belong - * @param nodes pre-selected list of content nodes + * @param nodes pre-selected list of content nodes, all in a group or a subset of a group * @param acceptIncompleteCoverage if some of the nodes are unavailable and this parameter is * false, verify that the remaining set of nodes has sufficient coverage * @return the invoker or empty if some node in the @@ -48,10 +47,10 @@ public abstract class InvokerFactory { */ Optional<SearchInvoker> createSearchInvoker(VespaBackEndSearcher searcher, Query query, - OptionalInt groupId, List<Node> nodes, boolean acceptIncompleteCoverage, int maxHits) { + Group group = searchCluster.group(nodes.get(0).group()).get(); // Nodes must be of the same group List<SearchInvoker> invokers = new ArrayList<>(nodes.size()); Set<Integer> failed = null; for (Node node : nodes) { @@ -90,7 +89,7 @@ public abstract class InvokerFactory { if (invokers.size() == 1 && failed == null) { return Optional.of(invokers.get(0)); } else { - return Optional.of(new InterleavedSearchInvoker(invokers, searchCluster.isGroupWellBalanced(groupId), searchCluster, failed)); + return Optional.of(new InterleavedSearchInvoker(invokers, searchCluster, group, failed)); } } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/SearchPath.java b/container-search/src/main/java/com/yahoo/search/dispatch/SearchPath.java index f6480f80c01..b29c3297aea 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/SearchPath.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/SearchPath.java @@ -41,7 +41,7 @@ public class SearchPath { if (sp.isPresent()) { return sp.get().mapToNodes(cluster); } else { - return Collections.emptyList(); + return List.of(); } } @@ -75,7 +75,7 @@ public class SearchPath { private List<Node> mapToNodes(SearchCluster cluster) { if (cluster.groups().isEmpty()) { - return Collections.emptyList(); + return List.of(); } Group selectedGroup = selectGroup(cluster); 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 7faad9d51cc..727fb64faef 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 @@ -16,16 +16,17 @@ 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 + private final static int minDocsPerNodeToRequireLowSkew = 100; + private final int id; private final ImmutableList<Node> nodes; - private final AtomicBoolean hasSufficientCoverage = new AtomicBoolean(true); private final AtomicBoolean hasFullCoverage = new AtomicBoolean(true); private final AtomicLong activeDocuments = new AtomicLong(0); private final AtomicBoolean isBlockingWrites = new AtomicBoolean(false); - private final AtomicBoolean isContentWellBalanced = new AtomicBoolean(true); - private final static double MAX_UNBALANCE = 0.10; // If documents on a node is more than 10% off from the average the group is unbalanced - private static final Logger log = Logger.getLogger(Group.class.getName()); + private final AtomicBoolean isBalanced = new AtomicBoolean(true); public Group(int id, List<Node> nodes) { this.id = id; @@ -60,37 +61,43 @@ public class Group { return (int) nodes.stream().filter(node -> node.isWorking() == Boolean.TRUE).count(); } - void aggregateNodeValues() { + public void aggregateNodeValues() { long activeDocs = nodes.stream().filter(node -> node.isWorking() == Boolean.TRUE).mapToLong(Node::getActiveDocuments).sum(); activeDocuments.set(activeDocs); isBlockingWrites.set(nodes.stream().anyMatch(Node::isBlockingWrites)); int numWorkingNodes = workingNodes(); if (numWorkingNodes > 0) { long average = activeDocs / numWorkingNodes; - long deviation = nodes.stream().filter(node -> node.isWorking() == Boolean.TRUE).mapToLong(node -> Math.abs(node.getActiveDocuments() - average)).sum(); - boolean isDeviationSmall = deviation <= maxUnbalance(activeDocs); - if ((!isContentWellBalanced.get() || isDeviationSmall != isContentWellBalanced.get()) && (activeDocs > 0)) { - log.info("Content is " + (isDeviationSmall ? "" : "not ") + "well balanced. Current deviation = " + deviation*100/activeDocs + " %" + - ". activeDocs = " + activeDocs + ", deviation = " + deviation + ", average = " + average); - isContentWellBalanced.set(isDeviationSmall); + long skew = nodes.stream().filter(node -> node.isWorking() == Boolean.TRUE).mapToLong(node -> Math.abs(node.getActiveDocuments() - average)).sum(); + boolean balanced = skew <= activeDocs * maxContentSkew; + if (!isBalanced.get() || balanced != isBalanced.get()) { + if (!isSparse()) + log.info("Content is " + (balanced ? "" : "not ") + "well balanced. Current deviation = " + + skew * 100 / activeDocs + " %. activeDocs = " + activeDocs + ", skew = " + skew + + ", average = " + average); + isBalanced.set(balanced); } } else { - isContentWellBalanced.set(true); + isBalanced.set(true); } } - double maxUnbalance(long activeDocs) { - return Math.max(1, activeDocs * MAX_UNBALANCE); - } - /** Returns the active documents on this group. If unknown, 0 is returned. */ - long getActiveDocuments() { return activeDocuments.get(); } + long activeDocuments() { return activeDocuments.get(); } /** Returns whether any node in this group is currently blocking write operations */ public boolean isBlockingWrites() { return isBlockingWrites.get(); } - public boolean isContentWellBalanced() { return isContentWellBalanced.get(); } - public boolean isFullCoverageStatusChanged(boolean hasFullCoverageNow) { + /** Returns whether the nodes in the group have about the same number of documents */ + public boolean isBalanced() { return isBalanced.get(); } + + /** Returns whether this group has too few documents per node to expect it to be balanced */ + public boolean isSparse() { + if (nodes.isEmpty()) return false; + return activeDocuments.get() / nodes.size() < minDocsPerNodeToRequireLowSkew; + } + + public boolean fullCoverageStatusChanged(boolean hasFullCoverageNow) { boolean previousState = hasFullCoverage.getAndSet(hasFullCoverageNow); return previousState != hasFullCoverageNow; } 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 9ae25518969..54d5dfc91af 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 @@ -14,13 +14,10 @@ import com.yahoo.search.cluster.NodeManager; import com.yahoo.search.dispatch.TopKEstimator; import com.yahoo.vespa.config.search.DispatchConfig; -import java.util.ArrayList; -import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.OptionalInt; import java.util.concurrent.Executor; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -311,9 +308,9 @@ public class SearchCluster implements NodeManager<Node> { // 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.getActiveDocuments(), - group.getActiveDocuments()); - trackGroupCoverageChanges(group, sufficientCoverage, group.getActiveDocuments()); + boolean sufficientCoverage = isGroupCoverageSufficient(group.activeDocuments(), + group.activeDocuments()); + trackGroupCoverageChanges(group, sufficientCoverage, group.activeDocuments()); } private void pingIterationCompletedMultipleGroups() { @@ -321,7 +318,7 @@ public class SearchCluster implements NodeManager<Node> { long medianDocuments = medianDocumentsPerGroup(); boolean anyGroupsSufficientCoverage = false; for (Group group : orderedGroups()) { - boolean sufficientCoverage = isGroupCoverageSufficient(group.getActiveDocuments(), + boolean sufficientCoverage = isGroupCoverageSufficient(group.activeDocuments(), medianDocuments); anyGroupsSufficientCoverage = anyGroupsSufficientCoverage || sufficientCoverage; updateSufficientCoverage(group, sufficientCoverage); @@ -331,7 +328,7 @@ public class SearchCluster implements NodeManager<Node> { private long medianDocumentsPerGroup() { if (orderedGroups().isEmpty()) return 0; - var activeDocuments = orderedGroups().stream().map(Group::getActiveDocuments).collect(Collectors.toList()); + var activeDocuments = orderedGroups().stream().map(Group::activeDocuments).collect(Collectors.toList()); return (long)Quantiles.median().compute(activeDocuments); } @@ -357,12 +354,6 @@ public class SearchCluster implements NodeManager<Node> { return true; } - public boolean isGroupWellBalanced(OptionalInt groupId) { - if (groupId.isEmpty()) return false; - Group group = groups().get(groupId.getAsInt()); - return (group != null) && group.isContentWellBalanced(); - } - /** * Calculate whether a subset of nodes in a group has enough coverage */ @@ -375,12 +366,12 @@ public class SearchCluster implements NodeManager<Node> { private void trackGroupCoverageChanges(Group group, boolean fullCoverage, long medianDocuments) { if ( ! hasInformationAboutAllNodes()) return; // Be silent until we know what we are talking about. - boolean changed = group.isFullCoverageStatusChanged(fullCoverage); + boolean changed = group.fullCoverageStatusChanged(fullCoverage); if (changed || (!fullCoverage && System.currentTimeMillis() > nextLogTime)) { nextLogTime = System.currentTimeMillis() + 30 * 1000; if (fullCoverage) { log.info("Cluster " + clusterId + ": " + group + " has full coverage. " + - "Active documents: " + group.getActiveDocuments() + "/" + medianDocuments + ", " + + "Active documents: " + group.activeDocuments() + "/" + medianDocuments + ", " + "working nodes: " + group.workingNodes() + "/" + group.nodes().size()); } else { StringBuilder unresponsive = new StringBuilder(); @@ -389,7 +380,7 @@ public class SearchCluster implements NodeManager<Node> { unresponsive.append('\n').append(node); } log.warning("Cluster " + clusterId + ": " + group + " has reduced coverage: " + - "Active documents: " + group.getActiveDocuments() + "/" + medianDocuments + ", " + + "Active documents: " + group.activeDocuments() + "/" + medianDocuments + ", " + "working nodes: " + group.workingNodes() + "/" + group.nodes().size() + ", unresponsive nodes: " + (unresponsive.toString().isEmpty() ? " none" : unresponsive)); } 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 943390cb10c..be761acf2c2 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 @@ -157,7 +157,6 @@ public class DispatcherTest { @Override public Optional<SearchInvoker> createSearchInvoker(VespaBackEndSearcher searcher, Query query, - OptionalInt groupId, List<Node> nodes, boolean acceptIncompleteCoverage, int maxHitsPerNode) { @@ -167,7 +166,7 @@ public class DispatcherTest { boolean nonEmpty = events[step].returnInvoker(nodes, acceptIncompleteCoverage); step++; if (nonEmpty) { - return Optional.of(new MockInvoker(nodes.get(0).key(), groupId)); + return Optional.of(new MockInvoker(nodes.get(0).key())); } else { return Optional.empty(); } diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/InterleavedSearchInvokerTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/InterleavedSearchInvokerTest.java index 730aa0800e7..21a15165ab3 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/InterleavedSearchInvokerTest.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/InterleavedSearchInvokerTest.java @@ -7,6 +7,8 @@ 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.dispatch.searchcluster.Group; +import com.yahoo.search.dispatch.searchcluster.Node; import com.yahoo.search.dispatch.searchcluster.SearchCluster; import com.yahoo.search.result.Coverage; import com.yahoo.search.result.DefaultErrorHit; @@ -53,7 +55,7 @@ public class InterleavedSearchInvokerTest { @Test public void requireThatAdaptiveTimeoutsAreNotUsedWithFullCoverageRequirement() throws IOException { SearchCluster cluster = new MockSearchCluster("!", createDispatchConfig(100.0), 1, 3); - SearchInvoker invoker = createInterleavedInvoker(cluster, 3); + SearchInvoker invoker = createInterleavedInvoker(cluster, new Group(0, List.of()), 3); expectedEvents.add(new Event(5000, 100, 0)); expectedEvents.add(new Event(4900, 100, 1)); @@ -67,7 +69,7 @@ public class InterleavedSearchInvokerTest { @Test public void requireThatTimeoutsAreNotMarkedAsAdaptive() throws IOException { SearchCluster cluster = new MockSearchCluster("!", createDispatchConfig(100.0), 1, 3); - SearchInvoker invoker = createInterleavedInvoker(cluster, 3); + SearchInvoker invoker = createInterleavedInvoker(cluster, new Group(0, List.of()), 3); expectedEvents.add(new Event(5000, 300, 0)); expectedEvents.add(new Event(4700, 300, 1)); @@ -85,7 +87,7 @@ public class InterleavedSearchInvokerTest { @Test public void requireThatAdaptiveTimeoutDecreasesTimeoutWhenCoverageIsReached() throws IOException { SearchCluster cluster = new MockSearchCluster("!", createDispatchConfig(50.0), 1, 4); - SearchInvoker invoker = createInterleavedInvoker(cluster, 4); + SearchInvoker invoker = createInterleavedInvoker(cluster, new Group(0, List.of()), 4); expectedEvents.add(new Event(5000, 100, 0)); expectedEvents.add(new Event(4900, 100, 1)); @@ -106,7 +108,7 @@ public class InterleavedSearchInvokerTest { SearchCluster cluster = new MockSearchCluster("!", 1, 2); invokers.add(new MockInvoker(0, createCoverage(50155, 50155, 50155, 1, 1, 0))); invokers.add(new MockInvoker(1, createCoverage(49845, 49845, 49845, 1, 1, 0))); - SearchInvoker invoker = createInterleavedInvoker(cluster, 0); + SearchInvoker invoker = createInterleavedInvoker(cluster, new Group(0, List.of()), 0); expectedEvents.add(new Event(null, 100, 0)); expectedEvents.add(new Event(null, 200, 1)); @@ -127,7 +129,7 @@ public class InterleavedSearchInvokerTest { SearchCluster cluster = new MockSearchCluster("!", 1, 2); invokers.add(new MockInvoker(0, createCoverage(10101, 50155, 50155, 1, 1, DEGRADED_BY_MATCH_PHASE))); invokers.add(new MockInvoker(1, createCoverage(13319, 49845, 49845, 1, 1, DEGRADED_BY_MATCH_PHASE))); - SearchInvoker invoker = createInterleavedInvoker(cluster, 0); + SearchInvoker invoker = createInterleavedInvoker(cluster, new Group(0, List.of()), 0); expectedEvents.add(new Event(null, 100, 0)); expectedEvents.add(new Event(null, 200, 1)); @@ -149,7 +151,7 @@ public class InterleavedSearchInvokerTest { SearchCluster cluster = new MockSearchCluster("!", 1, 2); invokers.add(new MockInvoker(0, createCoverage(5000, 50155, 50155, 1, 1, DEGRADED_BY_TIMEOUT))); invokers.add(new MockInvoker(1, createCoverage(4900, 49845, 49845, 1, 1, DEGRADED_BY_TIMEOUT))); - SearchInvoker invoker = createInterleavedInvoker(cluster, 0); + SearchInvoker invoker = createInterleavedInvoker(cluster, new Group(0, List.of()),0); expectedEvents.add(new Event(null, 100, 0)); expectedEvents.add(new Event(null, 200, 1)); @@ -171,7 +173,7 @@ public class InterleavedSearchInvokerTest { SearchCluster cluster = new MockSearchCluster("!", 1, 2); invokers.add(new MockInvoker(0, createCoverage(50155, 50155, 50155, 1, 1, 0))); invokers.add(new MockInvoker(1, createCoverage(49845, 49845, 49845, 1, 1, 0))); - SearchInvoker invoker = createInterleavedInvoker(cluster, 0); + SearchInvoker invoker = createInterleavedInvoker(cluster, new Group(0, List.of()), 0); expectedEvents.add(new Event(null, 100, 0)); expectedEvents.add(null); @@ -205,8 +207,8 @@ public class InterleavedSearchInvokerTest { private static final List<Double> A5Aux = Arrays.asList(-1.0,11.0,8.5,7.5,-7.0,3.0,2.0); private static final List<Double> B5Aux = Arrays.asList(9.0,8.0,-3.0,7.0,6.0,1.0, -1.0); - private void validateThatTopKProbabilityOverrideTakesEffect(Double topKProbability, int expectedK, boolean isContentWellBalanced) throws IOException { - InterleavedSearchInvoker invoker = createInterLeavedTestInvoker(A5, B5, isContentWellBalanced); + private void validateThatTopKProbabilityOverrideTakesEffect(Double topKProbability, int expectedK, Group group) throws IOException { + InterleavedSearchInvoker invoker = createInterLeavedTestInvoker(A5, B5, group); query.setHits(8); query.properties().set(Dispatcher.topKProbability, topKProbability); SearchInvoker [] invokers = invoker.invokers().toArray(new SearchInvoker[0]); @@ -228,17 +230,37 @@ public class InterleavedSearchInvokerTest { @Test public void requireThatTopKProbabilityOverrideTakesEffect() throws IOException { - validateThatTopKProbabilityOverrideTakesEffect(null, 8, true); - validateThatTopKProbabilityOverrideTakesEffect(0.8, 7, true); + validateThatTopKProbabilityOverrideTakesEffect(null, 8, new Group(0, List.of())); + validateThatTopKProbabilityOverrideTakesEffect(0.8, 7, new Group(0, List.of())); } + @Test public void requireThatTopKProbabilityOverrideIsDisabledOnContentSkew() throws IOException { - validateThatTopKProbabilityOverrideTakesEffect(0.8, 8, false); + Node node0 = new Node(0, "host0", 0); + Node node1 = new Node(1, "host1", 0); + Group group = new Group(0, List.of(node0, node1)); + + node0.setActiveDocuments(1000000); + node1.setActiveDocuments(1100000); + group.aggregateNodeValues(); + validateThatTopKProbabilityOverrideTakesEffect(0.8, 8, group); + } + + @Test + public void requireThatTopKProbabilityOverrideIsDisabledOnLittleContent() throws IOException { + Node node0 = new Node(0, "host0", 0); + Node node1 = new Node(1, "host1", 0); + Group group = new Group(0, List.of(node0, node1)); + + node0.setActiveDocuments(10); + node1.setActiveDocuments(10); + group.aggregateNodeValues(); + validateThatTopKProbabilityOverrideTakesEffect(0.8, 8, group); } @Test public void requireThatMergeOfConcreteHitsObeySorting() throws IOException { - InterleavedSearchInvoker invoker = createInterLeavedTestInvoker(A5, B5, true); + InterleavedSearchInvoker invoker = createInterLeavedTestInvoker(A5, B5, new Group(0, List.of())); query.setHits(12); Result result = invoker.search(query, null); assertEquals(10, result.hits().size()); @@ -247,7 +269,7 @@ public class InterleavedSearchInvokerTest { assertEquals(0, result.getQuery().getOffset()); assertEquals(12, result.getQuery().getHits()); - invoker = createInterLeavedTestInvoker(B5, A5, true); + invoker = createInterLeavedTestInvoker(B5, A5, new Group(0, List.of())); result = invoker.search(query, null); assertEquals(10, result.hits().size()); assertEquals(11.0, result.hits().get(0).getRelevance().getScore(), DELTA); @@ -258,7 +280,7 @@ public class InterleavedSearchInvokerTest { @Test public void requireThatMergeOfConcreteHitsObeyOffset() throws IOException { - InterleavedSearchInvoker invoker = createInterLeavedTestInvoker(A5, B5, true); + InterleavedSearchInvoker invoker = createInterLeavedTestInvoker(A5, B5, new Group(0, List.of())); query.setHits(3); query.setOffset(5); Result result = invoker.search(query, null); @@ -268,7 +290,7 @@ public class InterleavedSearchInvokerTest { assertEquals(0, result.getQuery().getOffset()); assertEquals(3, result.getQuery().getHits()); - invoker = createInterLeavedTestInvoker(B5, A5, true); + invoker = createInterLeavedTestInvoker(B5, A5, new Group(0, List.of())); query.setOffset(5); result = invoker.search(query, null); assertEquals(3, result.hits().size()); @@ -280,7 +302,7 @@ public class InterleavedSearchInvokerTest { @Test public void requireThatMergeOfConcreteHitsObeyOffsetWithAuxilliaryStuff() throws IOException { - InterleavedSearchInvoker invoker = createInterLeavedTestInvoker(A5Aux, B5Aux, true); + InterleavedSearchInvoker invoker = createInterLeavedTestInvoker(A5Aux, B5Aux, new Group(0, List.of())); query.setHits(3); query.setOffset(5); Result result = invoker.search(query, null); @@ -291,7 +313,7 @@ public class InterleavedSearchInvokerTest { assertEquals(0, result.getQuery().getOffset()); assertEquals(3, result.getQuery().getHits()); - invoker = createInterLeavedTestInvoker(B5Aux, A5Aux, true); + invoker = createInterLeavedTestInvoker(B5Aux, A5Aux, new Group(0, List.of())); query.setOffset(5); result = invoker.search(query, null); assertEquals(7, result.hits().size()); @@ -302,13 +324,12 @@ public class InterleavedSearchInvokerTest { assertEquals(3, result.getQuery().getHits()); } - private static InterleavedSearchInvoker createInterLeavedTestInvoker(List<Double> a, List<Double> b, - boolean isContentWellBalanced) { + private static InterleavedSearchInvoker createInterLeavedTestInvoker(List<Double> a, List<Double> b, Group group) { SearchCluster cluster = new MockSearchCluster("!", 1, 2); List<SearchInvoker> invokers = new ArrayList<>(); invokers.add(createInvoker(a, 0)); invokers.add(createInvoker(b, 1)); - InterleavedSearchInvoker invoker = new InterleavedSearchInvoker(invokers, isContentWellBalanced, cluster, Collections.emptySet()); + InterleavedSearchInvoker invoker = new InterleavedSearchInvoker(invokers, cluster, group, Collections.emptySet()); invoker.responseAvailable(invokers.get(0)); invoker.responseAvailable(invokers.get(1)); return invoker; @@ -336,7 +357,7 @@ public class InterleavedSearchInvokerTest { Coverage errorCoverage = new Coverage(0, 0, 0); errorCoverage.setNodesTried(1); invokers.add(new SearchErrorInvoker(ErrorMessage.createBackendCommunicationError("node is down"), errorCoverage)); - SearchInvoker invoker = createInterleavedInvoker(cluster, 0); + SearchInvoker invoker = createInterleavedInvoker(cluster, new Group(0, List.of()), 0); expectedEvents.add(new Event(null, 1, 1)); expectedEvents.add(new Event(null, 100, 0)); @@ -354,12 +375,13 @@ public class InterleavedSearchInvokerTest { assertThat(cov.isDegradedByTimeout(), is(true)); } - private InterleavedSearchInvoker createInterleavedInvoker(SearchCluster searchCluster, int numInvokers) { + private InterleavedSearchInvoker createInterleavedInvoker(SearchCluster searchCluster, Group group, int numInvokers) { for (int i = 0; i < numInvokers; i++) { invokers.add(new MockInvoker(i)); } - return new InterleavedSearchInvoker(invokers, false, searchCluster, null) { + return new InterleavedSearchInvoker(invokers, searchCluster, group,null) { + @Override protected long currentTime() { return clock.millis(); diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/MockInvoker.java b/container-search/src/test/java/com/yahoo/search/dispatch/MockInvoker.java index 53d1a2457d0..d86fcdfc25d 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/MockInvoker.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/MockInvoker.java @@ -4,6 +4,7 @@ package com.yahoo.search.dispatch; import com.yahoo.prelude.fastsearch.FastHit; import com.yahoo.search.Query; import com.yahoo.search.Result; +import com.yahoo.search.dispatch.searchcluster.Group; import com.yahoo.search.dispatch.searchcluster.Node; import com.yahoo.search.result.Coverage; import com.yahoo.search.result.Hit; @@ -17,27 +18,17 @@ import java.util.OptionalInt; class MockInvoker extends SearchInvoker { private final Coverage coverage; - private final OptionalInt groupId; private Query query; private List<Hit> hits; int hitsRequested; - protected MockInvoker(int key, Coverage coverage, OptionalInt groupId) { + protected MockInvoker(int key, Coverage coverage) { super(Optional.of(new Node(key, "?", 0))); this.coverage = coverage; - this.groupId = groupId; - } - - protected MockInvoker(int key, OptionalInt groupId) { - this(key, null, groupId); - } - - protected MockInvoker(int key, Coverage coverage) { - this(key, coverage, OptionalInt.empty()); } protected MockInvoker(int key) { - this(key, null, OptionalInt.empty()); + this(key, null); } MockInvoker setHits(List<Hit> hits) { @@ -45,18 +36,15 @@ class MockInvoker extends SearchInvoker { return this; } - /** Returns the group to be invoked, if known */ - public OptionalInt groupId() { return groupId; } - @Override - protected Object sendSearchRequest(Query query, Object context) throws IOException { + protected Object sendSearchRequest(Query query, Object context) { this.query = query; hitsRequested = query.getHits(); return context; } @Override - protected InvokerResult getSearchResult(Execution execution) throws IOException { + protected InvokerResult getSearchResult(Execution execution) { InvokerResult ret = new InvokerResult(query, 10); if (coverage != null) { ret.getResult().setCoverage(coverage); @@ -80,8 +68,7 @@ class MockInvoker extends SearchInvoker { @Override public String toString() { - return "invoker with key " + distributionKey() + - (groupId().isPresent() ? " of group " + groupId().getAsInt() : ""); + return "invoker with key " + distributionKey(); } }
\ No newline at end of file diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterCoverageTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterCoverageTest.java index c9f7469acbb..8101aee74fd 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterCoverageTest.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterCoverageTest.java @@ -101,7 +101,7 @@ public class SearchClusterCoverageTest { } @Test - public void one_group_few_docs_has_well_balanced_content() { + public void one_group_few_docs_unbalanced() { var tester = new SearchClusterTester(1, 2); Node node0 = tester.group(0).nodes().get(0); @@ -115,7 +115,27 @@ public class SearchClusterCoverageTest { node1.setActiveDocuments(0); tester.pingIterationCompleted(); - assertTrue(tester.group(0).isContentWellBalanced()); + assertFalse(tester.group(0).isBalanced()); + assertTrue(tester.group(0).isSparse()); + } + + @Test + public void one_group_many_docs_unbalanced() { + var tester = new SearchClusterTester(1, 2); + + Node node0 = tester.group(0).nodes().get(0); + Node node1 = tester.group(0).nodes().get(1); + + // 1 document + node0.setWorking(true); + node1.setWorking(true); + + node0.setActiveDocuments(1000000); + node1.setActiveDocuments(100000); + + tester.pingIterationCompleted(); + assertFalse(tester.group(0).isBalanced()); + assertFalse(tester.group(0).isSparse()); } } 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 48134094faf..f46717ce180 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 @@ -337,45 +337,45 @@ public class SearchClusterTest { @Test public void requireThatEmptyGroupIsInBalance() { Group group = new Group(0, new ArrayList<>()); - assertTrue(group.isContentWellBalanced()); + assertTrue(group.isBalanced()); group.aggregateNodeValues(); - assertTrue(group.isContentWellBalanced()); + assertTrue(group.isBalanced()); } @Test public void requireThatSingleNodeGroupIsInBalance() { Group group = new Group(0, Arrays.asList(new Node(1, "n", 1))); group.nodes().forEach(node -> node.setWorking(true)); - assertTrue(group.isContentWellBalanced()); + assertTrue(group.isBalanced()); group.aggregateNodeValues(); - assertTrue(group.isContentWellBalanced()); + assertTrue(group.isBalanced()); group.nodes().get(0).setActiveDocuments(1000); group.aggregateNodeValues(); - assertTrue(group.isContentWellBalanced()); + assertTrue(group.isBalanced()); } @Test public void requireThatMultiNodeGroupDetectsBalance() { Group group = new Group(0, Arrays.asList(new Node(1, "n1", 1), new Node(2, "n2", 1))); - assertTrue(group.isContentWellBalanced()); + assertTrue(group.isBalanced()); group.nodes().forEach(node -> node.setWorking(true)); - assertTrue(group.isContentWellBalanced()); + assertTrue(group.isBalanced()); group.aggregateNodeValues(); - assertTrue(group.isContentWellBalanced()); + assertTrue(group.isBalanced()); group.nodes().get(0).setActiveDocuments(1000); group.aggregateNodeValues(); - assertFalse(group.isContentWellBalanced()); + assertFalse(group.isBalanced()); group.nodes().get(1).setActiveDocuments(100); group.aggregateNodeValues(); - assertFalse(group.isContentWellBalanced()); + assertFalse(group.isBalanced()); group.nodes().get(1).setActiveDocuments(800); group.aggregateNodeValues(); - assertFalse(group.isContentWellBalanced()); + assertFalse(group.isBalanced()); group.nodes().get(1).setActiveDocuments(818); group.aggregateNodeValues(); - assertFalse(group.isContentWellBalanced()); + assertFalse(group.isBalanced()); group.nodes().get(1).setActiveDocuments(819); group.aggregateNodeValues(); - assertTrue(group.isContentWellBalanced()); + assertTrue(group.isBalanced()); } } |