From 00a4cc0a3e9bd19b9e30f391e45524e8317758e7 Mon Sep 17 00:00:00 2001 From: Chris Earle Date: Thu, 18 Aug 2016 12:36:58 -0400 Subject: [PATCH] Warn on not enough masters during election 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. --- .../discovery/zen/ElectMasterService.java | 15 +++++++-------- .../elasticsearch/discovery/zen/ZenDiscovery.java | 10 +++++++--- .../discovery/zen/ElectMasterServiceTests.java | 15 +++++++++++++++ .../NodeRemovalClusterStateTaskExecutorTests.java | 2 ++ 4 files changed, 31 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/discovery/zen/ElectMasterService.java b/core/src/main/java/org/elasticsearch/discovery/zen/ElectMasterService.java index f0de15da9ae52..92b20c5199b89 100644 --- a/core/src/main/java/org/elasticsearch/discovery/zen/ElectMasterService.java +++ b/core/src/main/java/org/elasticsearch/discovery/zen/ElectMasterService.java @@ -111,14 +111,14 @@ public int minimumMasterNodes() { return minimumMasterNodes; } - public boolean hasEnoughMasterNodes(Iterable nodes) { + public int countMasterNodes(Iterable 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 candidates) { @@ -149,13 +149,12 @@ public DiscoveryNode tieBreakActiveMasters(Collection activeMaste return activeMasters.stream().min(ElectMasterService::compareNodes).get(); } + public boolean hasEnoughMasterNodes(Iterable nodes) { + return minimumMasterNodes < 1 || countMasterNodes(nodes) >= minimumMasterNodes; + } + public boolean hasTooManyMasterNodes(Iterable nodes) { - int count = 0; - for (DiscoveryNode node : nodes) { - if (node.isMasterNode()) { - count++; - } - } + final int count = countMasterNodes(nodes); return count > 1 && minimumMasterNodes <= count / 2; } diff --git a/core/src/main/java/org/elasticsearch/discovery/zen/ZenDiscovery.java b/core/src/main/java/org/elasticsearch/discovery/zen/ZenDiscovery.java index 72c33c873d4ae..f5dbaac62423f 100644 --- a/core/src/main/java/org/elasticsearch/discovery/zen/ZenDiscovery.java +++ b/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 execute(final ClusterState currentState, final L final ClusterState remainingNodesClusterState = remainingNodesClusterState(currentState, remainingNodesBuilder); final ClusterTasksResult.Builder resultBuilder = ClusterTasksResult.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 { diff --git a/core/src/test/java/org/elasticsearch/discovery/zen/ElectMasterServiceTests.java b/core/src/test/java/org/elasticsearch/discovery/zen/ElectMasterServiceTests.java index f59cc1cc6fb0f..2f3bd44f47586 100644 --- a/core/src/test/java/org/elasticsearch/discovery/zen/ElectMasterServiceTests.java +++ b/core/src/test/java/org/elasticsearch/discovery/zen/ElectMasterServiceTests.java @@ -139,4 +139,19 @@ public void testElectMaster() { } } } + + public void testCountMasterNodes() { + List nodes = generateRandomNodes(); + ElectMasterService service = electMasterService(); + + int masterNodes = 0; + + for (DiscoveryNode node : nodes) { + if (node.isMasterNode()) { + masterNodes++; + } + } + + assertEquals(masterNodes, service.countMasterNodes(nodes)); + } } diff --git a/core/src/test/java/org/elasticsearch/discovery/zen/NodeRemovalClusterStateTaskExecutorTests.java b/core/src/test/java/org/elasticsearch/discovery/zen/NodeRemovalClusterStateTaskExecutorTests.java index 70b291fee6028..bf25a7babd179 100644 --- a/core/src/test/java/org/elasticsearch/discovery/zen/NodeRemovalClusterStateTaskExecutorTests.java +++ b/core/src/test/java/org/elasticsearch/discovery/zen/NodeRemovalClusterStateTaskExecutorTests.java @@ -108,6 +108,8 @@ ClusterState remainingNodesClusterState(ClusterState currentState, DiscoveryNode final ClusterStateTaskExecutor.ClusterTasksResult 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