Do not monitor fs4

3 files changed, 31 insertions, 299 deletions

diff --git a/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterMonitor.java b/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterMonitor.java
deleted file mode 100644
index c075a0f842b..00000000000
--- a/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterMonitor.java
+++ /dev/null
@@ -1,161 +0,0 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.prelude.cluster;
-
-import java.util.Map;
-import java.util.Optional;
-import java.util.concurrent.ScheduledFuture;
-import java.util.concurrent.TimeUnit;
-import java.util.logging.Level;
-import java.util.logging.Logger;
-
-import com.yahoo.component.provider.Freezable;
-import com.yahoo.container.handler.VipStatus;
-import com.yahoo.prelude.fastsearch.VespaBackEndSearcher;
-import com.yahoo.search.result.ErrorMessage;
-
-/**
- * Monitors of a cluster of remote nodes. The monitor uses an internal thread
- * for node monitoring.
- *
- * @author bratseth
- * @author Steinar Knutsen
- */
-public class ClusterMonitor implements Runnable, Freezable {
-
-    // The ping thread wil start using the system, but we cannot be guaranteed that all components
-    // in the system is up. As a workaround for not being able to find out when the system
-    // is ready to be used, we wait some time before starting the ping thread
-    private static final int pingThreadInitialDelayMs = 3000;
-
-    private final MonitorConfiguration configuration;
-
-    private final static Logger log = Logger.getLogger(ClusterMonitor.class.getName());
-
-    private final ClusterSearcher nodeManager;
-
-    private final Optional<VipStatus> vipStatus;
-
-    /** A map from Node to corresponding MonitoredNode */
-    private final Map<VespaBackEndSearcher, NodeMonitor> nodeMonitors = new java.util.IdentityHashMap<>();
-
-    private ScheduledFuture<?>  future;
-
-    private boolean isFrozen = false;
-
-    ClusterMonitor(ClusterSearcher manager, QrMonitorConfig monitorConfig, Optional<VipStatus> vipStatus) {
-        configuration = new MonitorConfiguration(monitorConfig);
-        nodeManager = manager;
-        this.vipStatus = vipStatus;
-        log.fine("checkInterval is " + configuration.getCheckInterval() + " ms");
-    }
-
-    /** Returns the configuration of this cluster monitor */
-    MonitorConfiguration getConfiguration() {
-        return configuration;
-    }
-
-    void startPingThread() {
-        if ( ! isFrozen())
-            throw new IllegalStateException("Do not start the monitoring thread before the set of " +
-                                            "nodes to monitor is complete/the ClusterMonitor is frozen.");
-        future = nodeManager.getScheduledExecutor().scheduleAtFixedRate(this, pingThreadInitialDelayMs, configuration.getCheckInterval(), TimeUnit.MILLISECONDS);
-    }
-
-    /**
-     * Adds a new node for monitoring.
-     */
-    void add(VespaBackEndSearcher node) {
-        if (isFrozen())
-            throw new IllegalStateException("Can not add new nodes after ClusterMonitor has been frozen.");
-        nodeMonitors.put(node, new NodeMonitor(node));
-        updateVipStatus();
-    }
-
-    /** Called from ClusterSearcher/NodeManager when a node failed */
-    void failed(VespaBackEndSearcher node, ErrorMessage error) {
-        NodeMonitor monitor = nodeMonitors.get(node);
-        boolean wasWorking = monitor.isWorking();
-        monitor.failed(error);
-        if (wasWorking && !monitor.isWorking()) {
-            log.info("Failed monitoring node '" + node + "' due to '" + error);
-            nodeManager.failed(node);
-        }
-        updateVipStatus();
-    }
-
-    /** Called when a node responded */
-    void responded(VespaBackEndSearcher node, boolean hasSearchNodesOnline) {
-        NodeMonitor monitor = nodeMonitors.get(node);
-        boolean wasFailing = !monitor.isWorking();
-        monitor.responded(hasSearchNodesOnline);
-        if (wasFailing && monitor.isWorking()) {
-            log.info("Failed node '" + node + "' started working again.");
-            nodeManager.working(node);
-        }
-        updateVipStatus();
-    }
-
-    private void updateVipStatus() {
-        if ( ! vipStatus.isPresent()) return;
-        if ( ! hasInformationAboutAllNodes()) return;
-        
-        if (hasWorkingNodesWithDocumentsOnline()) {
-            vipStatus.get().addToRotation(nodeManager.getId().stringValue());
-        } else {
-            vipStatus.get().removeFromRotation(nodeManager.getId().stringValue());
-        }
-    }
-
-    private boolean hasInformationAboutAllNodes() {
-        for (NodeMonitor monitor : nodeMonitors.values()) {
-            if ( ! monitor.statusIsKnown())
-                return false;
-        }
-        return true;
-    }
-
-    private boolean hasWorkingNodesWithDocumentsOnline() {
-        for (NodeMonitor node : nodeMonitors.values()) {
-            if (node.isWorking() && node.searchNodesOnline())
-                return true;
-        }
-        return false;
-    }
-
-    /**
-     * Ping all nodes which needs pinging to discover state changes
-     */
-    private void ping() throws InterruptedException {
-        for (NodeMonitor monitor : nodeMonitors.values()) {
-            nodeManager.ping(monitor.getNode());
-        }
-    }
-
-    @Override
-    public void run() {
-        log.finest("Activating ping");
-        try {
-            ping();
-        } catch (Exception e) {
-            log.log(Level.WARNING, "Error in monitor thread", e);
-        }
-    }
-
-    public void shutdown() {
-        if (future != null) {
-            future.cancel(true);
-        }
-    }
-
-    @Override
-    public void freeze() {
-        isFrozen  = true;
-
-    }
-
-    @Override
-    public boolean isFrozen() {
-        return isFrozen;
-    }
-
-}
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 3a284fddee4..0780d5e9d65 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
@@ -2,19 +2,13 @@
 package com.yahoo.prelude.cluster;
 
 import com.yahoo.cloud.config.ClusterInfoConfig;
