Browse Source

Repeat cluster.initial_master_nodes log warning (#92744)

Today we log a warning about removing `cluster.initial_master_nodes` at
startup, but this is easy to miss in amongst all the other messages.
With this commit we repeat the warning message every 12 hours.

Relates #92741
David Turner 2 years ago
parent
commit
6a0440340e

+ 5 - 0
docs/changelog/92744.yaml

@@ -0,0 +1,5 @@
+pr: 92744
+summary: Repeat `cluster.initial_master_nodes` log warning
+area: Cluster Coordination
+type: enhancement
+issues: []

+ 14 - 8
server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java

@@ -129,14 +129,9 @@ public class ClusterBootstrapService implements Coordinator.PeerFinderListener {
             if (bootstrapRequirements.isEmpty()) {
                 logger.info("this node is locked into cluster UUID [{}] and will not attempt further cluster bootstrapping", clusterUUID);
             } else {
-                logger.warn(
-                    """
-                        this node is locked into cluster UUID [{}] but [{}] is set to {}; \
-                        remove this setting to avoid possible data loss caused by subsequent cluster bootstrap attempts""",
-                    clusterUUID,
-                    INITIAL_MASTER_NODES_SETTING.getKey(),
-                    bootstrapRequirements
-                );
+                transportService.getThreadPool()
+                    .scheduleWithFixedDelay(() -> logRemovalWarning(clusterUUID), TimeValue.timeValueHours(12), Names.SAME);
+                logRemovalWarning(clusterUUID);
             }
         } else {
             logger.info(
@@ -147,6 +142,17 @@ public class ClusterBootstrapService implements Coordinator.PeerFinderListener {
         }
     }
 
+    private void logRemovalWarning(String clusterUUID) {
+        logger.warn(
+            """
+                this node is locked into cluster UUID [{}] but [{}] is set to {}; \
+                remove this setting to avoid possible data loss caused by subsequent cluster bootstrap attempts""",
+            clusterUUID,
+            INITIAL_MASTER_NODES_SETTING.getKey(),
+            bootstrapRequirements
+        );
+    }
+
     @Override
     public void onFoundPeersUpdated() {
         final Set<DiscoveryNode> nodes = getDiscoveredNodes();

+ 24 - 14
server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java

@@ -8,16 +8,15 @@
 package org.elasticsearch.cluster.coordination;
 
 import org.apache.logging.log4j.Level;
-import org.apache.logging.log4j.LogManager;
 import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.Version;
 import org.elasticsearch.cluster.metadata.Metadata;
 import org.elasticsearch.cluster.node.DiscoveryNode;
 import org.elasticsearch.cluster.node.DiscoveryNodeRole;
 import org.elasticsearch.common.UUIDs;
-import org.elasticsearch.common.logging.Loggers;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.concurrent.DeterministicTaskQueue;
+import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.discovery.DiscoveryModule;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.MockLogAppender;
@@ -661,13 +660,9 @@ public class ClusterBootstrapServiceTests extends ESTestCase {
         );
     }
 
-    public void testBootstrapStateLogging() throws IllegalAccessException {
+    public void testBootstrapStateLogging() {
         final var mockAppender = new MockLogAppender();
-        mockAppender.start();
-        final var serviceLogger = LogManager.getLogger(ClusterBootstrapService.class);
-        Loggers.addAppender(serviceLogger, mockAppender);
-
-        try {
+        try (var ignored = mockAppender.capturing(ClusterBootstrapService.class)) {
             mockAppender.addExpectation(
                 new MockLogAppender.SeenEventExpectation(
                     "fresh node message",
@@ -710,14 +705,16 @@ public class ClusterBootstrapServiceTests extends ESTestCase {
 
             mockAppender.assertAllExpectationsMatched();
 
+            final var warningMessagePattern = """
+                this node is locked into cluster UUID [test-uuid] but [cluster.initial_master_nodes] is set to [node1, node2]; \
+                remove this setting to avoid possible data loss caused by subsequent cluster bootstrap attempts""";
+
             mockAppender.addExpectation(
                 new MockLogAppender.SeenEventExpectation(
                     "bootstrapped node message if bootstrapping still configured",
                     ClusterBootstrapService.class.getCanonicalName(),
                     Level.WARN,
-                    """
-                        this node is locked into cluster UUID [test-uuid] but [cluster.initial_master_nodes] is set to [node1, node2]; \
-                        remove this setting to avoid possible data loss caused by subsequent cluster bootstrap attempts"""
+                    warningMessagePattern
                 )
             );
 
@@ -731,9 +728,22 @@ public class ClusterBootstrapServiceTests extends ESTestCase {
 
             mockAppender.assertAllExpectationsMatched();
 
-        } finally {
-            Loggers.removeAppender(serviceLogger, mockAppender);
-            mockAppender.stop();
+            mockAppender.addExpectation(
+                new MockLogAppender.SeenEventExpectation(
+                    "bootstrapped node message if bootstrapping still configured",
+                    ClusterBootstrapService.class.getCanonicalName(),
+                    Level.WARN,
+                    warningMessagePattern
+                )
+            );
+
+            var startTime = deterministicTaskQueue.getCurrentTimeMillis();
+            while (deterministicTaskQueue.getCurrentTimeMillis() <= startTime + TimeValue.timeValueHours(12).millis()) {
+                deterministicTaskQueue.runAllRunnableTasks();
+                deterministicTaskQueue.advanceTime();
+            }
+
+            mockAppender.assertAllExpectationsMatched();
         }
     }
 }