Revert "Revert "dispatch.summaries by default when possible""

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<SearchCluster.Node> 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);
         }