Warn on not enough masters during election (#20063)

This changes the trace level logging to warn, and adds the needed number to the message as well.

My fear is that it may get noisy, but this is an issue that you want to be noisy.

Author: pickypg

core/src/main/java/org/elasticsearch/discovery/zen/ElectMasterService.java

@@ -111,14 +111,14 @@ public int minimumMasterNodes() {
         return minimumMasterNodes;
     }
 
-    public boolean hasEnoughMasterNodes(Iterable<DiscoveryNode> nodes) {
+    public int countMasterNodes(Iterable<DiscoveryNode> nodes) {
         int count = 0;
         for (DiscoveryNode node : nodes) {
             if (node.isMasterNode()) {
                 count++;
             }
         }
-        return count > 0 && (minimumMasterNodes < 0 || count >= minimumMasterNodes);
+        return count;
     }
 
     public boolean hasEnoughCandidates(Collection<MasterCandidate> candidates) {
@@ -149,13 +149,12 @@ public DiscoveryNode tieBreakActiveMasters(Collection<DiscoveryNode> activeMaste
         return activeMasters.stream().min(ElectMasterService::compareNodes).get();
     }
 
+    public boolean hasEnoughMasterNodes(Iterable<DiscoveryNode> nodes) {
+        return minimumMasterNodes < 1 || countMasterNodes(nodes) >= minimumMasterNodes;
+    }
+
     public boolean hasTooManyMasterNodes(Iterable<DiscoveryNode> nodes) {
-        int count = 0;
-        for (DiscoveryNode node : nodes) {
-            if (node.isMasterNode()) {
-                count++;
-            }
-        }
+        final int count = countMasterNodes(nodes);
         return count > 1 && minimumMasterNodes <= count / 2;
     }
 

core/src/main/java/org/elasticsearch/discovery/zen/ZenDiscovery.java

@@ -47,6 +47,7 @@
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.lease.Releasables;
+import org.elasticsearch.common.logging.LoggerMessageFormat;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Setting.Property;
 import org.elasticsearch.common.settings.Settings;
@@ -580,8 +581,10 @@ public ClusterTasksResult<Task> execute(final ClusterState currentState, final L
             final ClusterState remainingNodesClusterState = remainingNodesClusterState(currentState, remainingNodesBuilder);
 
             final ClusterTasksResult.Builder<Task> resultBuilder = ClusterTasksResult.<Task>builder().successes(tasks);
-            if (!electMasterService.hasEnoughMasterNodes(remainingNodesClusterState.nodes())) {
-                rejoin.accept("not enough master nodes");
+            if (electMasterService.hasEnoughMasterNodes(remainingNodesClusterState.nodes()) == false) {
+                final int masterNodes = electMasterService.countMasterNodes(remainingNodesClusterState.nodes());
+                rejoin.accept(LoggerMessageFormat.format("not enough master nodes (has [{}], but needed [{}])",
+                                                         masterNodes, electMasterService.minimumMasterNodes()));
                 return resultBuilder.build(currentState);
             } else {
                 return resultBuilder.build(allocationService.deassociateDeadNodes(remainingNodesClusterState, true, describeTasks(tasks)));
@@ -920,7 +923,8 @@ private DiscoveryNode findMaster() {
                 return winner.getNode();
             } else {
                 // if we don't have enough master nodes, we bail, because there are not enough master to elect from
-                logger.trace("not enough master nodes [{}]", masterCandidates);
+                logger.warn("not enough master nodes discovered during pinging (found [{}], but needed [{}]), pinging again",
+                            masterCandidates, electMaster.minimumMasterNodes());
                 return null;
             }
         } else {

core/src/test/java/org/elasticsearch/discovery/zen/ElectMasterServiceTests.java

@@ -139,4 +139,19 @@ public void testElectMaster() {
             }
         }
     }
+
+    public void testCountMasterNodes() {
+        List<DiscoveryNode> nodes = generateRandomNodes();
+        ElectMasterService service = electMasterService();
+
+        int masterNodes = 0;
+
+        for (DiscoveryNode node : nodes) {
+            if (node.isMasterNode()) {
+                masterNodes++;
+            }
+        }
+
+        assertEquals(masterNodes, service.countMasterNodes(nodes));
+    }
 }

core/src/test/java/org/elasticsearch/discovery/zen/NodeRemovalClusterStateTaskExecutorTests.java

@@ -108,6 +108,8 @@ ClusterState remainingNodesClusterState(ClusterState currentState, DiscoveryNode
         final ClusterStateTaskExecutor.ClusterTasksResult<ZenDiscovery.NodeRemovalClusterStateTaskExecutor.Task> result =
                 executor.execute(clusterState, tasks);
         verify(electMasterService).hasEnoughMasterNodes(eq(remainingNodesClusterState.get().nodes()));
+        verify(electMasterService).countMasterNodes(eq(remainingNodesClusterState.get().nodes()));
+        verify(electMasterService).minimumMasterNodes();
         verifyNoMoreInteractions(electMasterService);
 
         // ensure that we did not reroute