Do not start cluster monitor thread in test as it will race with explicit ping in test.

5 files changed, 17 insertions, 8 deletions

diff --git a/container-search/src/main/java/com/yahoo/search/cluster/ClusterMonitor.java b/container-search/src/main/java/com/yahoo/search/cluster/ClusterMonitor.java
index d4b6279be89..d871a256a62 100644
--- a/container-search/src/main/java/com/yahoo/search/cluster/ClusterMonitor.java
+++ b/container-search/src/main/java/com/yahoo/search/cluster/ClusterMonitor.java
@@ -38,6 +38,9 @@ public class ClusterMonitor<T> {
     /** A map from Node to corresponding MonitoredNode */
     private final Map<T, TrafficNodeMonitor<T>> nodeMonitors = Collections.synchronizedMap(new java.util.LinkedHashMap<>());
 
+    /** @deprecated It is not advised to start the monitoring thread in the constructor.
+     * Use ClusterMonitor(NodeManager<T> manager, false) and explicit start(). */
+    @Deprecated
     public ClusterMonitor(NodeManager<T> manager) {
         this(manager, true);
     }
@@ -50,6 +53,12 @@ public class ClusterMonitor<T> {
         }
     }
 
+    public void start() {
+        if ( ! monitorThread.isAlive()) {
+            monitorThread.start();
+        }
+    }
+
     /** Returns the configuration of this cluster monitor */
     public MonitorConfiguration getConfiguration() { return configuration; }
 
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 3fb0059ecb9..45e85c10f3f 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
@@ -124,7 +124,7 @@ public class Dispatcher extends AbstractComponent {
         this.metricContext = metric.createContext(null);
         this.maxHitsPerNode = dispatchConfig.maxHitsPerNode();
 
-        searchCluster.startClusterMonitoring(pingFactory);
+        searchCluster.startClusterMonitoring(pingFactory, true);
         try {
             while ( ! searchCluster.hasInformationAboutAllNodes()) {
                 Thread.sleep(1);
diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java
index cbe24eb6907..54d49569ede 100644
--- a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java
+++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java
@@ -82,20 +82,23 @@ public class SearchCluster implements NodeManager<Node> {
                                                                        nodesByHost,
                                                                        groups);
 
-        this.clusterMonitor = new ClusterMonitor<>(this);
+        this.clusterMonitor = new ClusterMonitor<>(this, false);
     }
 
     public void shutDown() {
         clusterMonitor.shutdown();
     }
 
-    public void startClusterMonitoring(PingFactory pingFactory) {
+    public void startClusterMonitoring(PingFactory pingFactory, boolean startPingThread) {
         this.pingFactory = pingFactory;
 
         for (var group : orderedGroups) {
             for (var node : group.nodes())
                 clusterMonitor.add(node, true);
         }
+        if (startPingThread) {
+            clusterMonitor.start();
+        }
     }
 
     ClusterMonitor<Node> clusterMonitor() { return clusterMonitor; }
diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/DispatcherTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/DispatcherTest.java
index a1ae3b6a19d..4cfb5d36b63 100644
--- a/container-search/src/test/java/com/yahoo/search/dispatch/DispatcherTest.java
+++ b/container-search/src/test/java/com/yahoo/search/dispatch/DispatcherTest.java
@@ -1,10 +1,8 @@
 // Copyright 2019 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.search.dispatch;
 
-import com.yahoo.prelude.Pong;
 import com.yahoo.prelude.fastsearch.VespaBackEndSearcher;
 import com.yahoo.prelude.fastsearch.test.MockMetric;
-import com.yahoo.processing.request.CompoundName;
 import com.yahoo.search.Query;
 import com.yahoo.search.Result;
 import com.yahoo.search.cluster.ClusterMonitor;
@@ -18,7 +16,6 @@ import org.junit.Test;
 import java.util.List;
 import java.util.Optional;
 import java.util.OptionalInt;
-import java.util.concurrent.Callable;
 
 import static com.yahoo.search.dispatch.MockSearchCluster.createDispatchConfig;
 import static org.junit.Assert.assertEquals;
diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTest.java
index aaef06363a0..7d5289e74eb 100644
--- a/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTest.java
+++ b/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterTest.java
@@ -62,7 +62,7 @@ public class SearchClusterTest {
         }
 
         void startMonitoring() {
-            searchCluster.startClusterMonitoring(new Factory(nodesPerGroup, numDocsPerNode, pingCounts));
+            searchCluster.startClusterMonitoring(new Factory(nodesPerGroup, numDocsPerNode, pingCounts), false);
         }
 
         private int maxPingCount() {
@@ -87,7 +87,7 @@ public class SearchClusterTest {
 
         void waitOneFullPingRound() {
             int minPingCount = minPingCount();
-            int atLeast = maxPingCount() + 2;
+            int atLeast = maxPingCount() + 1;
             while (minPingCount < atLeast) {
                 ExecutorService executor = Executors.newCachedThreadPool();
                 searchCluster.clusterMonitor().ping(executor);