When pooling ClusterMonitors and SearchClusters you will add a cluster every time there is a reconfig.

As nothing will purge them, you both keep stuff alive forever and end up with more clusters that you have.
Hence the magic for not removing vipstatus when there are multiple clusters kick in preventing nodes being taken OOR.
Now it is using the ComponentId for identifying a cluster.

10 files changed, 37 insertions, 47 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
index 4e708e32a2d..c075a0f842b 100644
--- a/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterMonitor.java
+++ b/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterMonitor.java
@@ -100,9 +100,9 @@ public class ClusterMonitor implements Runnable, Freezable {
         if ( ! hasInformationAboutAllNodes()) return;
         
         if (hasWorkingNodesWithDocumentsOnline()) {
-            vipStatus.get().addToRotation(this);
+            vipStatus.get().addToRotation(nodeManager.getId().stringValue());
         } else {
-            vipStatus.get().removeFromRotation(this);
+            vipStatus.get().removeFromRotation(nodeManager.getId().stringValue());
         }
     }
 
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 2fdcad32c32..3cb95fd0f7f 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
@@ -110,7 +110,7 @@ public class ClusterSearcher extends Searcher {
         super(id);
         this.fs4ResourcePool = fs4ResourcePool;
 
-        Dispatcher dispatcher = new Dispatcher(dispatchConfig, fs4ResourcePool, clusterInfoConfig.nodeCount(), vipStatus);
+        Dispatcher dispatcher = new Dispatcher(id.stringValue(), dispatchConfig, fs4ResourcePool, clusterInfoConfig.nodeCount(), vipStatus);
 
         monitor = (dispatcher.searchCluster().directDispatchTarget().isPresent()) // dispatcher should decide vip status instead
                 ? new ClusterMonitor(this, monitorConfig, Optional.empty())
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 708d80d4969..39fc0d31681 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
@@ -49,22 +49,14 @@ public class Dispatcher extends AbstractComponent {
     private final RpcResourcePool rpcResourcePool;
     private final boolean multilevelDispatch;
 
-    public Dispatcher(DispatchConfig dispatchConfig, FS4ResourcePool fs4ResourcePool, int containerClusterSize, VipStatus vipStatus) {
-        this.searchCluster = new SearchCluster(dispatchConfig, fs4ResourcePool, containerClusterSize, vipStatus);
+    public Dispatcher(String clusterId, DispatchConfig dispatchConfig, FS4ResourcePool fs4ResourcePool, int containerClusterSize, VipStatus vipStatus) {
+        this.searchCluster = new SearchCluster(clusterId, dispatchConfig, fs4ResourcePool, containerClusterSize, vipStatus);
         this.loadBalancer = new LoadBalancer(searchCluster,
                 dispatchConfig.distributionPolicy() == DispatchConfig.DistributionPolicy.ROUNDROBIN);
         this.rpcResourcePool = new RpcResourcePool(dispatchConfig);
         this.multilevelDispatch = dispatchConfig.useMultilevelDispatch();
     }
 
-    /** For testing */
-    public Dispatcher(Map<Integer, Client.NodeConnection> nodeConnections, Client client) {
-        this.searchCluster = null;
-        this.loadBalancer = new LoadBalancer(searchCluster, true);
-        this.rpcResourcePool = new RpcResourcePool(client, nodeConnections);
-        this.multilevelDispatch = false;
-    }
-
     /** Returns the search cluster this dispatches to */
     public SearchCluster searchCluster() {
         return searchCluster;
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 b51620a978b..b8d76906f70 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
@@ -41,6 +41,7 @@ public class SearchCluster implements NodeManager<Node> {
     private final double minGroupCoverage;
     private final int maxNodesDownPerGroup;
     private final int size;
+    private final String clusterId;
     private final ImmutableMap<Integer, Group> groups;
     private final ImmutableMultimap<String, Node> nodesByHost;
     private final ImmutableList<Group> orderedGroups;
@@ -60,13 +61,14 @@ public class SearchCluster implements NodeManager<Node> {
     // Only needed until query requests are moved to rpc
     private final FS4ResourcePool fs4ResourcePool;
 
-    public SearchCluster(DispatchConfig dispatchConfig, FS4ResourcePool fs4ResourcePool, int containerClusterSize, VipStatus vipStatus) {
-        this(dispatchConfig.minActivedocsPercentage(), dispatchConfig.minGroupCoverage(), dispatchConfig.maxNodesDownPerGroup(),
+    public SearchCluster(String clusterId, DispatchConfig dispatchConfig, FS4ResourcePool fs4ResourcePool, int containerClusterSize, VipStatus vipStatus) {
+        this(clusterId, dispatchConfig.minActivedocsPercentage(), dispatchConfig.minGroupCoverage(), dispatchConfig.maxNodesDownPerGroup(),
                 toNodes(dispatchConfig), fs4ResourcePool, containerClusterSize, vipStatus);
     }
 
-    public SearchCluster(double minActivedocsCoverage, double minGroupCoverage, int maxNodesDownPerGroup, List<Node> nodes, FS4ResourcePool fs4ResourcePool,
+    public SearchCluster(String clusterId, double minActivedocsCoverage, double minGroupCoverage, int maxNodesDownPerGroup, List<Node> nodes, FS4ResourcePool fs4ResourcePool,
             int containerClusterSize, VipStatus vipStatus) {
+        this.clusterId = clusterId;
         this.minActivedocsCoveragePercentage = minActivedocsCoverage;
         this.minGroupCoverage = minGroupCoverage;
         this.maxNodesDownPerGroup = maxNodesDownPerGroup;
@@ -194,7 +196,7 @@ public class SearchCluster implements NodeManager<Node> {
         node.setWorking(true);
 
         if (usesDirectDispatchTo(node))
-            vipStatus.addToRotation(this);
+            vipStatus.addToRotation(clusterId);
     }
 
     /** Used by the cluster monitor to manage node status */
@@ -204,16 +206,16 @@ public class SearchCluster implements NodeManager<Node> {
 
         // Take ourselves out if we usually dispatch only to our own host
         if (usesDirectDispatchTo(node))
-            vipStatus.removeFromRotation(this);
+            vipStatus.removeFromRotation(clusterId);
     }
 
     private void updateSufficientCoverage(Group group, boolean sufficientCoverage) {
         // update VIP status if we direct dispatch to this group and coverage status changed
         if (usesDirectDispatchTo(group) && sufficientCoverage != group.hasSufficientCoverage()) {
             if (sufficientCoverage) {
-                vipStatus.addToRotation(this);
+                vipStatus.addToRotation(clusterId);
             } else {
-                vipStatus.removeFromRotation(this);
+                vipStatus.removeFromRotation(clusterId);
             }
         }
         group.setHasSufficientCoverage(sufficientCoverage);
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 79b43563c6a..3b8155efc95 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
@@ -83,7 +83,7 @@ public class FastSearcherTestCase {
         Logger.getLogger(FastSearcher.class.getName()).setLevel(Level.ALL);
         FastSearcher fastSearcher = new FastSearcher(new MockBackend(),
                                                      new FS4ResourcePool(1),
-                                                     new MockDispatcher(Collections.emptyList()),
+                                                     new MockDispatcher("a", Collections.emptyList()),
                                                      new SummaryParameters(null),
                                                      new ClusterParams("testhittype"),
                                                      new CacheParams(100, 1e64),
@@ -104,7 +104,7 @@ public class FastSearcherTestCase {
         Logger.getLogger(FastSearcher.class.getName()).setLevel(Level.ALL);
         FastSearcher fastSearcher = new FastSearcher(new MockBackend(),
                                                      new FS4ResourcePool(1),
-                                                     new MockDispatcher(Collections.emptyList()),
+                                                     new MockDispatcher("a", Collections.emptyList()),
                                                      new SummaryParameters(null),
                                                      new ClusterParams("testhittype"),
                                                      new CacheParams(100, 1e64),
@@ -137,7 +137,7 @@ public class FastSearcherTestCase {
         MockFS4ResourcePool mockFs4ResourcePool = new MockFS4ResourcePool();
         FastSearcher fastSearcher = new FastSearcher(new MockBackend(),
                 mockFs4ResourcePool,
-                new MockDispatcher(nodes, mockFs4ResourcePool, 1, new VipStatus()),
+                new MockDispatcher("a", nodes, mockFs4ResourcePool, 1, new VipStatus()),
                 new SummaryParameters(null),
                 new ClusterParams("testhittype"),
                 new CacheParams(0, 0),
@@ -177,7 +177,7 @@ public class FastSearcherTestCase {
             new DocumentdbInfoConfig(new DocumentdbInfoConfig.Builder().documentdb(new DocumentdbInfoConfig.Documentdb.Builder().name("testDb")));
         FastSearcher fastSearcher = new FastSearcher(mockBackend,
                                                      new FS4ResourcePool(1),
-                                                     new MockDispatcher(Collections.emptyList()),
+                                                     new MockDispatcher("a", Collections.emptyList()),
                                                      new SummaryParameters(null),
                                                      new ClusterParams("testhittype"),
                                                      new CacheParams(100, 1e64),
@@ -369,7 +369,7 @@ public class FastSearcherTestCase {
         Logger.getLogger(FastSearcher.class.getName()).setLevel(Level.ALL);
         return new FastSearcher(mockBackend,
                                 new FS4ResourcePool(1),
-                                new MockDispatcher(Collections.emptyList()),
+                                new MockDispatcher("a", Collections.emptyList()),
                                 new SummaryParameters(null),
                                 new ClusterParams("testhittype"),
                                 new CacheParams(100, 1e64),
@@ -382,7 +382,7 @@ public class FastSearcherTestCase {
         MockFSChannel.resetDocstamp();
         FastSearcher fastSearcher = new FastSearcher(new MockBackend(),
                                                      new FS4ResourcePool(1),
-                                                     new MockDispatcher(Collections.emptyList()),
+                                                     new MockDispatcher("a", Collections.emptyList()),
                                                      new SummaryParameters(null),
                                                      new ClusterParams("testhittype"),
                                                      new CacheParams(100, 1e64),
@@ -425,7 +425,7 @@ public class FastSearcherTestCase {
         MockFSChannel.resetDocstamp();
         FastSearcher fastSearcher = new FastSearcher(new MockBackend(),
                                                      new FS4ResourcePool(1),
-                                                     new MockDispatcher(Collections.emptyList()),
+                                                     new MockDispatcher("a", Collections.emptyList()),
                                                      new SummaryParameters(null),
                                                      new ClusterParams("testhittype"),
                                                      new CacheParams(100, 1e64),
@@ -479,7 +479,7 @@ public class FastSearcherTestCase {
     @Test
     public void testSinglePassGroupingIsNotForcedWithSingleNodeGroups() {
         MockDispatcher dispatcher =
-                new MockDispatcher(ImmutableList.of(new Node(0, "host0", 123, 0),
+                new MockDispatcher("a", ImmutableList.of(new Node(0, "host0", 123, 0),
                                                     new Node(2, "host1", 123, 0)));
 
         FastSearcher fastSearcher = new FastSearcher(new MockBackend(),
@@ -524,7 +524,7 @@ public class FastSearcherTestCase {
         Backend backend = listeners.getBackend(server.host.getHostString(),server.host.getPort());
         FastSearcher fastSearcher = new FastSearcher(backend,
                                                      new FS4ResourcePool(1),
-                                                     new MockDispatcher(Collections.emptyList()),
+                                                     new MockDispatcher("a", Collections.emptyList()),
                                                      new SummaryParameters(null),
                                                      new ClusterParams("testhittype"),
                                                      new CacheParams(0, 0.0d),
diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTester.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTester.java
index 12c313dbfe3..5377eb670c4 100644
--- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTester.java
+++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTester.java
@@ -49,7 +49,7 @@ class FastSearcherTester {
         clustersStatus.setContainerHasClusters(true);
         vipStatus = new VipStatus(clustersStatus);
         mockFS4ResourcePool = new MockFS4ResourcePool();
-        mockDispatcher = new MockDispatcher(searchNodes, mockFS4ResourcePool, containerClusterSize, vipStatus);
+        mockDispatcher = new MockDispatcher("a", searchNodes, mockFS4ResourcePool, containerClusterSize, vipStatus);
         fastSearcher = new FastSearcher(new MockBackend(selfHostname, 0L, true),
                                         mockFS4ResourcePool,
                                         mockDispatcher,
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 800b1bc21f0..1cd2c4ae791 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
@@ -14,20 +14,16 @@ import java.util.List;
 class MockDispatcher extends Dispatcher {
 
     public MockDispatcher(Node node) {
-        this(Collections.singletonList(node));
+        this(node.hostname(), Collections.singletonList(node));
     }
 
-    public MockDispatcher(List<Node> nodes) {
-        super(toDispatchConfig(nodes), new FS4ResourcePool(1), 1, new VipStatus());
+    public MockDispatcher(String clusterId, List<Node> nodes) {
+        this(clusterId, nodes, new FS4ResourcePool(1), 1, new VipStatus());
     }
 
-    public MockDispatcher(List<Node> nodes, VipStatus vipStatus) {
-        super(toDispatchConfig(nodes), new FS4ResourcePool(1), 1, vipStatus);
-    }
-
-    public MockDispatcher(List<Node> nodes, FS4ResourcePool fs4ResourcePool,
+    public MockDispatcher(String clusterId, List<Node> nodes, FS4ResourcePool fs4ResourcePool,
                           int containerClusterSize, VipStatus vipStatus) {
-        super(toDispatchConfig(nodes), fs4ResourcePool, containerClusterSize, vipStatus);
+        super(clusterId, toDispatchConfig(nodes), fs4ResourcePool, containerClusterSize, vipStatus);
     }
 
     private static DispatchConfig toDispatchConfig(List<Node> nodes) {
diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/LoadBalancerTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/LoadBalancerTest.java
index 698cee743e4..31d1399e3bb 100644
--- a/container-search/src/test/java/com/yahoo/search/dispatch/LoadBalancerTest.java
+++ b/container-search/src/test/java/com/yahoo/search/dispatch/LoadBalancerTest.java
@@ -23,7 +23,7 @@ public class LoadBalancerTest {
     @Test
     public void requreThatLoadBalancerServesSingleNodeSetups() {
         Node n1 = new Node(0, "test-node1", 0, 0);
-        SearchCluster cluster = new SearchCluster(88.0, 99.0, 0, Arrays.asList(n1), null, 1, null);
+        SearchCluster cluster = new SearchCluster("a", 88.0, 99.0, 0, Arrays.asList(n1), null, 1, null);
         LoadBalancer lb = new LoadBalancer(cluster, true);
 
         Optional<Group> grp = lb.takeGroupForQuery(new Query(), null);
@@ -37,7 +37,7 @@ public class LoadBalancerTest {
     public void requreThatLoadBalancerServesMultiGroupSetups() {
         Node n1 = new Node(0, "test-node1", 0, 0);
         Node n2 = new Node(1, "test-node2", 1, 1);
-        SearchCluster cluster = new SearchCluster(88.0, 99.0, 0, Arrays.asList(n1, n2), null, 1, null);
+        SearchCluster cluster = new SearchCluster("a", 88.0, 99.0, 0, Arrays.asList(n1, n2), null, 1, null);
         LoadBalancer lb = new LoadBalancer(cluster, true);
 
         Optional<Group> grp = lb.takeGroupForQuery(new Query(), null);
@@ -53,7 +53,7 @@ public class LoadBalancerTest {
         Node n2 = new Node(1, "test-node2", 1, 0);
         Node n3 = new Node(0, "test-node3", 0, 1);
         Node n4 = new Node(1, "test-node4", 1, 1);
-        SearchCluster cluster = new SearchCluster(88.0, 99.0, 0, Arrays.asList(n1, n2, n3, n4), null, 2, null);
+        SearchCluster cluster = new SearchCluster("a", 88.0, 99.0, 0, Arrays.asList(n1, n2, n3, n4), null, 2, null);
         LoadBalancer lb = new LoadBalancer(cluster, true);
 
         Optional<Group> grp = lb.takeGroupForQuery(new Query(), null);
@@ -64,7 +64,7 @@ public class LoadBalancerTest {
     public void requreThatLoadBalancerReturnsDifferentGroups() {
         Node n1 = new Node(0, "test-node1", 0, 0);
         Node n2 = new Node(1, "test-node2", 1, 1);
-        SearchCluster cluster = new SearchCluster(88.0, 99.0, 0, Arrays.asList(n1, n2), null, 1, null);
+        SearchCluster cluster = new SearchCluster("a", 88.0, 99.0, 0, Arrays.asList(n1, n2), null, 1, null);
         LoadBalancer lb = new LoadBalancer(cluster, true);
 
         // get first group
@@ -84,7 +84,7 @@ public class LoadBalancerTest {
     public void requreThatLoadBalancerReturnsGroupWithShortestQueue() {
         Node n1 = new Node(0, "test-node1", 0, 0);
         Node n2 = new Node(1, "test-node2", 1, 1);
-        SearchCluster cluster = new SearchCluster(88.0, 99.0, 0, Arrays.asList(n1, n2), null, 1, null);
+        SearchCluster cluster = new SearchCluster("a", 88.0, 99.0, 0, Arrays.asList(n1, n2), null, 1, null);
         LoadBalancer lb = new LoadBalancer(cluster, true);
 
         // get first group
diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java b/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java
index 0c0a65ded17..fc505097472 100644
--- a/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java
+++ b/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java
@@ -21,8 +21,8 @@ public class MockSearchCluster extends SearchCluster {
     private final ImmutableMap<Integer, Group> groups;
     private final ImmutableMultimap<String, Node> nodesByHost;
 
-    public MockSearchCluster(int groups, int nodesPerGroup) {
-        super(100, 100, 0, Collections.emptyList(), null, 1, null);
+    public MockSearchCluster(String clusterId, int groups, int nodesPerGroup) {
+        super(clusterId, 100, 100, 0, Collections.emptyList(), null, 1, null);
 
         ImmutableMap.Builder<Integer, Group> groupBuilder = ImmutableMap.builder();
         ImmutableMultimap.Builder<String, Node> hostBuilder = ImmutableMultimap.builder();
diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/SearchPathTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/SearchPathTest.java
index a1f926d3201..5a4457780e2 100644
--- a/container-search/src/test/java/com/yahoo/search/dispatch/SearchPathTest.java
+++ b/container-search/src/test/java/com/yahoo/search/dispatch/SearchPathTest.java
@@ -69,7 +69,7 @@ public class SearchPathTest {
 
     @Test
     public void searchPathMustFilterNodesBasedOnDefinition() {
-        MockSearchCluster cluster = new MockSearchCluster(3, 3);
+        MockSearchCluster cluster = new MockSearchCluster("a",3, 3);
 
         assertThat(distKeysAsString(SearchPath.selectNodes("1/1", cluster)), equalTo("5"));
         assertThat(distKeysAsString(SearchPath.selectNodes("/1", cluster)), equalTo("4,5,6"));