Improvements to handling of cluster removal

- Don't change health status to "initializing" when creating a new VipStatus,
  as 'initializing' now requires all clusters to be up to transition to 'up',
  which means that if we're already up but are missing a cluster we'll go from
  'up' to 'initializing' and stay there.
- Forget up/down status for removed clusters.
- Nicer logging on ignorable reconfiguration errors.

3 files changed, 26 insertions, 13 deletions

diff --git a/container-core/src/main/java/com/yahoo/container/handler/ClustersStatus.java b/container-core/src/main/java/com/yahoo/container/handler/ClustersStatus.java
index 0ed0daa2141..f939f9e8025 100644
--- a/container-core/src/main/java/com/yahoo/container/handler/ClustersStatus.java
+++ b/container-core/src/main/java/com/yahoo/container/handler/ClustersStatus.java
@@ -4,8 +4,11 @@ package com.yahoo.container.handler;
 import com.google.inject.Inject;
 import com.yahoo.component.AbstractComponent;
 
+import java.util.Collection;
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.Map;
+import java.util.Set;
 
 /**
  * A component which tracks the up/down status of any clusters which should influence
@@ -37,11 +40,15 @@ public class ClustersStatus extends AbstractComponent {
     /** The status of clusters, when known. Note that clusters may exist for which there is no knowledge yet. */
     private final Map<String, Boolean> clusterStatus = new HashMap<>();
 
-    public void setContainerHasClusters(boolean containerHasClusters) {
+    /** Sets the current clusters of this container */
+    public void setClusters(Set<String> clusters) {
         synchronized (mutex) {
-            this.containerHasClusters = containerHasClusters;
-            if ( ! containerHasClusters)
-                clusterStatus.clear(); // forget container clusters which was configured away
+            this.containerHasClusters = clusters.size() > 0;
+            for (Iterator<String> i = clusterStatus.keySet().iterator(); i.hasNext(); ) {
+                String existingCluster = i.next();
+                if ( ! clusters.contains(existingCluster))
+                    i.remove(); // forget clusters which was configured away
+            }
         }
     }
 
@@ -78,9 +85,11 @@ public class ClustersStatus extends AbstractComponent {
     public boolean containerShouldReceiveTraffic() {
         return containerShouldReceiveTraffic(Require.ONE);
     }
+
     /**
      *  Returns whether this container should receive traffic based on the state of this
-     *  @param require Requirement for being up, ALL or ONE.
+     *
+     *  @param require requirement for being up, ALL or ONE.
      */
     public boolean containerShouldReceiveTraffic(Require require) {
         synchronized (mutex) {
diff --git a/container-core/src/main/java/com/yahoo/container/handler/VipStatus.java b/container-core/src/main/java/com/yahoo/container/handler/VipStatus.java
index 0bf86e8f440..e1b5b769906 100644
--- a/container-core/src/main/java/com/yahoo/container/handler/VipStatus.java
+++ b/container-core/src/main/java/com/yahoo/container/handler/VipStatus.java
@@ -6,6 +6,8 @@ import com.yahoo.container.QrSearchersConfig;
 import com.yahoo.container.core.VipStatusConfig;
 import com.yahoo.container.jdisc.state.StateMonitor;
 
+import java.util.stream.Collectors;
+
 /**
  * A component which keeps track of whether or not this container instance should receive traffic
  * and respond that it is in good health.
@@ -59,8 +61,7 @@ public class VipStatus {
         this.clustersStatus = clustersStatus;
         this.healthState = healthState;
         initiallyInRotation = vipStatusConfig.initiallyInRotation();
-        healthState.status(StateMonitor.Status.initializing);
-        clustersStatus.setContainerHasClusters(! dispatchers.searchcluster().isEmpty());
+        clustersStatus.setClusters(dispatchers.searchcluster().stream().map(c -> c.name()).collect(Collectors.toSet()));
         updateCurrentlyInRotation();
     }
 
diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/state/StateMonitor.java b/container-core/src/main/java/com/yahoo/container/jdisc/state/StateMonitor.java
index 78b65622150..0018dd22dd9 100644
--- a/container-core/src/main/java/com/yahoo/container/jdisc/state/StateMonitor.java
+++ b/container-core/src/main/java/com/yahoo/container/jdisc/state/StateMonitor.java
@@ -47,11 +47,13 @@ public class StateMonitor extends AbstractComponent {
 
     @Inject
     public StateMonitor(HealthMonitorConfig config, Timer timer) {
-        this(config, timer, runnable -> {
-            Thread thread = new Thread(runnable, "StateMonitor");
-            thread.setDaemon(true);
-            return thread;
-        });
+        this(config,
+             timer,
+             runnable -> {
+                 Thread thread = new Thread(runnable, "StateMonitor");
+                 thread.setDaemon(true);
+                 return thread;
+             });
     }
 
     StateMonitor(HealthMonitorConfig config, Timer timer, ThreadFactory threadFactory) {
@@ -59,7 +61,8 @@ public class StateMonitor extends AbstractComponent {
              Status.valueOf(config.initialStatus()),
              timer, threadFactory);
     }
-    /* For Testing */
+
+    /* Public for testing only */
     public StateMonitor(long snapshotIntervalMS, Status status, Timer timer, ThreadFactory threadFactory) {
         this.timer = timer;
         this.snapshotIntervalMs = snapshotIntervalMS;