Nonfunctional changes preparing for direct dispatch

7 files changed, 105 insertions, 62 deletions

diff --git a/container-search/src/main/java/com/yahoo/fs4/mplex/Backend.java b/container-search/src/main/java/com/yahoo/fs4/mplex/Backend.java
index 47e57600fb6..3f88dbf9d79 100644
--- a/container-search/src/main/java/com/yahoo/fs4/mplex/Backend.java
+++ b/container-search/src/main/java/com/yahoo/fs4/mplex/Backend.java
@@ -76,7 +76,7 @@ public class Backend implements ConnectionFactory {
     }
 
     public Backend(String host, int port, String serverDiscriminator, ListenerPool listenerPool, ConnectionPool connectionPool) {
-        final String fileNamePattern = "qrs." + serverDiscriminator + '.' + host + ":" + port + ".%s" + ".dump";
+        String fileNamePattern = "qrs." + serverDiscriminator + '.' + host + ":" + port + ".%s" + ".dump";
         packetDumper = new PacketDumper(new File(Defaults.getDefaults().vespaHome() + "logs/vespa/qrs/"), fileNamePattern);
         packetListener = new PacketNotificationsBroadcaster(packetDumper, new PacketQueryTracer());
         this.listeners = listenerPool;
diff --git a/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterSearcher.java b/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterSearcher.java
index 9af5d5ecd7b..288dc229999 100644
--- a/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterSearcher.java
+++ b/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterSearcher.java
@@ -101,11 +101,11 @@ public class ClusterSearcher extends Searcher {
                            QrMonitorConfig monitorConfig,
                            DispatchConfig dispatchConfig,
                            Statistics manager,
-                           FS4ResourcePool listeners,
+                           FS4ResourcePool fs4ResourcePool,
                            VipStatus vipStatus) {
         super(id);
         this.hasher = new Hasher();
-        this.fs4ResourcePool = listeners;
+        this.fs4ResourcePool = fs4ResourcePool;
         monitor = new ClusterMonitor(this, monitorConfig, vipStatus);
         int searchClusterIndex = clusterConfig.clusterId();
         clusterModelName = clusterConfig.clusterName();
@@ -146,7 +146,7 @@ public class ClusterSearcher extends Searcher {
         } else {
             for (int i = 0; i < searchClusterConfig.dispatcher().size(); i++) {
                 Backend b = createBackend(searchClusterConfig.dispatcher(i));
-                FastSearcher searcher = searchDispatch(searchClusterIndex,
+                FastSearcher searcher = searchDispatch(searchClusterIndex, fs4ResourcePool, 
                                                        searchClusterConfig, cacheParams, emulationConfig, docSumParams,
                                                        documentDbConfig, b, dispatcher, i);
                 try {
@@ -209,6 +209,7 @@ public class ClusterSearcher extends Searcher {
     }
 
     private static FastSearcher searchDispatch(int searchclusterIndex,
+                                               FS4ResourcePool fs4ResourcePool,
                                                QrSearchersConfig.Searchcluster searchClusterConfig,
                                                CacheParams cacheParams,
                                                LegacyEmulationConfig emulConfig,
@@ -220,7 +221,7 @@ public class ClusterSearcher extends Searcher {
         ClusterParams clusterParams = makeClusterParams(searchclusterIndex,
                                                         searchClusterConfig,
                                                         emulConfig, i);
-        return new FastSearcher(backend, dispatcher, docSumParams, clusterParams, cacheParams, documentdbInfoConfig);
+        return new FastSearcher(backend, fs4ResourcePool, dispatcher, docSumParams, clusterParams, cacheParams, documentdbInfoConfig);
     }
 
     private static VdsStreamingSearcher vdsCluster(int searchclusterIndex,
@@ -259,13 +260,13 @@ public class ClusterSearcher extends Searcher {
 
     public Map<String, Backend.BackendStatistics> getBackendStatistics() {
         Map<String, Backend.BackendStatistics> backendStatistics = new TreeMap<>();
-        for (final Backend backend : backends) {
+        for (Backend backend : backends) {
             backendStatistics.put(backend.toString(), backend.getStatistics());
         }
         return backendStatistics;
     }
 
-    private Backend createBackend(final QrSearchersConfig.Searchcluster.Dispatcher disp) {
+    private Backend createBackend(QrSearchersConfig.Searchcluster.Dispatcher disp) {
         return fs4ResourcePool.getBackend(disp.host(), disp.port());
     }
 
diff --git a/container-search/src/main/java/com/yahoo/prelude/cluster/dispatchprototype/DispatchClusterSearcher.java b/container-search/src/main/java/com/yahoo/prelude/cluster/dispatchprototype/DispatchClusterSearcher.java
index 0c4c81c5fc9..a05d222abde 100644
--- a/container-search/src/main/java/com/yahoo/prelude/cluster/dispatchprototype/DispatchClusterSearcher.java
+++ b/container-search/src/main/java/com/yahoo/prelude/cluster/dispatchprototype/DispatchClusterSearcher.java
@@ -31,6 +31,7 @@ import static com.yahoo.container.QrSearchersConfig.Searchcluster;
  *
  * @author bakksjo
  */
+// 2016-08-16 (bratseth): We should probably just remove this now. It was a prototype that never went anywhere
 @Beta
 @After("*")
 public class DispatchClusterSearcher extends Searcher {
diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4ResourcePool.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4ResourcePool.java
index f479bed14bd..82490ffafec 100644
--- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4ResourcePool.java
+++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FS4ResourcePool.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;
 
+import com.google.inject.Inject;
 import com.yahoo.component.AbstractComponent;
 import com.yahoo.concurrent.ThreadFactoryFactory;
 import com.yahoo.container.Server;
@@ -29,20 +30,25 @@ import java.util.logging.Logger;
  * @since 5.4.0
  */
 public class FS4ResourcePool extends AbstractComponent {
+
     private static final Logger logger = Logger.getLogger(FS4ResourcePool.class.getName());
     private static final AtomicInteger instanceCounter = new AtomicInteger(0);
     private final int instanceId;
     private final ListenerPool listeners;
     private final Timer timer = new Timer();  // This is a timer for cleaning the closed connections
-    private Map<String, Backend> connectionPoolMap = new HashMap<>();
+    private final Map<String, Backend> connectionPoolMap = new HashMap<>();
     private final ExecutorService executor;
     private final ScheduledExecutorService scheduledExecutor;
 
+    @Inject
     public FS4ResourcePool(Fs4Config fs4Config) {
+        this(fs4Config.numlistenerthreads());
+    }
+    
+    public FS4ResourcePool(int listenerThreads) {
         instanceId = instanceCounter.getAndIncrement();
-        logger.log(Level.INFO, "Constructing an FS4ResourcePool with id '" + instanceId + "' with config '" + fs4Config.toString() + "'");
         String name = "FS4-" + instanceId;
-        listeners = new ListenerPool(name, fs4Config.numlistenerthreads());
+        listeners = new ListenerPool(name, listenerThreads);
         executor = Executors.newCachedThreadPool(ThreadFactoryFactory.getDaemonThreadFactory(name));
         scheduledExecutor = Executors.newScheduledThreadPool(1, ThreadFactoryFactory.getDaemonThreadFactory(name + ".scheduled"));
     }
@@ -53,8 +59,8 @@ public class FS4ResourcePool extends AbstractComponent {
     public ScheduledExecutorService getScheduledExecutor() {
         return scheduledExecutor;
     }
-    public Backend getBackend(String host, int port) {
 
+    public Backend getBackend(String host, int port) {
         String key = host + ":" + port;
         synchronized (connectionPoolMap) {
             Backend pool = connectionPoolMap.get(key);
@@ -85,4 +91,5 @@ public class FS4ResourcePool extends AbstractComponent {
             logger.warning("Executors failed terminating within timeout of 10 seconds : " + e);
         }
     }
+
 }
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 cd94d2a59ec..3a5958e7c27 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
@@ -66,21 +66,32 @@ public class FastSearcher extends VespaBackEndSearcher {
     /** Edition of the index */
     private int docstamp;
 
-    private Backend backend;
+    private final Backend backend;
+    
+    private final FS4ResourcePool fs4ResourcePool;
 
     /**
      * Creates a Fastsearcher.
      *
-     * @param backend       The backend object for this FastSearcher
-     * @param docSumParams  Document summary parameters
-     * @param clusterParams The cluster number, and other cluster backend parameters
-     * @param cacheParams   The size, lifetime, and controller of our cache
-     * @param documentdbInfoConfig Document database parameters
+     * @param backend       The backend object containing the connection to the dispatch node this should talk to
+     *                      over the fs4 protocol
+     * @param fs4ResourcePool the resource pool used to create direct connections to the local search nodes when
+     *                        bypassing the dispatch node
+     * @param dispatcher the dispatcher used (when enabled) to send summary requests over the rpc protocol.
+     *                   Eventually we will move everything to this protocol and never use dispatch nodes.
+     *                   At that point we won't need a cluster searcher above this to select and pass the right
+     *                   backend.
+     * @param docSumParams  document summary parameters
+     * @param clusterParams the cluster number, and other cluster backend parameters
+     * @param cacheParams   the size, lifetime, and controller of our cache
+     * @param documentdbInfoConfig document database parameters
      */
-    public FastSearcher(Backend backend, Dispatcher dispatcher, SummaryParameters docSumParams, ClusterParams clusterParams,
+    public FastSearcher(Backend backend, FS4ResourcePool fs4ResourcePool,
+                        Dispatcher dispatcher, SummaryParameters docSumParams, ClusterParams clusterParams,
                         CacheParams cacheParams, DocumentdbInfoConfig documentdbInfoConfig) {
         init(docSumParams, clusterParams, cacheParams, documentdbInfoConfig);
         this.backend = backend;
+        this.fs4ResourcePool = fs4ResourcePool;
         this.dispatcher = dispatcher;
     }
 
@@ -201,11 +212,9 @@ public class FastSearcher extends VespaBackEndSearcher {
     public Result doSearch2(Query query, QueryPacket queryPacket, CacheKey cacheKey, Execution execution) {
         FS4Channel channel = null;
         try {
-            channel = backend.openChannel();
+            channel = chooseBackend().openChannel();
             channel.setQuery(query);
 
-            // If not found, then fetch from the source. The call to
-            // insert into cache will be made from within searchTwoPhase
             Result result = searchTwoPhase(channel, query, queryPacket, cacheKey);
 
             if (query.properties().getBoolean(Ranking.RANKFEATURES, false)) {
@@ -227,13 +236,24 @@ public class FastSearcher extends VespaBackEndSearcher {
             result.hits().addError(ErrorMessage.createBackendCommunicationError(getName() + " failed: "+ e.getMessage()));
             return result;
         } finally {
-            if (channel != null) {
+            if (channel != null)
                 channel.close();
-            }
         }
     }
 
     /**
+     * Returns the backend object to issue a search request over.
+     * Normally this is the backend field of this instance, which connects to the dispatch node this talk to
+     * (which is why this instance was chosen by the cluster controller). However, when certain conditions obtain
+     * (see below), we will instead return a backend instance which connects directly to the local search node
+     * for efficiency.
+     */
+    private Backend chooseBackend() {
+        // TODO: Implement
+        return backend;
+    }
+
+    /**
      * Only used to fill the sddocname field when using direct dispatching as that is normally done in VespaBackEndSearcher.decodeSummary
      * @param result The 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 ce9a8e6b8c6..419e0cf41e5 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
@@ -205,9 +205,8 @@ public abstract class VespaBackEndSearcher extends PingableSearcher {
         QueryPacket queryPacket = QueryPacket.create(query);
         int compressionLimit = query.properties().getInteger(PACKET_COMPRESSION_LIMIT, 0);
         queryPacket.setCompressionLimit(compressionLimit);
-        if (compressionLimit != 0) {
+        if (compressionLimit != 0)
             queryPacket.setCompressionType(query.properties().getString(PACKET_COMPRESSION_TYPE, "lz4"));
-        }
 
         if (isLoggingFine())
             getLogger().fine("made QueryPacket: " + queryPacket);
@@ -220,15 +219,12 @@ public abstract class VespaBackEndSearcher extends PingableSearcher {
         }
 
         if (result == null) {
-            String next = null;
             result = doSearch2(query, queryPacket, cacheKey, execution);
-            if (isLoggingFine()) {
+            if (isLoggingFine())
                 getLogger().fine("Result NOT retrieved from cache");
-            }
 
-            if (query.getTraceLevel() >= 1) {
+            if (query.getTraceLevel() >= 1)
                 query.trace(getName() + " dispatch response: " + result, false, 1);
-            }
             result.trace(getName());
         }
         return result;
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 faaf3f5c2b9..927faee1e5c 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
@@ -59,11 +59,13 @@ public class FastSearcherTestCase {
     @Test
     public void testNoNormalizing() {
         Logger.getLogger(FastSearcher.class.getName()).setLevel(Level.ALL);
-        FastSearcher fastSearcher = new FastSearcher(new MockBackend(), new MockDispatcher(),
-                new SummaryParameters(null),
-                new ClusterParams("testhittype"),
-                new CacheParams(100, 1e64),
-                documentdbInfoConfig);
+        FastSearcher fastSearcher = new FastSearcher(new MockBackend(),
+                                                     new FS4ResourcePool(1),
+                                                     new MockDispatcher(),
+                                                     new SummaryParameters(null),
+                                                     new ClusterParams("testhittype"),
+                                                     new CacheParams(100, 1e64),
+                                                     documentdbInfoConfig);
 
         MockFSChannel.setEmptyDocsums(false);
 
@@ -78,11 +80,13 @@ public class FastSearcherTestCase {
     @Test
     public void testNullQuery() {
         Logger.getLogger(FastSearcher.class.getName()).setLevel(Level.ALL);
-        FastSearcher fastSearcher = new FastSearcher(new MockBackend(), new MockDispatcher(),
-                new SummaryParameters(null),
-                new ClusterParams("testhittype"),
-                new CacheParams(100, 1e64),
-                documentdbInfoConfig);
+        FastSearcher fastSearcher = new FastSearcher(new MockBackend(), 
+                                                     new FS4ResourcePool(1),
+                                                     new MockDispatcher(),
+                                                     new SummaryParameters(null),
+                                                     new ClusterParams("testhittype"),
+                                                     new CacheParams(100, 1e64),
+                                                     documentdbInfoConfig);
 
         String query = "?junkparam=ignored";
         Result result = doSearch(fastSearcher,new Query(query), 0, 10);
@@ -99,9 +103,13 @@ public class FastSearcherTestCase {
         mockBackend = new MockBackend();
         DocumentdbInfoConfig documentdbConfigWithOneDb =
             new DocumentdbInfoConfig(new DocumentdbInfoConfig.Builder().documentdb(new DocumentdbInfoConfig.Documentdb.Builder().name("testDb")));
-        FastSearcher fastSearcher = new FastSearcher(mockBackend, new MockDispatcher(), new SummaryParameters(null),
+        FastSearcher fastSearcher = new FastSearcher(mockBackend,
+                                                     new FS4ResourcePool(1),
+                                                     new MockDispatcher(), 
+                                                     new SummaryParameters(null),
                                                      new ClusterParams("testhittype"),
-                                                     new CacheParams(100, 1e64), documentdbConfigWithOneDb);
+                                                     new CacheParams(100, 1e64), 
+                                                     documentdbConfigWithOneDb);
 
         Query query = new Query("?query=foo&model.restrict=testDb");
         query.prepare();
@@ -285,19 +293,25 @@ public class FastSearcherTestCase {
 
         MockFSChannel.resetDocstamp();
         Logger.getLogger(FastSearcher.class.getName()).setLevel(Level.ALL);
-        return new FastSearcher(mockBackend, new MockDispatcher(), new SummaryParameters(null),
-                                new ClusterParams("testhittype"), new CacheParams(100, 1e64), config);
+        return new FastSearcher(mockBackend,
+                                new FS4ResourcePool(1),
+                                new MockDispatcher(), 
+                                new SummaryParameters(null),
+                                new ClusterParams("testhittype"), 
+                                new CacheParams(100, 1e64), config);
     }
 
     @Ignore
     public void testSinglePhaseCachedSupersets() {
         Logger.getLogger(FastSearcher.class.getName()).setLevel(Level.ALL);
         MockFSChannel.resetDocstamp();
-        FastSearcher fastSearcher = new FastSearcher(new MockBackend(), new MockDispatcher(),
-                new SummaryParameters(null),
-                new ClusterParams("testhittype"),
-                new CacheParams(100, 1e64),
-                documentdbInfoConfig);
+        FastSearcher fastSearcher = new FastSearcher(new MockBackend(),
+                                                     new FS4ResourcePool(1),
+                                                     new MockDispatcher(),
+                                                     new SummaryParameters(null),
+                                                     new ClusterParams("testhittype"),
+                                                     new CacheParams(100, 1e64),
+                                                     documentdbInfoConfig);
 
         CacheControl c = fastSearcher.getCacheControl();
 
@@ -334,11 +348,13 @@ public class FastSearcherTestCase {
     public void testMultiPhaseCachedSupersets() {
         Logger.getLogger(FastSearcher.class.getName()).setLevel(Level.ALL);
         MockFSChannel.resetDocstamp();
-        FastSearcher fastSearcher = new FastSearcher(new MockBackend(), new MockDispatcher(),
-                new SummaryParameters(null),
-                new ClusterParams("testhittype"),
-                new CacheParams(100, 1e64),
-                documentdbInfoConfig);
+        FastSearcher fastSearcher = new FastSearcher(new MockBackend(),
+                                                     new FS4ResourcePool(1),
+                                                     new MockDispatcher(),
+                                                     new SummaryParameters(null),
+                                                     new ClusterParams("testhittype"),
+                                                     new CacheParams(100, 1e64),
+                                                     documentdbInfoConfig);
 
         Result result = doSearch(fastSearcher,new Query("?query=ignored"), 0, 2);
         result = doSearch(fastSearcher,new Query("?query=ignored"), 1, 1);
@@ -368,13 +384,15 @@ public class FastSearcherTestCase {
         BackendTestCase.MockServer server = new BackendTestCase.MockServer();
         FS4ResourcePool listeners = new FS4ResourcePool(new Fs4Config());
         Backend backend = listeners.getBackend(server.host.getHostString(),server.host.getPort());
-        FastSearcher fastSearcher = new FastSearcher(backend, new MockDispatcher(),
-                new SummaryParameters(null),
-                new ClusterParams("testhittype"),
-                new CacheParams(0, 0.0d),
-                documentdbInfoConfig);
+        FastSearcher fastSearcher = new FastSearcher(backend,
+                                                     new FS4ResourcePool(1),
+                                                     new MockDispatcher(),
+                                                     new SummaryParameters(null),
+                                                     new ClusterParams("testhittype"),
+                                                     new CacheParams(0, 0.0d),
+                                                     documentdbInfoConfig);
         server.dispatch.packetData = BackendTestCase.PONG;
-        Chain<Searcher> chain = new Chain<Searcher>(fastSearcher);
+        Chain<Searcher> chain = new Chain<>(fastSearcher);
         Execution e = new Execution(chain, Execution.Context.createContextStub());
         Pong pong = e.ping(new Ping());
         assertEquals(127, pong.getPongPacket(0).getDocstamp());
@@ -393,9 +411,9 @@ public class FastSearcherTestCase {
         assertEquals("", other.getPingInfo());
         pong.setPingInfo("blbl");
         assertEquals("Result of pinging using blbl error : Service is misconfigured (as usual)",
-                pong.toString());
+                     pong.toString());
         assertEquals("Result of pinging error : Service is misconfigured (as usual)",
-                other.toString());
+                     other.toString());
     }
 
     private void clearCache(FastSearcher fastSearcher) {