Force single-pass grouping with single node groups

9 files changed, 112 insertions, 28 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 7b6781b2744..86964aa667f 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
@@ -24,6 +24,8 @@ import com.yahoo.search.Query;
 import com.yahoo.search.Result;
 import com.yahoo.search.dispatch.Dispatcher;
 import com.yahoo.search.dispatch.SearchCluster;
+import com.yahoo.search.grouping.GroupingRequest;
+import com.yahoo.search.grouping.request.GroupingOperation;
 import com.yahoo.search.query.Ranking;
 import com.yahoo.search.result.ErrorMessage;
 import com.yahoo.search.result.Hit;
@@ -32,9 +34,7 @@ import com.yahoo.search.searchchain.Execution;
 import edu.umd.cs.findbugs.annotations.NonNull;
 
 import java.io.IOException;
-import java.text.SimpleDateFormat;
 import java.util.Iterator;
-import java.util.TimeZone;
 import java.util.logging.Level;
 
 import static com.yahoo.container.util.Util.quote;
@@ -97,24 +97,13 @@ public class FastSearcher extends VespaBackEndSearcher {
         this.selfHostname = HostName.getLocalhost();
     }
 
-    private static SimpleDateFormat isoDateFormat;
-
-    static {
-        isoDateFormat = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss z");
-        isoDateFormat.setTimeZone(TimeZone.getTimeZone("GMT"));
-    }
-
-    private int countNumberOfFastHits(Result result) {
-        int numFastHits = 0;
-
-        for (Iterator<com.yahoo.search.result.Hit> i = hitIterator(result); i.hasNext();) {
-            com.yahoo.search.result.Hit hit = i.next();
-
-            if (hit instanceof FastHit) {
-                numFastHits++;
-            }
+    private int countFastHits(Result result) {
+        int count = 0;
+        for (Iterator<Hit> i = hitIterator(result); i.hasNext();) {
+            if (i.next() instanceof FastHit)
+                count++;
         }
-        return numFastHits;
+        return count;
     }
 
     /**
@@ -184,6 +173,8 @@ public class FastSearcher extends VespaBackEndSearcher {
     public Result doSearch2(Query query, QueryPacket queryPacket, CacheKey cacheKey, Execution execution) {
         FS4Channel channel = null;
         try {
+            if (dispatcher.searchCluster().groupSize() == 1)
+                forceSinglePassGrouping(query);
             channel = chooseBackend(query).openChannel();
             channel.setQuery(query);
 
@@ -212,6 +203,18 @@ public class FastSearcher extends VespaBackEndSearcher {
                 channel.close();
         }
     }
+    
+    /** When we only search a single node, doing all grouping in one pass is more efficient */
+    private void forceSinglePassGrouping(Query query) {
+        for (GroupingRequest groupingRequest : GroupingRequest.getRequests(query))
+            forceSinglePassGrouping(groupingRequest.getRootOperation());
+    }
+    
+    private void forceSinglePassGrouping(GroupingOperation operation) {
+        operation.setForceSinglePass(true);
+        for (GroupingOperation childOperation : operation.getChildren())
+            forceSinglePassGrouping(childOperation);
+    }
 
     /**
      * Returns the backend object to issue a search request over.
@@ -287,7 +290,7 @@ public class FastSearcher extends VespaBackEndSearcher {
         try {
             DocsumPacketKey[] packetKeys;
 
-            if (countNumberOfFastHits(result) > 0) {
+            if (countFastHits(result) > 0) {
                 packetKeys = getPacketKeys(result, summaryClass, false);
                 if (packetKeys.length == 0) {
                     receivedPackets = new Packet[0];
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 9dac736dee2..aec60473541 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
@@ -641,9 +641,7 @@ public abstract class VespaBackEndSearcher extends PingableSearcher {
         return getLogger().isLoggable(Level.FINE);
     }
 
-    public boolean isLocalDispatching() {
-        return localDispatching;
-    }
+    public boolean isLocalDispatching() { return localDispatching; }
 
     public void setLocalDispatching(boolean localDispatching) {
         this.localDispatching = localDispatching;
diff --git a/container-search/src/main/java/com/yahoo/prelude/querytransform/QueryRewrite.java b/container-search/src/main/java/com/yahoo/prelude/querytransform/QueryRewrite.java
index 84689c5adf2..59677ce9476 100644
--- a/container-search/src/main/java/com/yahoo/prelude/querytransform/QueryRewrite.java
+++ b/container-search/src/main/java/com/yahoo/prelude/querytransform/QueryRewrite.java
@@ -19,9 +19,10 @@ import com.yahoo.search.result.Hit;
 /**
  * @author baldersheim
  */
+// TODO: This overlaps with QueryCanonicalizer
 public class QueryRewrite {
 
-    private static enum Recall {
+    private enum Recall {
         RECALLS_EVERYTHING,
         RECALLS_NOTHING,
         UNKNOWN_RECALL
diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/SearchCluster.java b/container-search/src/main/java/com/yahoo/search/dispatch/SearchCluster.java
index b6ee70802d8..9de18ac47ad 100644
--- a/container-search/src/main/java/com/yahoo/search/dispatch/SearchCluster.java
+++ b/container-search/src/main/java/com/yahoo/search/dispatch/SearchCluster.java
@@ -147,6 +147,12 @@ public class SearchCluster implements NodeManager<SearchCluster.Node> {
     /** Returns the groups of this cluster as an immutable map indexed by group id */
     public ImmutableMap<Integer, Group> groups() { return groups; }
 
+    /** Returns the number of nodes per group - size()/groups.size() */
+    public int groupSize() { 
+        if (groups.size() == 0) return size();
+        return size() / groups.size(); 
+    }
+    
     /**
      * Returns the nodes of this cluster as an immutable map indexed by host.
      * One host may contain multiple nodes (on different ports), so this is a multi-map.
diff --git a/container-search/src/main/java/com/yahoo/search/grouping/GroupingRequest.java b/container-search/src/main/java/com/yahoo/search/grouping/GroupingRequest.java
index 8ace3ed72de..b02b4b490c6 100644
--- a/container-search/src/main/java/com/yahoo/search/grouping/GroupingRequest.java
+++ b/container-search/src/main/java/com/yahoo/search/grouping/GroupingRequest.java
@@ -18,7 +18,7 @@ import java.util.*;
  * {@link GroupingOperation} using {@link #setRootOperation(GroupingOperation)}. Once the search returns, access the
  * result {@link Group} using the {@link #getResultGroup(Result)} method.
  *
- * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen</a>
+ * @author Simon Thoresen
  */
 public class GroupingRequest {
 
diff --git a/container-search/src/main/java/com/yahoo/search/grouping/request/AllOperation.java b/container-search/src/main/java/com/yahoo/search/grouping/request/AllOperation.java
index e78be0c1c1a..1c7fbc73b87 100644
--- a/container-search/src/main/java/com/yahoo/search/grouping/request/AllOperation.java
+++ b/container-search/src/main/java/com/yahoo/search/grouping/request/AllOperation.java
@@ -5,7 +5,7 @@ package com.yahoo.search.grouping.request;
  * This is a grouping operation that processes the input list as a whole, as opposed to {@link EachOperation} which
  * processes each element of that list separately.
  *
- * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen</a>
+ * @author Simon Thoresen
  */
 public class AllOperation extends GroupingOperation {
 
diff --git a/container-search/src/main/java/com/yahoo/search/grouping/request/GroupingOperation.java b/container-search/src/main/java/com/yahoo/search/grouping/request/GroupingOperation.java
index d49713ba9f2..977dc91c733 100644
--- a/container-search/src/main/java/com/yahoo/search/grouping/request/GroupingOperation.java
+++ b/container-search/src/main/java/com/yahoo/search/grouping/request/GroupingOperation.java
@@ -16,7 +16,7 @@ import java.util.*;
  * com.yahoo.search.grouping.GroupingRequest} using the {@link com.yahoo.search.grouping.GroupingRequest#setRootOperation(GroupingOperation)}
  * method.
  *
- * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen</a>
+ * @author Simon Thoresen
  */
 public abstract class GroupingOperation extends GroupingNode {
 
@@ -579,4 +579,5 @@ public abstract class GroupingOperation extends GroupingNode {
             throw new IllegalArgumentException(input.formatException(e.getMessage()), e);
         }
     }
+
 }
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 3d08844038e..178f2ffed76 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
@@ -1,6 +1,7 @@
 // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 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.search.Fs4Config;
@@ -20,6 +21,11 @@ import com.yahoo.search.Result;
 import com.yahoo.search.Searcher;
 import com.yahoo.prelude.fastsearch.*;
 import com.yahoo.prelude.query.WordItem;
+import com.yahoo.search.dispatch.SearchCluster;
+import com.yahoo.search.grouping.GroupingRequest;
+import com.yahoo.search.grouping.request.AllOperation;
+import com.yahoo.search.grouping.request.EachOperation;
+import com.yahoo.search.grouping.request.GroupingOperation;
 import com.yahoo.search.rendering.RendererRegistry;
 import com.yahoo.search.result.ErrorMessage;
 import com.yahoo.search.searchchain.Execution;
@@ -235,7 +241,7 @@ public class FastSearcherTestCase {
     }
 
     @Test
-    public void requireThatPropertiesAreReencoded() throws Exception {
+    public void testThatPropertiesAreReencoded() throws Exception {
         FastSearcher fastSearcher = createFastSearcher();
 
         assertEquals(100, fastSearcher.getCacheControl().capacity()); // Default cache =100MB
@@ -373,6 +379,70 @@ public class FastSearcherTestCase {
         }
 
     }
+    
+    @Test
+    public void testSinglePassGroupingIsForcedWithSingleNodeGroups() {
+        FastSearcher fastSearcher = new FastSearcher(new MockBackend(),
+                                                     new FS4ResourcePool(1),
+                                                     new MockDispatcher(new SearchCluster.Node("host0", 123, 0)),
+                                                     new SummaryParameters(null),
+                                                     new ClusterParams("testhittype"),
+                                                     new CacheParams(100, 1e64),
+                                                     documentdbInfoConfig);
+        Query q = new Query("?query=foo");
+        GroupingRequest request1 = GroupingRequest.newInstance(q);
+        request1.setRootOperation(new AllOperation());
+
+        GroupingRequest request2 = GroupingRequest.newInstance(q);
+        AllOperation all = new AllOperation();
+        all.addChild(new EachOperation());
+        all.addChild(new EachOperation());
+        request2.setRootOperation(all);
+        
+        assertForceSinglePassIs(false, q);
+        fastSearcher.search(q, new Execution(Execution.Context.createContextStub()));
+        assertForceSinglePassIs(true, q);        
+    }
+
+    @Test
+    public void testSinglePassGroupingIsNotForcedWithSingleNodeGroups() {
+        MockDispatcher dispatcher = 
+                new MockDispatcher(ImmutableList.of(new SearchCluster.Node("host0", 123, 0),
+                                                    new SearchCluster.Node("host1", 123, 0)));
+
+        FastSearcher fastSearcher = new FastSearcher(new MockBackend(),
+                                                     new FS4ResourcePool(1),
+                                                     dispatcher,
+                                                     new SummaryParameters(null),
+                                                     new ClusterParams("testhittype"),
+                                                     new CacheParams(100, 1e64),
+                                                     documentdbInfoConfig);
+        Query q = new Query("?query=foo");
+        GroupingRequest request1 = GroupingRequest.newInstance(q);
+        request1.setRootOperation(new AllOperation());
+
+        GroupingRequest request2 = GroupingRequest.newInstance(q);
+        AllOperation all = new AllOperation();
+        all.addChild(new EachOperation());
+        all.addChild(new EachOperation());
+        request2.setRootOperation(all);
+
+        assertForceSinglePassIs(false, q);
+        fastSearcher.search(q, new Execution(Execution.Context.createContextStub()));
+        assertForceSinglePassIs(false, q);
+    }
+
+    private void assertForceSinglePassIs(boolean expected, Query query) {
+        for (GroupingRequest request : GroupingRequest.getRequests(query))
+            assertForceSinglePassIs(expected, request.getRootOperation());
+    }
+
+    private void assertForceSinglePassIs(boolean expected, GroupingOperation operation) {
+        assertEquals("Force single pass is " + expected + " in " + operation, 
+                     expected, operation.getForceSinglePass());
+        for (GroupingOperation child : operation.getChildren())
+            assertForceSinglePassIs(expected, child);
+    }
 
     @Test
     public void testPing() throws IOException, InterruptedException {
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 dd4b0ef8b5a..a820b0b8743 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
@@ -7,10 +7,15 @@ import com.yahoo.search.dispatch.Dispatcher;
 import com.yahoo.search.dispatch.SearchCluster;
 import com.yahoo.vespa.config.search.DispatchConfig;
 
+import java.util.Collections;
 import java.util.List;
 
 class MockDispatcher extends Dispatcher {
 
+    public MockDispatcher(SearchCluster.Node node) {
+        this(Collections.singletonList(node));
+    }
+
     public MockDispatcher(List<SearchCluster.Node> nodes) {
         super(toDispatchConfig(nodes), new FS4ResourcePool(1), 1, new VipStatus());
     }