-import com.yahoo.collections.Tuple2;
 import com.yahoo.component.ComponentId;
-import com.yahoo.component.chain.Chain;
 import com.yahoo.component.chain.dependencies.After;
-import com.yahoo.concurrent.Receiver;
-import com.yahoo.concurrent.Receiver.MessageState;
 import com.yahoo.container.QrSearchersConfig;
 import com.yahoo.container.handler.VipStatus;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.net.HostName;
 import com.yahoo.prelude.IndexFacts;
-import com.yahoo.prelude.Ping;
-import com.yahoo.prelude.Pong;
 import com.yahoo.prelude.fastsearch.ClusterParams;
 import com.yahoo.prelude.fastsearch.DocumentdbInfoConfig;
 import com.yahoo.prelude.fastsearch.FS4ResourcePool;
@@ -45,11 +39,7 @@ import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.Set;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.logging.Logger;
 
 import static com.yahoo.container.QrSearchersConfig.Searchcluster.Indexingmode.STREAMING;
 
@@ -63,10 +53,6 @@ import static com.yahoo.container.QrSearchersConfig.Searchcluster.Indexingmode.S
 @After("*")
 public class ClusterSearcher extends Searcher {
 
-    private final static Logger log = Logger.getLogger(ClusterSearcher.class.getName());
-
-    private final ClusterMonitor monitor;
-
     private final Value cacheHitRatio;
 
     private final String clusterModelName;
@@ -77,8 +63,6 @@ public class ClusterSearcher extends Searcher {
     // Mapping from rank profile names to document types containing them
     private final Map<String, Set<String>> rankProfiles = new HashMap<>();
 
-    private final FS4ResourcePool fs4ResourcePool;
-
     private final long maxQueryTimeout; // in milliseconds
     private final static long DEFAULT_MAX_QUERY_TIMEOUT = 600000L;
 
@@ -87,7 +71,6 @@ public class ClusterSearcher extends Searcher {
 
     private VespaBackEndSearcher server = null;
 
-
     /**
      * Creates a new ClusterSearcher.
      */
@@ -95,7 +78,6 @@ public class ClusterSearcher extends Searcher {
                            QrSearchersConfig qrsConfig,
                            ClusterConfig clusterConfig,
                            DocumentdbInfoConfig documentDbConfig,
-                           QrMonitorConfig monitorConfig,
                            DispatchConfig dispatchConfig,
                            ClusterInfoConfig clusterInfoConfig,
                            Statistics manager,
@@ -103,14 +85,9 @@ public class ClusterSearcher extends Searcher {
                            FS4ResourcePool fs4ResourcePool,
                            VipStatus vipStatus) {
         super(id);
-        this.fs4ResourcePool = fs4ResourcePool;
 
         Dispatcher dispatcher = Dispatcher.create(id.stringValue(), dispatchConfig, clusterInfoConfig.nodeCount(), vipStatus, metric);
 
-        monitor = (dispatcher.searchCluster().directDispatchTarget().isPresent()) // dispatcher should decide vip status instead
-                ? new ClusterMonitor(this, monitorConfig, Optional.empty())
-                : new ClusterMonitor(this, monitorConfig, Optional.of(vipStatus));
-
         int searchClusterIndex = clusterConfig.clusterId();
         clusterModelName = clusterConfig.clusterName();
         QrSearchersConfig.Searchcluster searchClusterConfig = getSearchClusterConfigFromClusterName(qrsConfig, clusterModelName);
@@ -160,8 +137,6 @@ public class ClusterSearcher extends Searcher {
         if ( server == null ) {
             throw new IllegalStateException("ClusterSearcher should have a top level dispatch.");
         }
-        monitor.freeze();
-        monitor.startPingThread();
     }
 
     private static QrSearchersConfig.Searchcluster getSearchClusterConfigFromClusterName(QrSearchersConfig config, String name) {
@@ -219,21 +194,14 @@ public class ClusterSearcher extends Searcher {
     /** Do not use, for internal testing purposes only. **/
     ClusterSearcher(Set<String> documentTypes) {
         this.documentTypes = documentTypes;
-        monitor = new ClusterMonitor(this, new QrMonitorConfig(new QrMonitorConfig.Builder()), Optional.of(new VipStatus()));
         cacheHitRatio = new Value("com.yahoo.prelude.cluster.ClusterSearcher.ClusterSearcher().dummy",
                                   Statistics.nullImplementation, new Value.Parameters());
         clusterModelName = "testScenario";
-        fs4ResourcePool = null;
         maxQueryTimeout = DEFAULT_MAX_QUERY_TIMEOUT;
         maxQueryCacheTimeout = DEFAULT_MAX_QUERY_CACHE_TIMEOUT;
     }
 
-    ClusterMonitor getMonitor() {
-        return monitor;
-    }
-
     void addBackendSearcher(VespaBackEndSearcher searcher) {
-        monitor.add(searcher);
         server = searcher;
     }
 
@@ -472,77 +440,6 @@ public class ClusterSearcher extends Searcher {
         cacheHitRatio.put(0.0);
     }
 
-    /** NodeManager method, called from ClusterMonitor. */
-    void working(VespaBackEndSearcher node) {
-        server = node;
-    }
-
-    /** Called from ClusterMonitor. */
-    void failed(VespaBackEndSearcher node) {
-        server = null;
-    }
-
-    /**
-     * Pinging a node, called from ClusterMonitor.
-     */
-    void ping(VespaBackEndSearcher node) throws InterruptedException {
-        log.fine("Sending ping to: " + node);
-        Pinger pinger = new Pinger(node);
-
-        getExecutor().execute(pinger);
-        Pong pong = pinger.getPong(); // handles timeout
-        if (pong == null) {
-            monitor.failed(node, ErrorMessage.createNoAnswerWhenPingingNode("Ping thread timed out."));
-        } else if (pong.badResponse()) {
-            monitor.failed(node, pong.getError(0));
-        } else {
-            monitor.responded(node, backendCanServeDocuments(pong));
-        }
-    }
-
-    private boolean backendCanServeDocuments(Pong pong) {
-        if ( ! pong.activeNodes().isPresent()) return true; // no information; assume true
-        return pong.activeNodes().get() > 0;
-    }
-
     @Override
-    public void deconstruct() {
-        monitor.shutdown();
-    }
-
-    private ExecutorService getExecutor() {
-        return fs4ResourcePool.getExecutor();
-    }
-
-    ScheduledExecutorService getScheduledExecutor() {
-        return fs4ResourcePool.getScheduledExecutor();
-    }
-
-    private class Pinger implements Runnable {
-
-        private final Searcher searcher;
-        private final Ping pingChallenge = new Ping(monitor.getConfiguration().getRequestTimeout());
-        private final Receiver<Pong> pong = new Receiver<>();
-
-        Pinger(final Searcher searcher) {
-            this.searcher = searcher;
-        }
-
-        @Override
-        public void run() {
-            pong.put(createExecution().ping(pingChallenge));
-        }
-
-        private Execution createExecution() {
-            return new Execution(new Chain<>(searcher),
-                                 new Execution.Context(null, null, null, null, null));
-        }
-
-        public Pong getPong() throws InterruptedException {
-            Tuple2<MessageState, Pong> reply = pong.get(pingChallenge.getTimeout() + 150);
-            return (reply.first != MessageState.VALID) ? null : reply.second;
-        }
-
-    }
-
+    public void deconstruct() { }
 }
diff --git a/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java b/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java
index 11d4bc62b7c..9ea7276583b 100644
--- a/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java
+++ b/container-search/src/test/java/com/yahoo/prelude/cluster/ClusterSearcherTestCase.java
@@ -7,7 +7,6 @@ import com.yahoo.container.QrConfig;
 import com.yahoo.container.QrSearchersConfig;
 import com.yahoo.container.handler.VipStatus;
 import com.yahoo.container.protect.Error;
-import com.yahoo.container.search.Fs4Config;
 import com.yahoo.prelude.IndexFacts;
 import com.yahoo.prelude.IndexModel;
 import com.yahoo.prelude.SearchDefinition;
@@ -47,12 +46,12 @@ import static org.junit.Assert.assertTrue;
  * @author bratseth
  */
 public class ClusterSearcherTestCase {
+    private static final double DELTA = 0.0000000000000001;
 
     @Test
     public void testNoBackends() {
         ClusterSearcher cluster = new ClusterSearcher(new LinkedHashSet<>(Arrays.asList("dummy")));
         try {
-            cluster.getMonitor().getConfiguration().setRequestTimeout(100);
             Execution execution = new Execution(cluster, Execution.Context.createContextStub());
             Query query = new Query("query=hello");
             query.setHits(10);
@@ -145,7 +144,7 @@ public class ClusterSearcherTestCase {
         private final String type3 = "type3";
         private final Map<String, List<Hit>> results = new LinkedHashMap<>();
         private final boolean expectAttributePrefetch;
-        public static final String ATTRIBUTE_PREFETCH = "attributeprefetch";
+        static final String ATTRIBUTE_PREFETCH = "attributeprefetch";
 
         private String getId(String type, int i) {
             return "id:ns:" + type + "::" + i;
@@ -195,7 +194,7 @@ public class ClusterSearcherTestCase {
                                              createHit(getId(type3, 2), 5)));
         }
 
-        public MyMockSearcher(boolean expectAttributePrefetch) {
+        MyMockSearcher(boolean expectAttributePrefetch) {
             this.expectAttributePrefetch = expectAttributePrefetch;
             init();
         }
@@ -262,8 +261,7 @@ public class ClusterSearcherTestCase {
     }
 
     private Execution createExecution(List<String> docTypesList, boolean expectAttributePrefetch) {
-        Set<String> documentTypes = new LinkedHashSet<>();
-        documentTypes.addAll(docTypesList);
+        Set<String> documentTypes = new LinkedHashSet<>(docTypesList);
         ClusterSearcher cluster = new ClusterSearcher(documentTypes);
         try {
             cluster.addBackendSearcher(new MyMockSearcher(
@@ -276,6 +274,7 @@ public class ClusterSearcherTestCase {
         }
     }
 
+    @Test
     public void testThatSingleDocumentTypeCanBeSearched() {
         { // Explicit 1 type in restrict set
             Execution execution = createExecution();
@@ -284,9 +283,9 @@ public class ClusterSearcherTestCase {
             assertEquals(3, result.getTotalHitCount());
             List<Hit> hits = result.hits().asList();
             assertEquals(3, hits.size());
-            assertEquals(9.0, hits.get(0).getRelevance().getScore());
-            assertEquals(6.0, hits.get(1).getRelevance().getScore());
-            assertEquals(3.0, hits.get(2).getRelevance().getScore());
+            assertEquals(9.0, hits.get(0).getRelevance().getScore(), DELTA);
+            assertEquals(6.0, hits.get(1).getRelevance().getScore(), DELTA);
+            assertEquals(3.0, hits.get(2).getRelevance().getScore(), DELTA);
         }
         { // Only 1 registered type in cluster searcher, empty restrict set
             // NB ! Empty restrict sets does not exist below the cluster searcher.
@@ -298,10 +297,11 @@ public class ClusterSearcherTestCase {
             assertEquals(3, result.getTotalHitCount());
             List<Hit> hits = result.hits().asList();
             assertEquals(3, hits.size());
-            assertEquals(9.0, hits.get(0).getRelevance().getScore());
+            assertEquals(9.0, hits.get(0).getRelevance().getScore(), DELTA);
         }
     }
 
+    @Test
     public void testThatSubsetOfDocumentTypesCanBeSearched() {
         Execution execution = createExecution();
         Query query = new Query("?query=hello&restrict=type1,type3");
@@ -310,14 +310,15 @@ public class ClusterSearcherTestCase {
         assertEquals(6, result.getTotalHitCount());
         List<Hit> hits = result.hits().asList();
         assertEquals(6, hits.size());
-        assertEquals(11.0, hits.get(0).getRelevance().getScore());
-        assertEquals(9.0,  hits.get(1).getRelevance().getScore());
-        assertEquals(8.0,  hits.get(2).getRelevance().getScore());
-        assertEquals(6.0,  hits.get(3).getRelevance().getScore());
-        assertEquals(5.0,  hits.get(4).getRelevance().getScore());
-        assertEquals(3.0,  hits.get(5).getRelevance().getScore());
+        assertEquals(11.0, hits.get(0).getRelevance().getScore(), DELTA);
+        assertEquals(9.0,  hits.get(1).getRelevance().getScore(), DELTA);
+        assertEquals(8.0,  hits.get(2).getRelevance().getScore(), DELTA);
+        assertEquals(6.0,  hits.get(3).getRelevance().getScore(), DELTA);
+        assertEquals(5.0,  hits.get(4).getRelevance().getScore(), DELTA);
+        assertEquals(3.0,  hits.get(5).getRelevance().getScore(), DELTA);
     }
 
+    @Test
     public void testThatMultipleDocumentTypesCanBeSearchedAndFilled() {
         Execution execution = createExecution();
         Query query = new Query("?query=hello");
@@ -326,15 +327,15 @@ public class ClusterSearcherTestCase {
         assertEquals(9, result.getTotalHitCount());
         List<Hit> hits = result.hits().asList();
         assertEquals(9, hits.size());
-        assertEquals(11.0, hits.get(0).getRelevance().getScore());
-        assertEquals(10.0, hits.get(1).getRelevance().getScore());
-        assertEquals(9.0,  hits.get(2).getRelevance().getScore());
-        assertEquals(8.0,  hits.get(3).getRelevance().getScore());
-        assertEquals(7.0,  hits.get(4).getRelevance().getScore());
-        assertEquals(6.0,  hits.get(5).getRelevance().getScore());
-        assertEquals(5.0,  hits.get(6).getRelevance().getScore());
-        assertEquals(4.0,  hits.get(7).getRelevance().getScore());
-        assertEquals(3.0,  hits.get(8).getRelevance().getScore());
+        assertEquals(11.0, hits.get(0).getRelevance().getScore(), DELTA);
+        assertEquals(10.0, hits.get(1).getRelevance().getScore(), DELTA);
+        assertEquals(9.0,  hits.get(2).getRelevance().getScore(), DELTA);
+        assertEquals(8.0,  hits.get(3).getRelevance().getScore(), DELTA);
+        assertEquals(7.0,  hits.get(4).getRelevance().getScore(), DELTA);
+        assertEquals(6.0,  hits.get(5).getRelevance().getScore(), DELTA);
+        assertEquals(5.0,  hits.get(6).getRelevance().getScore(), DELTA);
+        assertEquals(4.0,  hits.get(7).getRelevance().getScore(), DELTA);
+        assertEquals(3.0,  hits.get(8).getRelevance().getScore(), DELTA);
         for (int i = 0; i < 9; ++i) {
             assertNull(hits.get(i).getField("score"));
         }
@@ -389,7 +390,7 @@ public class ClusterSearcherTestCase {
         assertResult(9, Arrays.asList(5.0, 4.0),   getResult(6, 2, ex));
         assertResult(9, Arrays.asList(4.0, 3.0),   getResult(7, 2, ex));
         assertResult(9, Arrays.asList(3.0),        getResult(8, 2, ex));
-        assertResult(9, new ArrayList<Double>(), getResult(9, 2, ex));
+        assertResult(9, new ArrayList<>(), getResult(9, 2, ex));
         assertResult(9, Arrays.asList(11.0, 10.0, 9.0, 8.0, 7.0), getResult(0, 5, ex));
         assertResult(9, Arrays.asList(6.0, 5.0, 4.0, 3.0),        getResult(5, 5, ex));
 
@@ -424,11 +425,7 @@ public class ClusterSearcherTestCase {
         final String yahoo = "www.yahoo.com";
 
         try {
-            if (null != InetAddress.getByName(yahoo)) {
-                canFindYahoo = true;
-            } else {
-                canFindYahoo = false;
-            }
+            canFindYahoo = (null != InetAddress.getByName(yahoo));
         } catch (Exception e) {
             canFindYahoo = false;
         }
@@ -541,7 +538,6 @@ public class ClusterSearcherTestCase {
                                    qrSearchersConfig.build(),
                                    clusterConfig.build(),
                                    documentDbConfig.build(),
-                                   new QrMonitorConfig.Builder().build(),
                                    new DispatchConfig.Builder().build(),
                                    createClusterInfoConfig(),
                                    Statistics.nullImplementation,
@@ -589,7 +585,7 @@ public class ClusterSearcherTestCase {
 
     @Test
     public void testThatQueryTimeoutIsCappedWithSpecifiedMax() {
-        QueryTimeoutFixture f = new QueryTimeoutFixture(Double.valueOf(70), null);
+        QueryTimeoutFixture f = new QueryTimeoutFixture(70.0, null);
         f.query.setTimeout(70001);
         f.search();
         assertEquals(70000, f.query.getTimeout());
@@ -615,7 +611,7 @@ public class ClusterSearcherTestCase {
 
     @Test
     public void testThatQueryCacheIsDisabledIfTimeoutIsLargerThanConfiguredMax() {
-        QueryTimeoutFixture f = new QueryTimeoutFixture(null, Double.valueOf(5));
+        QueryTimeoutFixture f = new QueryTimeoutFixture(null, 5.0);
         f.query.setTimeout(5001);
         f.query.getRanking().setQueryCache(true);
         f.search();