diff options
author | Jon Bratseth <bratseth@verizonmedia.com> | 2019-11-14 09:33:00 +0100 |
---|---|---|
committer | Jon Bratseth <bratseth@verizonmedia.com> | 2019-11-14 09:33:00 +0100 |
commit | 4ea03600dd88069460934d36c7843f713acc0965 (patch) | |
tree | c3bd0e42843130361552893377f2428e4558d526 /container-search | |
parent | 03d90c743ae83cfea09be55cb7f1787aa8c8453b (diff) |
Remove leftovers from dispatching through fdispatch
Diffstat (limited to 'container-search')
5 files changed, 61 insertions, 98 deletions
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 1a373b6ea71..c04553ae2f5 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 @@ -147,7 +147,7 @@ public class FastSearcher extends VespaBackEndSearcher { * on the same host. */ private SearchInvoker getSearchInvoker(Query query) { - return dispatcher.getSearchInvoker(query, this).get(); + return dispatcher.getSearchInvoker(query, this); } /** @@ -156,11 +156,9 @@ public class FastSearcher extends VespaBackEndSearcher { * content nodes. */ private FillInvoker getFillInvoker(Result result) { - return dispatcher.getFillInvoker(result, this).get(); + return dispatcher.getFillInvoker(result, this); } - - private static Optional<String> quotedSummaryClass(String summaryClass) { return Optional.of(summaryClass == null ? "[null]" : quote(summaryClass)); } 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 1f58a2df5c2..ab010001d90 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 @@ -49,7 +49,6 @@ public class Dispatcher extends AbstractComponent { private static final String INTERNAL = "internal"; private static final String PROTOBUF = "protobuf"; - private static final String FDISPATCH_METRIC = "dispatch_fdispatch"; private static final String INTERNAL_METRIC = "dispatch_internal"; private static final int MAX_GROUP_SELECTION_ATTEMPTS = 3; @@ -61,7 +60,6 @@ public class Dispatcher extends AbstractComponent { private final SearchCluster searchCluster; private final LoadBalancer loadBalancer; - private final boolean multilevelDispatch; private final InvokerFactory invokerFactory; @@ -115,11 +113,13 @@ public class Dispatcher extends AbstractComponent { InvokerFactory invokerFactory, PingFactory pingFactory, Metric metric) { + if (dispatchConfig.useMultilevelDispatch()) + throw new IllegalArgumentException(searchCluster + " is configured with multilevel dispatch, but this is not supported"); + this.searchCluster = searchCluster; this.loadBalancer = new LoadBalancer(searchCluster, dispatchConfig.distributionPolicy() == DispatchConfig.DistributionPolicy.ROUNDROBIN); this.invokerFactory = invokerFactory; - this.multilevelDispatch = dispatchConfig.useMultilevelDispatch(); this.metric = metric; this.metricContext = metric.createContext(null); @@ -137,27 +137,18 @@ public class Dispatcher extends AbstractComponent { searchCluster.shutDown(); } - public Optional<FillInvoker> getFillInvoker(Result result, VespaBackEndSearcher searcher) { + public FillInvoker getFillInvoker(Result result, VespaBackEndSearcher searcher) { return invokerFactory.createFillInvoker(searcher, result); } - public Optional<SearchInvoker> getSearchInvoker(Query query, VespaBackEndSearcher searcher) { - if (multilevelDispatch) { - emitDispatchMetric(Optional.empty()); - return Optional.empty(); - } - - Optional<SearchInvoker> invoker = getSearchPathInvoker(query, searcher); + public SearchInvoker getSearchInvoker(Query query, VespaBackEndSearcher searcher) { + SearchInvoker invoker = getSearchPathInvoker(query, searcher).orElseGet(() -> getInternalInvoker(query, searcher)); - if (invoker.isEmpty()) { - invoker = getInternalInvoker(query, searcher); - } - if (invoker.isPresent() && query.properties().getBoolean(com.yahoo.search.query.Model.ESTIMATE)) { + if (query.properties().getBoolean(com.yahoo.search.query.Model.ESTIMATE)) { query.setHits(0); query.setOffset(0); } - emitDispatchMetric(invoker); - + metric.add(INTERNAL_METRIC, 1, metricContext); return invoker; } @@ -177,12 +168,13 @@ public class Dispatcher extends AbstractComponent { } } - private Optional<SearchInvoker> getInternalInvoker(Query query, VespaBackEndSearcher searcher) { + private SearchInvoker getInternalInvoker(Query query, VespaBackEndSearcher searcher) { Optional<Node> directNode = searchCluster.localCorpusDispatchTarget(); if (directNode.isPresent()) { Node node = directNode.get(); - query.trace(false, 2, "Dispatching directly to ", node); - return invokerFactory.createSearchInvoker(searcher, query, OptionalInt.empty(), Arrays.asList(node), true); + query.trace(false, 2, "Dispatching to ", node); + return invokerFactory.createSearchInvoker(searcher, query, OptionalInt.empty(), Arrays.asList(node), true) + .orElseThrow(() -> new IllegalStateException("Could not dispatch directly to " + node)); } int covered = searchCluster.groupsWithSufficientCoverage(); @@ -201,10 +193,10 @@ public class Dispatcher extends AbstractComponent { group.nodes(), acceptIncompleteCoverage); if (invoker.isPresent()) { - query.trace(false, 2, "Dispatching internally to search group ", group.id()); + query.trace(false, 2, "Dispatching to group ", group.id()); query.getModel().setSearchPath("/" + group.id()); invoker.get().teardown((success, time) -> loadBalancer.releaseGroup(group, success, time)); - return invoker; + return invoker.get(); } else { loadBalancer.releaseGroup(group, false, 0); if (rejected == null) { @@ -213,16 +205,7 @@ public class Dispatcher extends AbstractComponent { rejected.add(group.id()); } } - - return Optional.empty(); - } - - private void emitDispatchMetric(Optional<SearchInvoker> invoker) { - if (invoker.isEmpty()) { - metric.add(FDISPATCH_METRIC, 1, metricContext); - } else { - metric.add(INTERNAL_METRIC, 1, metricContext); - } + throw new IllegalStateException("No suitable groups to dispatch query. Rejected: " + rejected); } } 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 78c50254a84..6030e989595 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 @@ -20,6 +20,7 @@ import java.util.Set; * @author ollivir */ public abstract class InvokerFactory { + protected final SearchCluster searchCluster; public InvokerFactory(SearchCluster searchCluster) { @@ -28,24 +29,18 @@ public abstract class InvokerFactory { protected abstract Optional<SearchInvoker> createNodeSearchInvoker(VespaBackEndSearcher searcher, Query query, Node node); - public abstract Optional<FillInvoker> createFillInvoker(VespaBackEndSearcher searcher, Result result); + public abstract FillInvoker createFillInvoker(VespaBackEndSearcher searcher, Result result); /** * Create a {@link SearchInvoker} for a list of content nodes. * - * @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 acceptIncompleteCoverage - * if some of the nodes are unavailable and this parameter is - * <b>false</b>, verify that the remaining set of nodes has enough - * coverage - * @return Optional containing the SearchInvoker or <i>empty</i> if some node in the + * @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 acceptIncompleteCoverage if some of the nodes are unavailable and this parameter is + * false, verify that the remaining set of nodes has sufficient coverage + * @return Optional containing the SearchInvoker or empty if some node in the * list is invalid and the remaining coverage is not sufficient */ public Optional<SearchInvoker> createSearchInvoker(VespaBackEndSearcher searcher, @@ -76,11 +71,11 @@ public abstract class InvokerFactory { if (failed != null) { List<Node> success = new ArrayList<>(nodes.size() - failed.size()); for (Node node : nodes) { - if (!failed.contains(node.key())) { + if ( ! failed.contains(node.key())) { success.add(node); } } - if (!searchCluster.isPartialGroupCoverageSufficient(groupId, success) && !acceptIncompleteCoverage) { + if ( ! searchCluster.isPartialGroupCoverageSufficient(groupId, success) && !acceptIncompleteCoverage) { return Optional.empty(); } if(invokers.size() == 0) { @@ -113,4 +108,5 @@ public abstract class InvokerFactory { } public void release() {} + } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcInvokerFactory.java b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcInvokerFactory.java index 6160d3dfa08..870f7aef9c5 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcInvokerFactory.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/rpc/RpcInvokerFactory.java @@ -40,7 +40,7 @@ public class RpcInvokerFactory extends InvokerFactory implements PingFactory { } @Override - public Optional<FillInvoker> createFillInvoker(VespaBackEndSearcher searcher, Result result) { + public FillInvoker createFillInvoker(VespaBackEndSearcher searcher, Result result) { Query query = result.getQuery(); boolean summaryNeedsQuery = searcher.summaryNeedsQuery(query); @@ -48,8 +48,8 @@ public class RpcInvokerFactory extends InvokerFactory implements PingFactory { boolean useDispatchDotSummaries = query.properties().getBoolean(dispatchSummaries, false); return ((useDispatchDotSummaries || !useProtoBuf) && ! summaryNeedsQuery) - ? Optional.of(new RpcFillInvoker(rpcResourcePool, searcher.getDocumentDatabase(query))) - : Optional.of(new RpcProtobufFillInvoker(rpcResourcePool, searcher.getDocumentDatabase(query), searcher.getServerId(), summaryNeedsQuery)); + ? new RpcFillInvoker(rpcResourcePool, searcher.getDocumentDatabase(query)) + : new RpcProtobufFillInvoker(rpcResourcePool, searcher.getDocumentDatabase(query), searcher.getServerId(), summaryNeedsQuery); } // for testing 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 30f6c5a495d..291b0f4890a 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,7 +11,6 @@ import com.yahoo.search.cluster.ClusterMonitor; import com.yahoo.search.dispatch.searchcluster.Node; import com.yahoo.search.dispatch.searchcluster.PingFactory; import com.yahoo.search.dispatch.searchcluster.SearchCluster; -import com.yahoo.vespa.config.search.DispatchConfig; import org.junit.Test; import java.util.List; @@ -20,46 +19,28 @@ import java.util.OptionalInt; import java.util.concurrent.Callable; import static com.yahoo.search.dispatch.MockSearchCluster.createDispatchConfig; -import static org.hamcrest.Matchers.is; -import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; /** * @author ollivir */ public class DispatcherTest { - private static final CompoundName internalDispatch = new CompoundName("dispatch.internal"); - - private static Query query() { - Query q = new Query(); - q.properties().set(internalDispatch, "true"); - return q; - } - - @Test - public void requireDispatcherToIgnoreMultilevelConfigurations() { - SearchCluster searchCluster = new MockSearchCluster("1", 2, 2); - DispatchConfig dispatchConfig = new DispatchConfig.Builder().useMultilevelDispatch(true).build(); - - var invokerFactory = new MockInvokerFactory(searchCluster); - - Dispatcher disp = new Dispatcher(searchCluster, dispatchConfig, invokerFactory, invokerFactory, new MockMetric()); - assertThat(disp.getSearchInvoker(query(), null).isPresent(), is(false)); - } @Test public void requireThatDispatcherSupportsSearchPath() { SearchCluster cl = new MockSearchCluster("1", 2, 2); - Query q = query(); + Query q = new Query(); q.getModel().setSearchPath("1/0"); // second node in first group MockInvokerFactory invokerFactory = new MockInvokerFactory(cl, (nodes, a) -> { - assertThat(nodes.size(), is(1)); - assertThat(nodes.get(0).key(), is(2)); + assertEquals(1, nodes.size()); + assertEquals(2, nodes.get(0).key()); return true; }); Dispatcher disp = new Dispatcher(cl, createDispatchConfig(), invokerFactory, invokerFactory, new MockMetric()); - Optional<SearchInvoker> invoker = disp.getSearchInvoker(q, null); - assertThat(invoker.isPresent(), is(true)); + SearchInvoker invoker = disp.getSearchInvoker(q, null); invokerFactory.verifyAllEventsProcessed(); } @@ -73,8 +54,7 @@ public class DispatcherTest { }; MockInvokerFactory invokerFactory = new MockInvokerFactory(cl, (n, a) -> true); Dispatcher disp = new Dispatcher(cl, createDispatchConfig(), invokerFactory, invokerFactory, new MockMetric()); - Optional<SearchInvoker> invoker = disp.getSearchInvoker(query(), null); - assertThat(invoker.isPresent(), is(true)); + SearchInvoker invoker = disp.getSearchInvoker(new Query(), null); invokerFactory.verifyAllEventsProcessed(); } @@ -83,31 +63,34 @@ public class DispatcherTest { SearchCluster cl = new MockSearchCluster("1", 2, 1); MockInvokerFactory invokerFactory = new MockInvokerFactory(cl, (n, acceptIncompleteCoverage) -> { - assertThat(acceptIncompleteCoverage, is(false)); + assertFalse(acceptIncompleteCoverage); return false; }, (n, acceptIncompleteCoverage) -> { - assertThat(acceptIncompleteCoverage, is(true)); + assertTrue(acceptIncompleteCoverage); return true; }); Dispatcher disp = new Dispatcher(cl, createDispatchConfig(), invokerFactory, invokerFactory, new MockMetric()); - Optional<SearchInvoker> invoker = disp.getSearchInvoker(query(), null); - assertThat(invoker.isPresent(), is(true)); + SearchInvoker invoker = disp.getSearchInvoker(new Query(), null); invokerFactory.verifyAllEventsProcessed(); } @Test public void requireThatInvokerConstructionDoesNotRepeatGroups() { - SearchCluster cl = new MockSearchCluster("1", 2, 1); + try { + SearchCluster cl = new MockSearchCluster("1", 2, 1); - MockInvokerFactory invokerFactory = new MockInvokerFactory(cl, (n, a) -> false, (n, a) -> false); - Dispatcher disp = new Dispatcher(cl, createDispatchConfig(), invokerFactory, invokerFactory, new MockMetric()); - Optional<SearchInvoker> invoker = disp.getSearchInvoker(query(), null); - assertThat(invoker.isPresent(), is(false)); - invokerFactory.verifyAllEventsProcessed(); + MockInvokerFactory invokerFactory = new MockInvokerFactory(cl, (n, a) -> false, (n, a) -> false); + Dispatcher disp = new Dispatcher(cl, createDispatchConfig(), invokerFactory, invokerFactory, new MockMetric()); + disp.getSearchInvoker(new Query(), null); + fail("Expected exception"); + } + catch (IllegalStateException e) { + assertEquals("No suitable groups to dispatch query. Rejected: [0, 1]", e.getMessage()); + } } interface FactoryStep { - public boolean returnInvoker(List<Node> nodes, boolean acceptIncompleteCoverage); + boolean returnInvoker(List<Node> nodes, boolean acceptIncompleteCoverage); } private static class MockInvokerFactory extends InvokerFactory implements PingFactory { @@ -121,8 +104,11 @@ public class DispatcherTest { } @Override - public Optional<SearchInvoker> createSearchInvoker(VespaBackEndSearcher searcher, Query query, OptionalInt groupId, - List<Node> nodes, boolean acceptIncompleteCoverage) { + public Optional<SearchInvoker> createSearchInvoker(VespaBackEndSearcher searcher, + Query query, + OptionalInt groupId, + List<Node> nodes, + boolean acceptIncompleteCoverage) { if (step >= events.length) { throw new RuntimeException("Was not expecting more calls to getSearchInvoker"); } @@ -136,7 +122,7 @@ public class DispatcherTest { } void verifyAllEventsProcessed() { - assertThat(step, is(events.length)); + assertEquals(events.length, step); } @Override @@ -146,7 +132,7 @@ public class DispatcherTest { } @Override - public Optional<FillInvoker> createFillInvoker(VespaBackEndSearcher searcher, Result result) { + public FillInvoker createFillInvoker(VespaBackEndSearcher searcher, Result result) { fail("Unexpected call to createFillInvoker"); return null; } |