From cd2d2d63c4962ef98c4da0c3db3ec1d6170b1cbf Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Thu, 12 Apr 2018 20:41:36 +0200 Subject: Revert "Revert "dispatch.summaries by default when possible"" --- .../com/yahoo/prelude/fastsearch/FastSearcher.java | 5 ++- .../prelude/fastsearch/VespaBackEndSearcher.java | 12 ----- .../java/com/yahoo/search/dispatch/Dispatcher.java | 6 ++- .../fastsearch/test/FastSearcherTestCase.java | 52 ++++++++++++++-------- .../fastsearch/test/fs4mock/MockFSChannel.java | 4 +- 5 files changed, 45 insertions(+), 34 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 7d822fd603b..a2fdcae4a18 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 @@ -56,6 +56,9 @@ public class FastSearcher extends VespaBackEndSearcher { /** If this is turned on this will make search queries directly to the local search node when possible */ private final static CompoundName dispatchDirect = new CompoundName("dispatch.direct"); + /** Unless turned off this will fill summaries by dispatching directly to search nodes over RPC when possible */ + private final static CompoundName dispatchSummaries = new CompoundName("dispatch.summaries"); + /** The compression method which will be used with rpc dispatch. "lz4" (default) and "none" is supported. */ private final static CompoundName dispatchCompression = new CompoundName("dispatch.compression"); @@ -255,7 +258,7 @@ public class FastSearcher extends VespaBackEndSearcher { Query query = result.getQuery(); traceQuery(getName(), "fill", query, query.getOffset(), query.getHits(), 2, quotedSummaryClass(summaryClass)); - if (wantsRPCSummaryFill(query)) { + if (query.properties().getBoolean(dispatchSummaries, true) && !summaryNeedsQuery(query)) { CompressionType compression = CompressionType.valueOf(query.properties().getString(dispatchCompression, "LZ4").toUpperCase()); fillSDDocName(result); diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java index eac1579a821..e5c9f048e53 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java @@ -55,8 +55,6 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { private static final CompoundName grouping=new CompoundName("grouping"); private static final CompoundName combinerows=new CompoundName("combinerows"); - /** If this is turned on this will fill summaries by dispatching directly to search nodes over RPC */ - private final static CompoundName dispatchSummaries = new CompoundName("dispatch.summaries"); protected static final CompoundName PACKET_COMPRESSION_LIMIT = new CompoundName("packetcompressionlimit"); protected static final CompoundName PACKET_COMPRESSION_TYPE = new CompoundName("packetcompressiontype"); @@ -110,10 +108,6 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { protected abstract void doPartialFill(Result result, String summaryClass); - protected static boolean wantsRPCSummaryFill(Query query) { - return query.properties().getBoolean(dispatchSummaries); - } - /** * Returns whether we need to send the query when fetching summaries. * This is necessary if the query requests summary features or dynamic snippeting @@ -216,12 +210,6 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { return new Result(query, ErrorMessage.createNullQuery(query.getHttpRequest().getUri().toString())); } - if (wantsRPCSummaryFill(query) && summaryNeedsQuery(query)) { - return new Result(query, ErrorMessage.createInvalidQueryParameter( - "When using dispatch.summaries and your summary/rankprofile require the query, " + - " you need to enable ranking.queryCache.")); - } - QueryRewrite.optimizeByRestrict(query); QueryRewrite.optimizeAndNot(query); QueryRewrite.collapseSingleComposites(query); 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 1b775d2c46f..9f68d327ade 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 @@ -38,7 +38,11 @@ import java.util.logging.Level; import java.util.logging.Logger; /** - * A dispatcher communicates with search nodes to (in the future) perform queries and (now) fill hits. + * A dispatcher communicates with search nodes to perform queries and fill hits. + * + * This is currently not functionally complete: Queries can only be dispatched to a single node, + * and summaries can only be requested when they do not need the query. + * * This class is multithread safe. * * @author bratseth diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java index 20150fc2671..01c4a488acd 100644 --- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java @@ -4,6 +4,7 @@ package com.yahoo.prelude.fastsearch.test; import com.google.common.collect.ImmutableList; import com.yahoo.component.chain.Chain; import com.yahoo.config.subscription.ConfigGetter; +import com.yahoo.container.handler.VipStatus; import com.yahoo.container.search.Fs4Config; import com.yahoo.fs4.mplex.*; import com.yahoo.fs4.test.QueryTestCase; @@ -14,6 +15,7 @@ import com.yahoo.prelude.fastsearch.DocumentdbInfoConfig; import com.yahoo.container.protect.Error; import com.yahoo.fs4.*; import com.yahoo.prelude.fastsearch.test.fs4mock.MockBackend; +import com.yahoo.prelude.fastsearch.test.fs4mock.MockFS4ResourcePool; import com.yahoo.prelude.fastsearch.test.fs4mock.MockFSChannel; import com.yahoo.processing.execution.Execution.Trace; import com.yahoo.search.Query; @@ -111,30 +113,45 @@ public class FastSearcherTestCase { .summaryclass(new DocumentdbInfoConfig.Documentdb.Summaryclass.Builder().name("simple").id(7)) .rankprofile(new DocumentdbInfoConfig.Documentdb.Rankprofile.Builder() .name("simpler").hasRankFeatures(false).hasSummaryFeatures(false)))); + + List nodes = new ArrayList<>(); + nodes.add(new SearchCluster.Node("host1", 5000, 0)); + nodes.add(new SearchCluster.Node("host2", 5000, 0)); + + MockFS4ResourcePool mockFs4ResourcePool = new MockFS4ResourcePool(); FastSearcher fastSearcher = new FastSearcher(new MockBackend(), - new FS4ResourcePool(1), - new MockDispatcher(Collections.emptyList()), + mockFs4ResourcePool, + new MockDispatcher(nodes, mockFs4ResourcePool, 1, new VipStatus()), new SummaryParameters(null), new ClusterParams("testhittype"), new CacheParams(100, 1e64), documentdbConfigWithOneDb); - String query = "?query=sddocname:a&dispatch.summaries"; - Result result = doSearch(fastSearcher,new Query(query), 0, 10); - ErrorMessage message = result.hits().getError(); - - assertNotNull("Got error", message); - assertEquals("Invalid query parameter", message.getMessage()); - assertEquals("When using dispatch.summaries and your summary/rankprofile require the query, you need to enable ranking.queryCache.", message.getDetailedMessage()); - assertEquals(Error.INVALID_QUERY_PARAMETER.code, message.getCode()); + { // No direct.summaries + String query = "?query=sddocname:a&summary=simple"; + Result result = doSearch(fastSearcher, new Query(query), 0, 10); + doFill(fastSearcher, result); + ErrorMessage error = result.hits().getError(); + assertNull("Since we don't route to the dispatcher we hit the mock backend, so no error", error); + } - query = "?query=sddocname:a&dispatch.summaries&ranking.queryCache"; - result = doSearch(fastSearcher,new Query(query), 0, 10); - assertNull(result.hits().getError()); + { // direct.summaries due to query cache + String query = "?query=sddocname:a&ranking.queryCache"; + Result result = doSearch(fastSearcher, new Query(query), 0, 10); + doFill(fastSearcher, result); + ErrorMessage error = result.hits().getError(); + assertEquals("Since we don't actually run summary backends we get this error when the Dispatcher is used", + "Error response from rpc node connection to host1:0: Connection error", error.getDetailedMessage()); + } - query = "?query=sddocname:a&dispatch.summaries&summary=simple&ranking=simpler"; - result = doSearch(fastSearcher,new Query(query), 0, 10); - assertNull(result.hits().getError()); + { // direct.summaries due to no summary features + String query = "?query=sddocname:a&dispatch.summaries&summary=simple&ranking=simpler"; + Result result = doSearch(fastSearcher, new Query(query), 0, 10); + doFill(fastSearcher, result); + ErrorMessage error = result.hits().getError(); + assertEquals("Since we don't actually run summary backends we get this error when the Dispatcher is used", + "Error response from rpc node connection to host1:0: Connection error", error.getDetailedMessage()); + } } @Test @@ -281,7 +298,7 @@ public class FastSearcherTestCase { assertEquals(100, fastSearcher.getCacheControl().capacity()); // Default cache =100MB - Query query = new Query("?query=ignored"); + Query query = new Query("?query=ignored&dispatch.summaries=false"); query.getRanking().setQueryCache(true); Result result = doSearch(fastSearcher, query, 0, 10); @@ -321,7 +338,6 @@ public class FastSearcherTestCase { answer.flip(); answer.get(expected); - assertEquals(expected.length, actual.length); for (int i = 0; i < expected.length; ++i) { if (expected[i] == IGNORE) { actual[i] = IGNORE; diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/fs4mock/MockFSChannel.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/fs4mock/MockFSChannel.java index 8d705406774..37a74831e56 100644 --- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/fs4mock/MockFSChannel.java +++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/fs4mock/MockFSChannel.java @@ -96,13 +96,13 @@ public class MockFSChannel extends FS4Channel { && lastQueryPacket.getLastOffset() >= 1) { result.addDocument( new DocumentInfo(DocsumDefinitionTestCase.createGlobalId(123), - 2003, 234, 1000)); + 2003, 234, 0)); } if (lastQueryPacket.getOffset() <= 1 && lastQueryPacket.getLastOffset() >= 2) { result.addDocument( new DocumentInfo(DocsumDefinitionTestCase.createGlobalId(456), - 1855, 234, 1001)); + 1855, 234, 1)); } packets.add(result); } -- cgit v1.2.3