Handle master failure in NodeSeenService (#77220)

* Handle master failure in NodeSeenService NodeSeenService can miss seeing nodes if the master changes while it's processing the cluster state update which adds the nodes to the cluster. This caused occasional test failures in the test intended to check that NodeSeenService is working as intended. This commit adjusts NodeSeenService's early returns to ensure that, if the master changed, the new master checks for seen nodes even if nodes were not added in that particular cluster state update. * Clarify & correct "just-became-master" check * Remove leftover debug logging (d'oh!) Co-authored-by: Elastic Machine <[email protected]>
elastic · Sep 9, 2021 · 62fed2f · 62fed2f
1 parent 4b4fc05
commit 62fed2f
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 5 deletions.
diff --git a/...n/src/internalClusterTest/java/org/elasticsearch/xpack/shutdown/NodeShutdownShardsIT.java b/...n/src/internalClusterTest/java/org/elasticsearch/xpack/shutdown/NodeShutdownShardsIT.java
@@ -70,7 +70,6 @@ public void testShardStatusStaysCompleteAfterNodeLeaves() throws Exception {
      * Similar to the previous test, but ensures that the status stays at `COMPLETE` when the node is offline when the shutdown is
      * registered. This may happen if {@link NodeSeenService} isn't working as expected.
      */
-    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/76689")
     public void testShardStatusStaysCompleteAfterNodeLeavesIfRegisteredWhileNodeOffline() throws Exception {
         assumeTrue("must be on a snapshot build of ES to run in order for the feature flag to be set", Build.CURRENT.isSnapshot());
         final String nodeToRestartName = internalCluster().startNode();

diff --git a/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/NodeSeenService.java b/x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/NodeSeenService.java
@@ -7,8 +7,8 @@
 
 package org.elasticsearch.xpack.shutdown;
 
-import org.apache.log4j.LogManager;
-import org.apache.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.message.ParameterizedMessage;
 import org.elasticsearch.cluster.ClusterChangedEvent;
 import org.elasticsearch.cluster.ClusterState;
@@ -46,8 +46,10 @@ public void clusterChanged(ClusterChangedEvent event) {
             return;
         }
 
-        if (event.nodesAdded() == false) {
-            // If there's no new nodes this cluster state update, nothing to do.
+        final boolean thisNodeJustBecameMaster = event.previousState().nodes().isLocalNodeElectedMaster() == false
+            && event.state().nodes().isLocalNodeElectedMaster();
+        if ((event.nodesAdded() || thisNodeJustBecameMaster) == false) {
+            // If there's both 1) no new nodes this cluster state update and 2) this node has not just become the master node, nothing to do
             return;
         }