Add node state reason for nodes that may have buckets pending.

3 files changed, 65 insertions, 13 deletions

diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/MaintenanceWhenPendingGlobalMerges.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/MaintenanceWhenPendingGlobalMerges.java
index 14dc36b1d7d..3af789c41d4 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/MaintenanceWhenPendingGlobalMerges.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/MaintenanceWhenPendingGlobalMerges.java
@@ -4,7 +4,9 @@ package com.yahoo.vespa.clustercontroller.core;
 import com.yahoo.document.FixedBucketSpaces;
 import com.yahoo.vdslib.state.*;
 
+import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Map;
 import java.util.Set;
 
 /**
@@ -29,16 +31,14 @@ public class MaintenanceWhenPendingGlobalMerges implements ClusterStateDeriver {
 
     @Override
     public AnnotatedClusterState derivedFrom(AnnotatedClusterState baselineState, String bucketSpace) {
-        AnnotatedClusterState derivedState = baselineState.clone();
         if (!bucketSpace.equals(bucketSpaceToDerive)) {
-            return derivedState;
+            return baselineState.clone();
         }
         Set<Integer> incompleteNodeIndices = nodesWithMergesNotDone(baselineState.getClusterState());
         if (incompleteNodeIndices.isEmpty()) {
-            return derivedState; // Nothing to do
+            return baselineState.clone();
         }
-        incompleteNodeIndices.forEach(nodeIndex -> setNodeInMaintenance(derivedState, nodeIndex));
-        return derivedState;
+        return setNodesInMaintenance(baselineState, incompleteNodeIndices);
     }
 
     private Set<Integer> nodesWithMergesNotDone(ClusterState baselineState) {
@@ -48,23 +48,32 @@ public class MaintenanceWhenPendingGlobalMerges implements ClusterStateDeriver {
             // FIXME should only set nodes into maintenance if they've not yet been up in the cluster
             // state since they came back as Reported state Up!
             // Must be implemented before this state deriver is enabled in production.
-            if (contentNodeIsAvailable(baselineState, nodeIndex) && hasMergesNotDone(bucketSpaceToCheck, nodeIndex)) {
+            if (contentNodeIsAvailable(baselineState, nodeIndex) && mayHaveMergesPending(bucketSpaceToCheck, nodeIndex)) {
                 incompleteNodes.add(nodeIndex);
             }
         }
         return incompleteNodes;
     }
 
-    private void setNodeInMaintenance(AnnotatedClusterState derivedState, int nodeIndex) {
-        derivedState.getClusterState().setNodeState(Node.ofStorage(nodeIndex),
-                new NodeState(NodeType.STORAGE, State.MAINTENANCE));
+    private AnnotatedClusterState setNodesInMaintenance(AnnotatedClusterState baselineState,
+                                                        Set<Integer> incompleteNodeIndices) {
+        ClusterState derivedState = baselineState.getClusterState().clone();
+        Map<Node, NodeStateReason> nodeStateReasons = new HashMap<>(baselineState.getNodeStateReasons());
+        incompleteNodeIndices.forEach(nodeIndex -> {
+            Node node = Node.ofStorage(nodeIndex);
+            derivedState.setNodeState(node, new NodeState(NodeType.STORAGE, State.MAINTENANCE));
+            nodeStateReasons.put(node, NodeStateReason.MAY_HAVE_MERGES_PENDING);
+        });
+        return new AnnotatedClusterState(derivedState,
+                baselineState.getClusterStateReason(),
+                nodeStateReasons);
     }
 
     private boolean contentNodeIsAvailable(ClusterState state, int nodeIndex) {
         return state.getNodeState(Node.ofStorage(nodeIndex)).getState().oneOf("uir");
     }
 
-    private boolean hasMergesNotDone(String bucketSpace, int nodeIndex) {
+    private boolean mayHaveMergesPending(String bucketSpace, int nodeIndex) {
         return mergePendingChecker.mayHaveMergesPending(bucketSpace, nodeIndex);
     }
 }
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateReason.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateReason.java
index a092e2012ec..7a6be664ec8 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateReason.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateReason.java
@@ -7,6 +7,7 @@ public enum NodeStateReason {
     NODE_TOO_UNSTABLE,
     WITHIN_MAINTENANCE_GRACE_PERIOD,
     FORCED_INTO_MAINTENANCE,
-    GROUP_IS_DOWN
+    GROUP_IS_DOWN,
+    MAY_HAVE_MERGES_PENDING
 
 }
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MaintenanceWhenPendingGlobalMergesTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MaintenanceWhenPendingGlobalMergesTest.java
index 9e17127d22b..46b23346c34 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MaintenanceWhenPendingGlobalMergesTest.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/MaintenanceWhenPendingGlobalMergesTest.java
@@ -3,8 +3,13 @@ package com.yahoo.vespa.clustercontroller.core;
 
 import com.yahoo.document.FixedBucketSpaces;
 import com.yahoo.vdslib.state.ClusterState;
+import com.yahoo.vdslib.state.Node;
 import org.junit.Test;
 
+import java.util.*;
+
+import static com.yahoo.vespa.clustercontroller.core.NodeStateReason.MAY_HAVE_MERGES_PENDING;
+import static com.yahoo.vespa.clustercontroller.core.NodeStateReason.NODE_TOO_UNSTABLE;
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.junit.Assert.assertThat;
 import static org.mockito.Matchers.anyInt;
@@ -31,6 +36,29 @@ public class MaintenanceWhenPendingGlobalMergesTest {
         return AnnotatedClusterState.withoutAnnotations(ClusterState.stateFromString(stateStr));
     }
 
+    private static class AnnotatedClusterStateBuilder {
+        private ClusterState clusterState;
+        private Map<Node, NodeStateReason> nodeStateReasons = new HashMap<>();
+
+        private AnnotatedClusterStateBuilder(String stateStr) {
+            clusterState = ClusterState.stateFromString(stateStr);
+        }
+
+        public static AnnotatedClusterStateBuilder ofState(String stateStr) {
+            return new AnnotatedClusterStateBuilder(stateStr);
+        }
+
+        public AnnotatedClusterStateBuilder reason(NodeStateReason reason, Integer... nodeIndices) {
+            Arrays.stream(nodeIndices).forEach(nodeIndex -> nodeStateReasons.put(Node.ofStorage(nodeIndex), reason));
+            return this;
+        }
+
+        public AnnotatedClusterState build() {
+            return new AnnotatedClusterState(clusterState, Optional.empty(), nodeStateReasons);
+        }
+
+    }
+
     @Test
     public void no_nodes_set_to_maintenance_in_global_bucket_space_state() {
         Fixture f = new Fixture();
@@ -45,7 +73,8 @@ public class MaintenanceWhenPendingGlobalMergesTest {
         when(f.mockPendingChecker.mayHaveMergesPending(globalSpace(), 1)).thenReturn(true);
         when(f.mockPendingChecker.mayHaveMergesPending(globalSpace(), 3)).thenReturn(true);
         AnnotatedClusterState derived = f.deriver.derivedFrom(stateFromString("distributor:5 storage:5"), defaultSpace());
-        assertThat(derived, equalTo(stateFromString("distributor:5 storage:5 .1.s:m .3.s:m")));
+        assertThat(derived, equalTo(AnnotatedClusterStateBuilder.ofState("distributor:5 storage:5 .1.s:m .3.s:m")
+                .reason(MAY_HAVE_MERGES_PENDING, 1, 3).build()));
     }
 
     @Test
@@ -69,7 +98,20 @@ public class MaintenanceWhenPendingGlobalMergesTest {
         when(f.mockPendingChecker.mayHaveMergesPending(eq(globalSpace()), anyInt())).thenReturn(true);
         AnnotatedClusterState derived = f.deriver.derivedFrom(stateFromString("distributor:5 storage:5 .1.s:m .2.s:r .3.s:i .4.s:d"), defaultSpace());
         // TODO reconsider role of retired here... It should not have merges pending towards it in the general case, but may be out of sync
-        assertThat(derived, equalTo(stateFromString("distributor:5 storage:5 .0.s:m .1.s:m .2.s:m .3.s:m .4.s:d")));
+        assertThat(derived, equalTo(AnnotatedClusterStateBuilder.ofState("distributor:5 storage:5 .0.s:m .1.s:m .2.s:m .3.s:m .4.s:d")
+                .reason(MAY_HAVE_MERGES_PENDING, 0, 2, 3).build()));
+    }
+
+    @Test
+    public void node_state_reasons_are_used_as_baseline_in_default_bucket_space_state() {
+        Fixture f = new Fixture();
+        when(f.mockPendingChecker.mayHaveMergesPending(globalSpace(), 1)).thenReturn(true);
+        when(f.mockPendingChecker.mayHaveMergesPending(globalSpace(), 3)).thenReturn(true);
+        AnnotatedClusterState derived = f.deriver.derivedFrom(AnnotatedClusterStateBuilder.ofState("distributor:5 storage:5")
+                .reason(NODE_TOO_UNSTABLE, 1, 2).build(), defaultSpace());
+        assertThat(derived, equalTo(AnnotatedClusterStateBuilder.ofState("distributor:5 storage:5 .1.s:m .3.s:m")
+                .reason(MAY_HAVE_MERGES_PENDING, 1, 3)
+                .reason(NODE_TOO_UNSTABLE, 2).build()));
     }
 
 }