From 90b260e83678edc36eb877ec235e0e6ce5892a48 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Thu, 24 Nov 2022 00:00:19 +0100 Subject: Cleanup the concept of orderedGroups. Just use a single way of accessing the groups. Simplify testing by introducing a GroupList to contain all acces to groups and nodes. --- .../java/com/yahoo/search/dispatch/Dispatcher.java | 11 ++- .../com/yahoo/search/dispatch/InvokerFactory.java | 2 +- .../com/yahoo/search/dispatch/LoadBalancer.java | 61 +++++------- .../java/com/yahoo/search/dispatch/SearchPath.java | 24 ++--- .../search/dispatch/searchcluster/GroupList.java | 18 ++++ .../dispatch/searchcluster/GroupListImpl.java | 38 +++++++ .../dispatch/searchcluster/SearchCluster.java | 109 ++++++++++----------- .../prelude/fastsearch/test/MockDispatcher.java | 3 +- .../com/yahoo/search/dispatch/DispatcherTest.java | 8 +- .../yahoo/search/dispatch/LoadBalancerTest.java | 34 +++---- .../yahoo/search/dispatch/MockSearchCluster.java | 84 +--------------- .../com/yahoo/search/dispatch/SearchPathTest.java | 6 +- .../dispatch/searchcluster/SearchClusterTest.java | 9 +- .../searchcluster/SearchClusterTester.java | 6 +- 14 files changed, 187 insertions(+), 226 deletions(-) create mode 100644 container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/GroupList.java create mode 100644 container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/GroupListImpl.java (limited to 'container-search') 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 41f3f5bdead..2c5a9372d06 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 @@ -26,6 +26,7 @@ import com.yahoo.vespa.config.search.DispatchConfig; import com.yahoo.vespa.config.search.DispatchNodesConfig; import java.time.Duration; +import java.util.Collection; import java.util.HashSet; import java.util.List; import java.util.Optional; @@ -83,7 +84,7 @@ public class Dispatcher extends AbstractComponent { private Dispatcher(ComponentId clusterId, DispatchConfig dispatchConfig, DispatchNodesConfig nodesConfig, VipStatus vipStatus, RpcResourcePool resourcePool) { - this(new SearchCluster(clusterId.stringValue(), dispatchConfig, nodesConfig, + this(new SearchCluster(clusterId.stringValue(), dispatchConfig.minActivedocsPercentage(), nodesConfig, vipStatus, new RpcPingFactory(resourcePool)), dispatchConfig, resourcePool); @@ -113,7 +114,7 @@ public class Dispatcher extends AbstractComponent { this.searchCluster = searchCluster; this.clusterMonitor = clusterMonitor; - this.loadBalancer = new LoadBalancer(searchCluster.orderedGroups(), toLoadBalancerPolicy(dispatchConfig.distributionPolicy())); + this.loadBalancer = new LoadBalancer(searchCluster.groups(), toLoadBalancerPolicy(dispatchConfig.distributionPolicy())); this.invokerFactory = invokerFactory; this.rpcResourcePool = rpcResourcePool; this.maxHitsPerNode = dispatchConfig.maxHitsPerNode(); @@ -216,9 +217,9 @@ public class Dispatcher extends AbstractComponent { } int covered = cluster.groupsWithSufficientCoverage(); - int groups = cluster.orderedGroups().size(); + int groups = cluster.numGroups(); int max = Integer.min(Integer.min(covered + 1, groups), MAX_GROUP_SELECTION_ATTEMPTS); - Set rejected = rejectGroupBlockingFeed(cluster.orderedGroups()); + Set rejected = rejectGroupBlockingFeed(cluster.groups()); for (int i = 0; i < max; i++) { Optional groupInCluster = loadBalancer.takeGroup(rejected); if (groupInCluster.isEmpty()) break; // No groups available @@ -254,7 +255,7 @@ public class Dispatcher extends AbstractComponent { * * @return a modifiable set containing the single group to reject, or null otherwise */ - private static Set rejectGroupBlockingFeed(List groups) { + private static Set rejectGroupBlockingFeed(Collection groups) { if (groups.size() == 1) return null; List groupsRejectingFeed = groups.stream().filter(Group::isBlockingWrites).toList(); if (groupsRejectingFeed.size() != 1) return null; 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 4b40dcf6c68..f3d87e04810 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 @@ -57,7 +57,7 @@ public abstract class InvokerFactory { List nodes, boolean acceptIncompleteCoverage, int maxHits) { - Group group = searchCluster.group(nodes.get(0).group()).get(); // Nodes must be of the same group + Group group = searchCluster.group(nodes.get(0).group()); // Nodes must be of the same group List invokers = new ArrayList<>(nodes.size()); Set failed = null; for (Node node : nodes) { 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 52cc6ad7711..edf9de4f135 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 @@ -5,7 +5,10 @@ import com.yahoo.search.dispatch.searchcluster.Group; import java.time.Duration; import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Random; import java.util.Set; @@ -28,15 +31,15 @@ public class LoadBalancer { private static final Duration INITIAL_QUERY_TIME = Duration.ofMillis(1); private static final double MIN_QUERY_TIME = Duration.ofMillis(1).toMillis()/1000.0; - private final List scoreboard; + private final Map scoreboard; private final GroupScheduler scheduler; public enum Policy { ROUNDROBIN, LATENCY_AMORTIZED_OVER_REQUESTS, LATENCY_AMORTIZED_OVER_TIME, BEST_OF_RANDOM_2} - public LoadBalancer(List groups, Policy policy) { - this.scoreboard = new ArrayList<>(groups.size()); + public LoadBalancer(Collection groups, Policy policy) { + this.scoreboard = new HashMap<>(); for (Group group : groups) { - scoreboard.add(new GroupStatus(group)); + scoreboard.put(group.id(), new GroupStatus(group)); } if (scoreboard.size() == 1) policy = Policy.ROUNDROBIN; @@ -81,12 +84,8 @@ public class LoadBalancer { */ public void releaseGroup(Group group, boolean success, RequestDuration searchTime) { synchronized (this) { - for (GroupStatus sched : scoreboard) { - if (sched.group.id() == group.id()) { - sched.release(success, searchTime); - break; - } - } + GroupStatus sched = scoreboard.get(group.id()); + sched.release(success, searchTime); } } @@ -146,31 +145,29 @@ public class LoadBalancer { private static class RoundRobinScheduler implements GroupScheduler { private int needle = 0; - private final List scoreboard; + private final Map scoreboard; - public RoundRobinScheduler(List scoreboard) { + public RoundRobinScheduler(Map scoreboard) { this.scoreboard = scoreboard; } @Override public Optional takeNextGroup(Set rejectedGroups) { GroupStatus bestCandidate = null; - int bestIndex = needle; - int index = needle; + int groupId = needle; for (int i = 0; i < scoreboard.size(); i++) { - GroupStatus candidate = scoreboard.get(index); - if (rejectedGroups == null || !rejectedGroups.contains(candidate.group.id())) { + GroupStatus candidate = scoreboard.get(groupId); + if (rejectedGroups == null || !rejectedGroups.contains(candidate.groupId())) { GroupStatus better = betterGroup(bestCandidate, candidate); if (better == candidate) { bestCandidate = candidate; - bestIndex = index; } } - index = nextScoreboardIndex(index); + groupId = nextScoreboardIndex(groupId); } - needle = nextScoreboardIndex(bestIndex); - return Optional.ofNullable(bestCandidate); + needle = nextScoreboardIndex(bestCandidate.groupId()); + return Optional.of(bestCandidate); } /** @@ -203,7 +200,7 @@ public class LoadBalancer { static class AdaptiveScheduler implements GroupScheduler { enum Type {TIME, REQUESTS} private final Random random; - private final List scoreboard; + private final Map scoreboard; private static double toDouble(Duration duration) { return duration.toNanos()/1_000_000_000.0; @@ -250,18 +247,16 @@ public class LoadBalancer { Duration averageSearchTime() { return fromDouble(averageSearchTime);} } - public AdaptiveScheduler(Type type, Random random, List scoreboard) { + public AdaptiveScheduler(Type type, Random random, Map scoreboard) { this.random = random; this.scoreboard = scoreboard; - for (GroupStatus gs : scoreboard) { - gs.setDecayer(type == Type.REQUESTS ? new DecayByRequests() : new DecayByTime()); - } + scoreboard.forEach((id, gs) -> gs.setDecayer(type == Type.REQUESTS ? new DecayByRequests() : new DecayByTime())); } private Optional selectGroup(double needle, boolean requireCoverage, Set rejected) { double sum = 0; int n = 0; - for (GroupStatus gs : scoreboard) { + for (GroupStatus gs : scoreboard.values()) { if (rejected == null || !rejected.contains(gs.group.id())) { if (!requireCoverage || gs.group.hasSufficientCoverage()) { sum += gs.weight(); @@ -273,7 +268,7 @@ public class LoadBalancer { return Optional.empty(); } double accum = 0; - for (GroupStatus gs : scoreboard) { + for (GroupStatus gs : scoreboard.values()) { if (rejected == null || !rejected.contains(gs.group.id())) { if (!requireCoverage || gs.group.hasSufficientCoverage()) { accum += gs.weight(); @@ -297,8 +292,8 @@ public class LoadBalancer { static class BestOfRandom2 implements GroupScheduler { private final Random random; - private final List scoreboard; - public BestOfRandom2(Random random, List scoreboard) { + private final Map scoreboard; + public BestOfRandom2(Random random, Map scoreboard) { this.random = random; this.scoreboard = scoreboard; } @@ -312,11 +307,10 @@ public class LoadBalancer { private GroupStatus selectBestOf2(Set rejectedGroups, boolean requireCoverage) { List candidates = new ArrayList<>(scoreboard.size()); - for (int i=0; i < scoreboard.size(); i++) { - GroupStatus gs = scoreboard.get(i); + for (GroupStatus gs : scoreboard.values()) { if (rejectedGroups == null || !rejectedGroups.contains(gs.group.id())) { if (!requireCoverage || gs.group.hasSufficientCoverage()) { - candidates.add(i); + candidates.add(gs.groupId()); } } } @@ -330,8 +324,7 @@ public class LoadBalancer { private GroupStatus selectRandom(List candidates) { if ( ! candidates.isEmpty()) { int index = random.nextInt(candidates.size()); - Integer groupIndex = candidates.remove(index); - return scoreboard.get(groupIndex); + return scoreboard.get(candidates.remove(index)); } return null; } 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 fc6d5ed01b1..10bf5e36988 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 @@ -3,8 +3,8 @@ package com.yahoo.search.dispatch; import com.yahoo.collections.Pair; import com.yahoo.search.dispatch.searchcluster.Group; +import com.yahoo.search.dispatch.searchcluster.GroupList; import com.yahoo.search.dispatch.searchcluster.Node; -import com.yahoo.search.dispatch.searchcluster.SearchCluster; import java.util.ArrayList; import java.util.Collection; @@ -36,7 +36,7 @@ public class SearchPath { * @return list of nodes chosen with the search path, or an empty list in which * case some other node selection logic should be used */ - public static List selectNodes(String searchPath, SearchCluster cluster) { + public static List selectNodes(String searchPath, GroupList cluster) { Optional sp = SearchPath.fromString(searchPath); if (sp.isPresent()) { return sp.get().mapToNodes(cluster); @@ -73,8 +73,8 @@ public class SearchPath { this.groups = groups; } - private List mapToNodes(SearchCluster cluster) { - if (cluster.groups().isEmpty()) { + private List mapToNodes(GroupList cluster) { + if (cluster.isEmpty()) { return List.of(); } @@ -100,14 +100,14 @@ public class SearchPath { return nodes.isEmpty() && groups.isEmpty(); } - private Group selectRandomGroupWithSufficientCoverage(SearchCluster cluster, List groupIds) { + private Group selectRandomGroupWithSufficientCoverage(GroupList cluster, List groupIds) { while ( groupIds.size() > 1 ) { int index = random.nextInt(groupIds.size()); int groupId = groupIds.get(index); - Optional group = cluster.group(groupId); - if (group.isPresent()) { - if (group.get().hasSufficientCoverage()) { - return group.get(); + Group group = cluster.group(groupId); + if (group != null) { + if (group.hasSufficientCoverage()) { + return group; } else { groupIds.remove(index); } @@ -115,10 +115,10 @@ public class SearchPath { throw new InvalidSearchPathException("Invalid searchPath, cluster does not have " + (groupId + 1) + " groups"); } } - return cluster.group(groupIds.get(0)).get(); + return cluster.group(groupIds.get(0)); } - private Group selectGroup(SearchCluster cluster) { + private Group selectGroup(GroupList cluster) { if ( ! groups.isEmpty()) { List potentialGroups = new ArrayList<>(); for (Selection groupSelection : groups) { @@ -130,7 +130,7 @@ public class SearchPath { } // pick any working group - return selectRandomGroupWithSufficientCoverage(cluster, new ArrayList<>(cluster.groups().keySet())); + return selectRandomGroupWithSufficientCoverage(cluster, new ArrayList<>(cluster.groupKeys())); } private static SearchPath parseElement(String element) { diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/GroupList.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/GroupList.java new file mode 100644 index 00000000000..7d625f55cb6 --- /dev/null +++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/GroupList.java @@ -0,0 +1,18 @@ +package com.yahoo.search.dispatch.searchcluster; + +import java.util.Collection; +import java.util.Set; + +/** + * Simple interface for groups and their nodes in the content cluster + * @author baldersheim + */ +public interface GroupList { + Group group(int id); + Set groupKeys(); + Collection groups(); + default boolean isEmpty() { + return numGroups() == 0; + } + int numGroups(); +} diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/GroupListImpl.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/GroupListImpl.java new file mode 100644 index 00000000000..b0cbf7d1d1d --- /dev/null +++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/GroupListImpl.java @@ -0,0 +1,38 @@ +package com.yahoo.search.dispatch.searchcluster; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; + +public class GroupListImpl implements GroupList { + private final Map groups; + public GroupListImpl(Map groups) { + this.groups = Map.copyOf(groups); + } + @Override public Group group(int id) { return groups.get(id); } + @Override public Set groupKeys() { return groups.keySet();} + @Override public Collection groups() { return groups.values(); } + @Override public int numGroups() { return groups.size(); } + public static GroupList buildGroupListForTest(int numGroups, int nodesPerGroup) { + return new GroupListImpl(buildGroupMapForTest(numGroups, nodesPerGroup)); + } + public static Map buildGroupMapForTest(int numGroups, int nodesPerGroup) { + Map groups = new HashMap<>(); + int distributionKey = 0; + for (int group = 0; group < numGroups; group++) { + List groupNodes = new ArrayList<>(); + for (int i = 0; i < nodesPerGroup; i++) { + Node node = new Node(distributionKey, "host" + distributionKey, group); + node.setWorking(true); + groupNodes.add(node); + distributionKey++; + } + Group g = new Group(group, groupNodes); + groups.put(group, g); + } + return Map.copyOf(groups); + } +} 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 ca2fce0b32b..bedfa965229 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 @@ -8,13 +8,13 @@ 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.vespa.config.search.DispatchConfig; import com.yahoo.vespa.config.search.DispatchNodesConfig; -import java.util.LinkedHashMap; +import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.concurrent.Executor; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -24,7 +24,7 @@ import java.util.stream.Collectors; * * @author bratseth */ -public class SearchCluster implements NodeManager { +public class SearchCluster implements NodeManager, GroupList { private static final Logger log = Logger.getLogger(SearchCluster.class.getName()); @@ -32,9 +32,7 @@ public class SearchCluster implements NodeManager { private final String clusterId; private final VipStatus vipStatus; private final PingFactory pingFactory; - private final Map groups; - private final List orderedGroups; - private final List nodes; + private final GroupList groups; private long nextLogTime = 0; /** @@ -47,55 +45,48 @@ public class SearchCluster implements NodeManager { */ private final Node localCorpusDispatchTarget; - public SearchCluster(String clusterId, DispatchConfig dispatchConfig, + public SearchCluster(String clusterId, double minActivedocsPercentage, DispatchNodesConfig nodesConfig, VipStatus vipStatus, PingFactory pingFactory) { + this(clusterId, minActivedocsPercentage, toNodes(nodesConfig), vipStatus, pingFactory); + } + public SearchCluster(String clusterId, double minActivedocsPercentage, List nodes, + VipStatus vipStatus, PingFactory pingFactory) { + this(clusterId, minActivedocsPercentage, toGroups(nodes), vipStatus, pingFactory); + } + public SearchCluster(String clusterId, double minActivedocsPercentage, GroupList groups, + VipStatus vipStatus, PingFactory pingFactory) { this.clusterId = clusterId; - this.minActivedocsPercentage = dispatchConfig.minActivedocsPercentage(); + this.minActivedocsPercentage = minActivedocsPercentage; this.vipStatus = vipStatus; this.pingFactory = pingFactory; - - this.nodes = toNodes(nodesConfig); - - // Create groups - ImmutableMap.Builder groupsBuilder = new ImmutableMap.Builder<>(); - for (Map.Entry> group : nodes.stream().collect(Collectors.groupingBy(Node::group)).entrySet()) { - Group g = new Group(group.getKey(), group.getValue()); - groupsBuilder.put(group.getKey(), g); - } - this.groups = groupsBuilder.build(); - LinkedHashMap groupIntroductionOrder = new LinkedHashMap<>(); - nodes.forEach(node -> groupIntroductionOrder.put(node.group(), groups.get(node.group()))); - this.orderedGroups = List.copyOf(groupIntroductionOrder.values()); - - this.localCorpusDispatchTarget = findLocalCorpusDispatchTarget(HostName.getLocalhost(), nodes, groups); + this.groups = groups; + this.localCorpusDispatchTarget = findLocalCorpusDispatchTarget(HostName.getLocalhost(), groups); } @Override public String name() { return clusterId; } public void addMonitoring(ClusterMonitor clusterMonitor) { - for (var group : orderedGroups()) { + for (var group : groups()) { for (var node : group.nodes()) clusterMonitor.add(node, true); } } - private static Node findLocalCorpusDispatchTarget(String selfHostname, - List nodes, - Map groups) { + private static Node findLocalCorpusDispatchTarget(String selfHostname, GroupList 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. - List localSearchNodes = nodes.stream() + List localSearchNodes = groups.groups().stream().flatMap(g -> g.nodes().stream()) .filter(node -> node.hostname().equals(selfHostname)) .toList(); // Only use direct dispatch if we have exactly 1 search node on the same machine: if (localSearchNodes.size() != 1) return null; Node localSearchNode = localSearchNodes.iterator().next(); - Group localSearchGroup = groups.get(localSearchNode.group()); + Group localSearchGroup = groups.group(localSearchNode.group()); // Only use direct dispatch if the local search node has the entire corpus if (localSearchGroup.nodes().size() != 1) return null; @@ -109,27 +100,30 @@ public class SearchCluster implements NodeManager { .toList(); } - /** Returns the groups of this cluster as an immutable map indexed by group id */ - public Map groups() { return groups; } - - /** Returns the groups of this cluster as an immutable list in introduction order */ - public List orderedGroups() { return orderedGroups; } - - /** Returns the n'th (zero-indexed) group in the cluster if possible */ - public Optional group(int n) { - if (orderedGroups().size() > n) { - return Optional.of(orderedGroups().get(n)); - } else { - return Optional.empty(); + private static GroupList toGroups(Collection nodes) { + ImmutableMap.Builder groupsBuilder = new ImmutableMap.Builder<>(); + for (Map.Entry> group : nodes.stream().collect(Collectors.groupingBy(Node::group)).entrySet()) { + Group g = new Group(group.getKey(), group.getValue()); + groupsBuilder.put(group.getKey(), g); } + return new GroupListImpl(groupsBuilder.build()); } - public boolean allGroupsHaveSize1() { - return nodes.size() == groups.size(); + @Override + public Group group(int id) { + return groups.group(id); } + @Override + public Set groupKeys() { return groups.groupKeys(); } + @Override + public Collection groups() { return groups.groups(); } + @Override + public int numGroups() { return groups.numGroups(); } + + public boolean allGroupsHaveSize1() { return groups().stream().allMatch(g -> g.nodes().size() == 1); } public int groupsWithSufficientCoverage() { - return (int)groups.values().stream().filter(g -> g.hasSufficientCoverage()).count(); + return (int)groups().stream().filter(Group::hasSufficientCoverage).count(); } /** @@ -140,7 +134,7 @@ public class SearchCluster implements NodeManager { if ( localCorpusDispatchTarget == null) return Optional.empty(); // Only use direct dispatch if the local group has sufficient coverage - Group localSearchGroup = groups().get(localCorpusDispatchTarget.group()); + Group localSearchGroup = group(localCorpusDispatchTarget.group()); if ( ! localSearchGroup.hasSufficientCoverage()) return Optional.empty(); // Only use direct dispatch if the local search node is not down @@ -181,7 +175,7 @@ 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 set back in service - if (nodeIsWorking || nodes.size() > 1) + if (nodeIsWorking || groups().stream().map(Group::nodes).count() > 1) setInRotationOnlyIf(nodeIsWorking); } } @@ -203,11 +197,11 @@ public class SearchCluster implements NodeManager { } public boolean hasInformationAboutAllNodes() { - return nodes.stream().allMatch(node -> node.isWorking() != null); + return groups().stream().allMatch(g -> g.nodes().stream().allMatch(node -> node.isWorking() != null)); } private boolean hasWorkingNodes() { - return nodes.stream().anyMatch(node -> node.isWorking() != Boolean.FALSE ); + return groups().stream().anyMatch(g -> g.nodes().stream().anyMatch(node -> node.isWorking() != Boolean.FALSE)); } private boolean usesLocalCorpusIn(Node node) { @@ -226,7 +220,7 @@ public class SearchCluster implements NodeManager { } private void pingIterationCompletedSingleGroup() { - Group group = groups().values().iterator().next(); + Group group = groups().iterator().next(); group.aggregateNodeValues(); // With just one group sufficient coverage may not be the same as full coverage, as the // group will always be marked sufficient for use. @@ -236,9 +230,9 @@ public class SearchCluster implements NodeManager { } private void pingIterationCompletedMultipleGroups() { - orderedGroups().forEach(Group::aggregateNodeValues); + groups().forEach(Group::aggregateNodeValues); long medianDocuments = medianDocumentsPerGroup(); - for (Group group : orderedGroups()) { + for (Group group : groups()) { boolean sufficientCoverage = isGroupCoverageSufficient(group.activeDocuments(), medianDocuments); updateSufficientCoverage(group, sufficientCoverage); trackGroupCoverageChanges(group, sufficientCoverage, medianDocuments); @@ -246,8 +240,8 @@ public class SearchCluster implements NodeManager { } private long medianDocumentsPerGroup() { - if (orderedGroups().isEmpty()) return 0; - var activeDocuments = orderedGroups().stream().map(Group::activeDocuments).collect(Collectors.toList()); + if (isEmpty()) return 0; + var activeDocuments = groups().stream().map(Group::activeDocuments).collect(Collectors.toList()); return (long)Quantiles.median().compute(activeDocuments); } @@ -258,8 +252,7 @@ public class SearchCluster implements NodeManager { */ @Override public void pingIterationCompleted() { - int numGroups = orderedGroups().size(); - if (numGroups == 1) { + if (numGroups() == 1) { pingIterationCompletedSingleGroup(); } else { pingIterationCompletedMultipleGroups(); @@ -268,16 +261,14 @@ public class SearchCluster implements NodeManager { private boolean isGroupCoverageSufficient(long activeDocuments, long medianDocuments) { double documentCoverage = 100.0 * (double) activeDocuments / medianDocuments; - if (medianDocuments > 0 && documentCoverage < minActivedocsPercentage) - return false; - return true; + return ! (medianDocuments > 0 && documentCoverage < minActivedocsPercentage); } /** * Calculate whether a subset of nodes in a group has enough coverage */ public boolean isPartialGroupCoverageSufficient(List nodes) { - if (orderedGroups().size() == 1) + if (numGroups() == 1) return true; long activeDocuments = nodes.stream().mapToLong(Node::getActiveDocuments).sum(); return isGroupCoverageSufficient(activeDocuments, medianDocumentsPerGroup()); 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 a444159952d..886c9818842 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 @@ -26,8 +26,7 @@ class MockDispatcher extends Dispatcher { public static MockDispatcher create(List nodes, RpcResourcePool rpcResourcePool, VipStatus vipStatus) { var dispatchConfig = toDispatchConfig(); - var nodesConfig = toNodesConfig(nodes); - var searchCluster = new SearchCluster("a", dispatchConfig, nodesConfig, vipStatus, new RpcPingFactory(rpcResourcePool)); + var searchCluster = new SearchCluster("a", dispatchConfig.minActivedocsPercentage(), nodes, vipStatus, new RpcPingFactory(rpcResourcePool)); return new MockDispatcher(new ClusterMonitor<>(searchCluster, true), searchCluster, dispatchConfig, rpcResourcePool); } 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 bc92afdb8fc..3c5b026b95e 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 @@ -110,7 +110,7 @@ public class DispatcherTest { void testGroup0IsSkippedWhenItIsBlockingFeed() { SearchCluster cluster = new MockSearchCluster("1", 3, 1); Dispatcher dispatcher = new Dispatcher(new ClusterMonitor<>(cluster, false), cluster, dispatchConfig, new MockInvokerFactory(cluster, dispatchConfig, (n, a) -> true)); - cluster.group(0).get().nodes().get(0).setBlockingWrites(true); + cluster.group(0).nodes().get(0).setBlockingWrites(true); cluster.pingIterationCompleted(); assertEquals(1, (dispatcher.getSearchInvoker(new Query(), null).distributionKey().get()).longValue(), @@ -122,8 +122,8 @@ public class DispatcherTest { void testGroup0IsSelectedWhenMoreAreBlockingFeed() { SearchCluster cluster = new MockSearchCluster("1", 3, 1); Dispatcher dispatcher = new Dispatcher(new ClusterMonitor<>(cluster, false), cluster, dispatchConfig, new MockInvokerFactory(cluster, dispatchConfig, (n, a) -> true)); - cluster.group(0).get().nodes().get(0).setBlockingWrites(true); - cluster.group(1).get().nodes().get(0).setBlockingWrites(true); + cluster.group(0).nodes().get(0).setBlockingWrites(true); + cluster.group(1).nodes().get(0).setBlockingWrites(true); cluster.pingIterationCompleted(); assertEquals(0, dispatcher.getSearchInvoker(new Query(), null).distributionKey().get().longValue(), @@ -135,7 +135,7 @@ public class DispatcherTest { void testGroup0IsSelectedWhenItIsBlockingFeedWhenNoOthers() { SearchCluster cluster = new MockSearchCluster("1", 1, 1); Dispatcher dispatcher = new Dispatcher(new ClusterMonitor<>(cluster, false), cluster, dispatchConfig, new MockInvokerFactory(cluster, dispatchConfig, (n, a) -> true)); - cluster.group(0).get().nodes().get(0).setBlockingWrites(true); + cluster.group(0).nodes().get(0).setBlockingWrites(true); cluster.pingIterationCompleted(); assertEquals(0, (dispatcher.getSearchInvoker(new Query(), null).distributionKey().get()).longValue(), 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 40711fb8dbe..62b56e6e8ff 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 @@ -11,9 +11,10 @@ import org.opentest4j.AssertionFailedError; import java.time.Duration; import java.time.Instant; -import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Random; @@ -110,14 +111,19 @@ public class LoadBalancerTest { assertEquals(Duration.ofNanos(1045087), decayer.averageSearchTime()); } + private Map createScoreBoard(int count) { + Map scoreboard = new HashMap<>(); + for (int i = 0; i < count; i++) { + GroupStatus gs = newGroupStatus(i); + scoreboard.put(gs.groupId(), gs); + } + return scoreboard; + } + @Test void requireEqualDistributionInFlatWeightListWithAdaptiveScheduler() { - List scoreboard = new ArrayList<>(); - for (int i = 0; i < 5; i++) { - scoreboard.add(newGroupStatus(i)); - } Random seq = sequence(0.0, 0.1, 0.2, 0.39, 0.4, 0.6, 0.8, 0.99999); - AdaptiveScheduler sched = new AdaptiveScheduler(AdaptiveScheduler.Type.REQUESTS, seq, scoreboard); + AdaptiveScheduler sched = new AdaptiveScheduler(AdaptiveScheduler.Type.REQUESTS, seq, createScoreBoard(5)); assertEquals(0, sched.takeNextGroup(null).get().groupId()); assertEquals(0, sched.takeNextGroup(null).get().groupId()); @@ -131,15 +137,11 @@ public class LoadBalancerTest { @Test void requireThatAdaptiveSchedulerObeysWeights() { - List scoreboard = new ArrayList<>(); - for (int i = 0; i < 5; i++) { - GroupStatus gs = newGroupStatus(i); - scoreboard.add(gs); - } + var scoreboard = createScoreBoard(5); Random seq = sequence(0.0, 0.4379, 0.4380, 0.6569, 0.6570, 0.8029, 0.8030, 0.9124, 0.9125); AdaptiveScheduler sched = new AdaptiveScheduler(AdaptiveScheduler.Type.REQUESTS, seq, scoreboard); - int i= 0; - for (GroupStatus gs : scoreboard) { + int i = 0; + for (GroupStatus gs : scoreboard.values()) { gs.setDecayer(new AdaptiveScheduler.DecayByRequests(1, Duration.ofMillis((long)(0.1 * (i + 1)*1000.0)))); i++; } @@ -161,10 +163,6 @@ public class LoadBalancerTest { } @Test void requireBestOfRandom2Scheduler() { - List scoreboard = new ArrayList<>(); - for (int i = 0; i < 5; i++) { - scoreboard.add(newGroupStatus(i)); - } Random seq = sequence( 0.1, 0.125, 0.1, 0.125, @@ -175,7 +173,7 @@ public class LoadBalancerTest { 0.9, 0.125, 0.9, 0.125 ); - BestOfRandom2 sched = new BestOfRandom2(seq, scoreboard); + BestOfRandom2 sched = new BestOfRandom2(seq, createScoreBoard(5)); assertEquals(0, allocate(sched.takeNextGroup(null).get()).groupId()); assertEquals(1, allocate(sched.takeNextGroup(null).get()).groupId()); 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 32ca63693b4..9a4931a8fa7 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 @@ -1,91 +1,25 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.dispatch; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableMultimap; -import com.yahoo.search.dispatch.searchcluster.Group; +import com.yahoo.search.dispatch.searchcluster.GroupListImpl; import com.yahoo.search.dispatch.searchcluster.Node; import com.yahoo.search.dispatch.searchcluster.SearchCluster; import com.yahoo.vespa.config.search.DispatchConfig; -import com.yahoo.vespa.config.search.DispatchNodesConfig; -import java.util.ArrayList; import java.util.List; -import java.util.Optional; /** * @author ollivir */ public class MockSearchCluster extends SearchCluster { - private final int numGroups; - 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(), createNodesConfig(), groups, nodesPerGroup); + super(clusterId, 88.0, GroupListImpl.buildGroupListForTest(groups, nodesPerGroup), null, null); } - public MockSearchCluster(String clusterId, DispatchConfig dispatchConfig, DispatchNodesConfig nodesConfig, int groups, int nodesPerGroup) { - super(clusterId, dispatchConfig, nodesConfig, null, null); - - ImmutableList.Builder orderedGroupBuilder = ImmutableList.builder(); - 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 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, groupNodes); - groupBuilder.put(group, g); - orderedGroupBuilder.add(g); - } - this.orderedGroups = orderedGroupBuilder.build(); - this.groups = groupBuilder.build(); - this.numGroups = groups; - this.numNodesPerGroup = nodesPerGroup; - } - - @Override - public ImmutableList orderedGroups() { - return orderedGroups; - } - - @Override - public ImmutableMap groups() { - return groups; - } - - @Override - public boolean allGroupsHaveSize1() { return numNodesPerGroup == 1;} - @Override public int groupsWithSufficientCoverage() { - return numGroups; - } - - @Override - public Optional group(int n) { - if (n < numGroups) { - return Optional.of(groups.get(n)); - } else { - return Optional.empty(); - } - } - - @Override - public Optional localCorpusDispatchTarget() { - return Optional.empty(); + return numGroups(); } @Override @@ -101,9 +35,6 @@ public class MockSearchCluster extends SearchCluster { public static DispatchConfig createDispatchConfig() { return createDispatchConfig(100.0); } - public static DispatchNodesConfig createNodesConfig(Node... nodes) { - return createNodesConfig(List.of(nodes)).build(); - } public static DispatchConfig createDispatchConfig(double minSearchCoverage) { return createDispatchConfigBuilder(minSearchCoverage).build(); @@ -121,13 +52,4 @@ public class MockSearchCluster extends SearchCluster { return builder; } - public static DispatchNodesConfig.Builder createNodesConfig(List nodes) { - DispatchNodesConfig.Builder builder = new DispatchNodesConfig.Builder(); - int port = 10000; - for (Node n : nodes) { - builder.node(new DispatchNodesConfig.Node.Builder().key(n.key()).host(n.hostname()).port(port++).group(n.group())); - } - return builder; - } - } diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/SearchPathTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/SearchPathTest.java index 12cbe488485..fcd587e19f7 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/SearchPathTest.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/SearchPathTest.java @@ -2,6 +2,8 @@ package com.yahoo.search.dispatch; import com.yahoo.search.dispatch.SearchPath.InvalidSearchPathException; +import com.yahoo.search.dispatch.searchcluster.GroupList; +import com.yahoo.search.dispatch.searchcluster.GroupListImpl; import com.yahoo.search.dispatch.searchcluster.Node; import org.junit.jupiter.api.Test; @@ -81,7 +83,7 @@ public class SearchPathTest { } } - private void verifyRandomGroup(MockSearchCluster cluster, String searchPath, Set possibleSolutions) { + private void verifyRandomGroup(GroupList cluster, String searchPath, Set possibleSolutions) { for (int i=0; i < 100; i++) { String nodes = distKeysAsString(SearchPath.selectNodes(searchPath, cluster)); assertTrue(possibleSolutions.contains(nodes)); @@ -90,7 +92,7 @@ public class SearchPathTest { @Test void searchPathMustFilterNodesBasedOnDefinition() { - MockSearchCluster cluster = new MockSearchCluster("a", 3, 3); + GroupList cluster = GroupListImpl.buildGroupListForTest(3, 3); assertEquals(distKeysAsString(SearchPath.selectNodes("1/1", cluster)), "4"); assertEquals(distKeysAsString(SearchPath.selectNodes("/1", cluster)), "3,4,5"); 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 283ef29c878..ddb0b9fa809 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 @@ -7,7 +7,6 @@ import com.yahoo.container.handler.VipStatus; import com.yahoo.net.HostName; import com.yahoo.prelude.Pong; import com.yahoo.search.cluster.ClusterMonitor; -import com.yahoo.search.dispatch.MockSearchCluster; import com.yahoo.search.result.ErrorMessage; import org.junit.jupiter.api.Test; @@ -56,7 +55,7 @@ public class SearchClusterTest { numDocsPerNode.add(new AtomicInteger(1)); pingCounts.add(new AtomicInteger(0)); } - searchCluster = new SearchCluster(clusterId, MockSearchCluster.createDispatchConfig(), MockSearchCluster.createNodesConfig(nodes).build(), + searchCluster = new SearchCluster(clusterId, 100.0, nodes, vipStatus, new Factory(nodesPerGroup, numDocsPerNode, pingCounts)); clusterMonitor = new ClusterMonitor(searchCluster, false); searchCluster.addMonitoring(clusterMonitor); @@ -334,7 +333,7 @@ public class SearchClusterTest { @Test void requireThatEmptyGroupIsInBalance() { - Group group = new Group(0, new ArrayList<>()); + Group group = new Group(0, List.of()); assertTrue(group.isBalanced()); group.aggregateNodeValues(); assertTrue(group.isBalanced()); @@ -342,7 +341,7 @@ public class SearchClusterTest { @Test void requireThatSingleNodeGroupIsInBalance() { - Group group = new Group(0, Arrays.asList(new Node(1, "n", 1))); + Group group = new Group(0, List.of(new Node(1, "n", 1))); group.nodes().forEach(node -> node.setWorking(true)); assertTrue(group.isBalanced()); group.aggregateNodeValues(); @@ -354,7 +353,7 @@ public class SearchClusterTest { @Test void requireThatMultiNodeGroupDetectsBalance() { - Group group = new Group(0, Arrays.asList(new Node(1, "n1", 1), new Node(2, "n2", 1))); + Group group = new Group(0, List.of(new Node(1, "n1", 1), new Node(2, "n2", 1))); assertTrue(group.isBalanced()); group.nodes().forEach(node -> node.setWorking(true)); assertTrue(group.isBalanced()); diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTester.java b/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTester.java index b5816d0c4f2..f5a9256648d 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTester.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTester.java @@ -16,15 +16,15 @@ public class SearchClusterTester { } public Group group(int id) { - return cluster.group(id).get(); + return cluster.group(id); } public void setWorking(int group, int node, boolean working) { - cluster.group(group).get().nodes().get(node).setWorking(working); + cluster.group(group).nodes().get(node).setWorking(working); } public void setDocsPerNode(int docs, int groupId) { - for (Node node : cluster.groups().get(groupId).nodes()) { + for (Node node : cluster.group(groupId).nodes()) { node.setWorking(true); node.setActiveDocuments(docs); } -- cgit v1.2.3