diff options
author | Jon Bratseth <bratseth@oath.com> | 2019-02-11 09:56:08 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-02-11 09:56:08 +0100 |
commit | 426cb815a2f9e9fbc1cb148d445259c575821d84 (patch) | |
tree | d4286f9280d5b8b4d09f17b5133989fb63a62df9 | |
parent | 087af5baf3342160a3ae4588688fb9494e1026d2 (diff) | |
parent | d6a54d3cdd79f368f1de07ec275b20c30c983444 (diff) |
Merge pull request #8414 from vespa-engine/ollivir/java-dispatcher-bugfixes
Java dispatcher bugfixes
4 files changed, 27 insertions, 13 deletions
diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4InvokerFactory.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4InvokerFactory.java index 612941ece7a..5700e316493 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4InvokerFactory.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4InvokerFactory.java @@ -24,6 +24,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.OptionalInt; import java.util.Set; /** @@ -69,7 +70,7 @@ public class FS4InvokerFactory { * @return Optional containing the SearchInvoker or <i>empty</i> if some node in the * list is invalid and the remaining coverage is not sufficient */ - public Optional<SearchInvoker> getSearchInvoker(Query query, int groupId, List<Node> nodes, boolean acceptIncompleteCoverage) { + public Optional<SearchInvoker> getSearchInvoker(Query query, OptionalInt groupId, List<Node> nodes, boolean acceptIncompleteCoverage) { List<SearchInvoker> invokers = new ArrayList<>(nodes.size()); Set<Integer> failed = null; for (Node node : nodes) { 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 1d39cffa9d2..146b132be22 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 @@ -21,6 +21,7 @@ import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Optional; +import java.util.OptionalInt; import java.util.Set; /** @@ -106,7 +107,7 @@ public class Dispatcher extends AbstractComponent { @FunctionalInterface private interface SearchInvokerSupplier { - Optional<SearchInvoker> supply(Query query, int groupId, List<Node> nodes, boolean acceptIncompleteCoverage); + Optional<SearchInvoker> supply(Query query, OptionalInt groupId, List<Node> nodes, boolean acceptIncompleteCoverage); } // build invoker based on searchpath @@ -121,7 +122,7 @@ public class Dispatcher extends AbstractComponent { return Optional.empty(); } else { query.trace(false, 2, "Dispatching internally with search path ", searchPath); - return invokerFactory.supply(query, -1, nodes, true); + return invokerFactory.supply(query, OptionalInt.empty(), nodes, true); } } catch (InvalidSearchPathException e) { return Optional.of(new SearchErrorInvoker(ErrorMessage.createIllegalQuery(e.getMessage()))); @@ -133,7 +134,7 @@ public class Dispatcher extends AbstractComponent { if (directNode.isPresent()) { Node node = directNode.get(); query.trace(false, 2, "Dispatching directly to ", node); - return invokerFactory.supply(query, -1, Arrays.asList(node), true); + return invokerFactory.supply(query, OptionalInt.empty(), Arrays.asList(node), true); } int covered = searchCluster.groupsWithSufficientCoverage(); @@ -148,7 +149,8 @@ public class Dispatcher extends AbstractComponent { } Group group = groupInCluster.get(); boolean acceptIncompleteCoverage = (i == max - 1); - Optional<SearchInvoker> invoker = invokerFactory.supply(query, group.id(), group.nodes(), acceptIncompleteCoverage); + Optional<SearchInvoker> invoker = invokerFactory.supply(query, OptionalInt.of(group.id()), group.nodes(), + acceptIncompleteCoverage); if (invoker.isPresent()) { query.trace(false, 2, "Dispatching internally to search group ", group.id()); query.getModel().setSearchPath("/" + group.id()); 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 e497ef6751b..a1658684b87 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 @@ -18,6 +18,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.OptionalInt; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; import java.util.concurrent.FutureTask; @@ -62,11 +63,11 @@ public class SearchCluster implements NodeManager<Node> { public SearchCluster(String clusterId, DispatchConfig dispatchConfig, FS4ResourcePool fs4ResourcePool, int containerClusterSize, VipStatus vipStatus) { this.clusterId = clusterId; this.dispatchConfig = dispatchConfig; - this.size = dispatchConfig.node().size(); this.fs4ResourcePool = fs4ResourcePool; this.vipStatus = vipStatus; List<Node> nodes = toNodes(dispatchConfig); + this.size = nodes.size(); // Create groups ImmutableMap.Builder<Integer, Group> groupsBuilder = new ImmutableMap.Builder<>(); @@ -331,9 +332,10 @@ public class SearchCluster implements NodeManager<Node> { } } - private void logIfInsufficientCoverage(boolean sufficient, int groupId, int nodes) { + private void logIfInsufficientCoverage(boolean sufficient, OptionalInt groupId, int nodes) { if (!sufficient) { - log.warning(() -> String.format("Coverage of group %s is only %d/%d (requires %d)", groupId, nodes, groupSize(), + String group = groupId.isPresent()? Integer.toString(groupId.getAsInt()) : "(unspecified)"; + log.warning(() -> String.format("Coverage of group %s is only %d/%d (requires %d)", group, nodes, groupSize(), groupSize() - dispatchConfig.maxNodesDownPerGroup())); } } @@ -341,14 +343,22 @@ public class SearchCluster implements NodeManager<Node> { /** * Calculate whether a subset of nodes in a group has enough coverage */ - public boolean isPartialGroupCoverageSufficient(int groupId, List<Node> nodes) { + public boolean isPartialGroupCoverageSufficient(OptionalInt knownGroupId, List<Node> nodes) { if (orderedGroups.size() == 1) { boolean sufficient = nodes.size() >= groupSize() - dispatchConfig.maxNodesDownPerGroup(); - logIfInsufficientCoverage(sufficient, groupId, nodes.size()); + logIfInsufficientCoverage(sufficient, knownGroupId, nodes.size()); return sufficient; } - int nodesInGroup = groups.get(groupId).nodes().size(); + if (knownGroupId.isEmpty()) { + return false; + } + int groupId = knownGroupId.getAsInt(); + Group group = groups.get(groupId); + if (group == null) { + return false; + } + int nodesInGroup = group.nodes().size(); long sumOfActiveDocuments = 0; int otherGroups = 0; for (Group g : orderedGroups) { @@ -363,7 +373,7 @@ public class SearchCluster implements NodeManager<Node> { } long averageDocumentsInOtherGroups = sumOfActiveDocuments / otherGroups; boolean sufficient = isGroupCoverageSufficient(nodes.size(), nodesInGroup, activeDocuments, averageDocumentsInOtherGroups); - logIfInsufficientCoverage(sufficient, groupId, nodes.size()); + logIfInsufficientCoverage(sufficient, knownGroupId, nodes.size()); return sufficient; } } 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 fae659b0d70..708caafa3f5 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 @@ -11,6 +11,7 @@ import org.junit.Test; import java.util.List; import java.util.Optional; +import java.util.OptionalInt; import static com.yahoo.search.dispatch.MockSearchCluster.createDispatchConfig; import static org.hamcrest.Matchers.is; @@ -112,7 +113,7 @@ public class DispatcherTest { } @Override - public Optional<SearchInvoker> getSearchInvoker(Query query, int groupId, List<Node> nodes, boolean acceptIncompleteCoverage) { + public Optional<SearchInvoker> getSearchInvoker(Query query, OptionalInt groupId, List<Node> nodes, boolean acceptIncompleteCoverage) { if (step >= events.length) { throw new RuntimeException("Was not expecting more calls to getSearchInvoker"); } |