Remove settings which can't be dynamic

Signed-off-by: Rahul Karajgikar <karajgik@amazon.com>

server/src/main/java/org/opensearch/cluster/coordination/Coordinator.java

@@ -133,8 +133,7 @@ public class Coordinator extends AbstractLifecycleComponent implements Discovery
         "cluster.publish.info_timeout",
         TimeValue.timeValueMillis(10000),
         TimeValue.timeValueMillis(1),
-        Setting.Property.NodeScope,
-        Setting.Property.Dynamic
+        Setting.Property.NodeScope
     );
 
     // the timeout for the publication of each value
@@ -167,7 +166,7 @@ public class Coordinator extends AbstractLifecycleComponent implements Discovery
     private final ElectionSchedulerFactory electionSchedulerFactory;
     private final SeedHostsResolver configuredHostsResolver;
     private TimeValue publishTimeout;
-    private TimeValue publishInfoTimeout;
+    private final TimeValue publishInfoTimeout;
     private final PublicationTransportHandler publicationHandler;
     private final LeaderChecker leaderChecker;
     private final FollowersChecker followersChecker;
@@ -250,9 +249,8 @@ public Coordinator(
         this.publishTimeout = PUBLISH_TIMEOUT_SETTING.get(settings);
         clusterSettings.addSettingsUpdateConsumer(PUBLISH_TIMEOUT_SETTING, this::setPublishTimeout);
         this.publishInfoTimeout = PUBLISH_INFO_TIMEOUT_SETTING.get(settings);
-        clusterSettings.addSettingsUpdateConsumer(PUBLISH_INFO_TIMEOUT_SETTING, this::setPublishInfoTimeout);
         this.random = random;
-        this.electionSchedulerFactory = new ElectionSchedulerFactory(settings, clusterSettings, random, transportService.getThreadPool());
+        this.electionSchedulerFactory = new ElectionSchedulerFactory(settings, random, transportService.getThreadPool());
         this.preVoteCollector = new PreVoteCollector(
             transportService,
             this::startElection,
@@ -263,7 +261,6 @@ public Coordinator(
         configuredHostsResolver = new SeedHostsResolver(nodeName, settings, transportService, seedHostsProvider);
         this.peerFinder = new CoordinatorPeerFinder(
             settings,
-            clusterSettings,
             transportService,
             new HandshakingTransportAddressConnector(settings, transportService),
             configuredHostsResolver
@@ -326,10 +323,6 @@ protected void setPublishTimeout(TimeValue publishTimeout) {
         this.publishTimeout = publishTimeout;
     }
 
-    protected void setPublishInfoTimeout(TimeValue publishInfoTimeout) {
-        this.publishInfoTimeout = publishInfoTimeout;
-    }
-
     private ClusterFormationState getClusterFormationState() {
         return new ClusterFormationState(
             settings,
@@ -1484,14 +1477,12 @@ private class CoordinatorPeerFinder extends PeerFinder {
 
         CoordinatorPeerFinder(
             Settings settings,
-            ClusterSettings clusterSettings,
             TransportService transportService,
             TransportAddressConnector transportAddressConnector,
             ConfiguredHostsResolver configuredHostsResolver
         ) {
             super(
                 settings,
-                clusterSettings,
                 transportService,
                 transportAddressConnector,
                 singleNodeDiscovery ? hostsResolver -> Collections.emptyList() : configuredHostsResolver

server/src/main/java/org/opensearch/cluster/coordination/ElectionSchedulerFactory.java

@@ -37,7 +37,6 @@
 import org.apache.logging.log4j.message.ParameterizedMessage;
 import org.opensearch.common.SuppressForbidden;
 import org.opensearch.common.lease.Releasable;
-import org.opensearch.common.settings.ClusterSettings;
 import org.opensearch.common.settings.Setting;
 import org.opensearch.common.settings.Setting.Property;
 import org.opensearch.common.settings.Settings;
@@ -88,8 +87,7 @@ public class ElectionSchedulerFactory {
         TimeValue.timeValueMillis(100),
         TimeValue.timeValueMillis(1),
         TimeValue.timeValueSeconds(10),
-        Property.NodeScope,
-        Property.Dynamic
+        Property.NodeScope
     );
 
     public static final Setting<TimeValue> ELECTION_BACK_OFF_TIME_SETTING = Setting.timeSetting(
@@ -113,8 +111,7 @@ public class ElectionSchedulerFactory {
         TimeValue.timeValueMillis(500),
         TimeValue.timeValueMillis(1),
         TimeValue.timeValueSeconds(300),
-        Property.NodeScope,
-        Property.Dynamic
+        Property.NodeScope
     );
 
     private TimeValue initialTimeout;
@@ -124,16 +121,14 @@ public class ElectionSchedulerFactory {
     private final ThreadPool threadPool;
     private final Random random;
 
-    public ElectionSchedulerFactory(Settings settings, ClusterSettings clusterSettings, Random random, ThreadPool threadPool) {
+    public ElectionSchedulerFactory(Settings settings, Random random, ThreadPool threadPool) {
         this.random = random;
         this.threadPool = threadPool;
 
         initialTimeout = ELECTION_INITIAL_TIMEOUT_SETTING.get(settings);
-        clusterSettings.addSettingsUpdateConsumer(ELECTION_INITIAL_TIMEOUT_SETTING, this::setElectionInitialTimeout);
         backoffTime = ELECTION_BACK_OFF_TIME_SETTING.get(settings);
         maxTimeout = ELECTION_MAX_TIMEOUT_SETTING.get(settings);
         duration = ELECTION_DURATION_SETTING.get(settings);
-        clusterSettings.addSettingsUpdateConsumer(ELECTION_DURATION_SETTING, this::setElectionDuration);
 
         if (maxTimeout.millis() < initialTimeout.millis()) {
             throw new IllegalArgumentException(

server/src/main/java/org/opensearch/cluster/coordination/FollowersChecker.java

@@ -112,15 +112,14 @@ public class FollowersChecker {
         3,
         1,
         10,
-        Setting.Property.NodeScope,
-        Setting.Property.Dynamic
+        Setting.Property.NodeScope
     );
 
     private final Settings settings;
 
     private TimeValue followerCheckInterval;
     private TimeValue followerCheckTimeout;
-    private int followerCheckRetryCount;
+    private final int followerCheckRetryCount;
     private final BiConsumer<DiscoveryNode, String> onNodeFailure;
     private final Consumer<FollowerCheckRequest> handleRequestAndUpdateState;
 
@@ -153,7 +152,6 @@ public FollowersChecker(
         followerCheckRetryCount = FOLLOWER_CHECK_RETRY_COUNT_SETTING.get(settings);
         clusterSettings.addSettingsUpdateConsumer(FOLLOWER_CHECK_INTERVAL_SETTING, this::setFollowerCheckInterval);
         clusterSettings.addSettingsUpdateConsumer(FOLLOWER_CHECK_TIMEOUT_SETTING, this::setFollowerCheckTimeout);
-        clusterSettings.addSettingsUpdateConsumer(FOLLOWER_CHECK_RETRY_COUNT_SETTING, this::setFollowerCheckRetryCount);
         updateFastResponseState(0, Mode.CANDIDATE);
         transportService.registerRequestHandler(
             FOLLOWER_CHECK_ACTION_NAME,
@@ -180,10 +178,6 @@ private void setFollowerCheckTimeout(TimeValue followerCheckTimeout) {
         this.followerCheckTimeout = followerCheckTimeout;
     }
 
-    private void setFollowerCheckRetryCount(int followerCheckRetryCount) {
-        this.followerCheckRetryCount = followerCheckRetryCount;
-    }
-
     /**
      * Update the set of known nodes, starting to check any new ones and stopping checking any previously-known-but-now-unknown ones.
      */

server/src/main/java/org/opensearch/discovery/PeerFinder.java

@@ -40,7 +40,6 @@
 import org.opensearch.cluster.node.DiscoveryNodes;
 import org.opensearch.common.Nullable;
 import org.opensearch.common.SetOnce;
-import org.opensearch.common.settings.ClusterSettings;
 import org.opensearch.common.settings.Setting;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.unit.TimeValue;
@@ -96,13 +95,12 @@ public abstract class PeerFinder {
         "discovery.request_peers_timeout",
         TimeValue.timeValueMillis(3000),
         TimeValue.timeValueMillis(1),
-        Setting.Property.NodeScope,
-        Setting.Property.Dynamic
+        Setting.Property.NodeScope
     );
 
     private final Settings settings;
     private TimeValue findPeersInterval;
-    private TimeValue requestPeersTimeout;
+    private final TimeValue requestPeersTimeout;
 
     private final Object mutex = new Object();
     private final TransportService transportService;
@@ -118,15 +116,13 @@ public abstract class PeerFinder {
 
     public PeerFinder(
         Settings settings,
-        ClusterSettings clusterSettings,
         TransportService transportService,
         TransportAddressConnector transportAddressConnector,
         ConfiguredHostsResolver configuredHostsResolver
     ) {
         this.settings = settings;
         findPeersInterval = DISCOVERY_FIND_PEERS_INTERVAL_SETTING.get(settings);
         requestPeersTimeout = DISCOVERY_REQUEST_PEERS_TIMEOUT_SETTING.get(settings);
-        clusterSettings.addSettingsUpdateConsumer(DISCOVERY_REQUEST_PEERS_TIMEOUT_SETTING, this::setRequestPeersTimeout);
         this.transportService = transportService;
         this.transportAddressConnector = transportAddressConnector;
         this.configuredHostsResolver = configuredHostsResolver;

server/src/test/java/org/opensearch/cluster/coordination/CoordinationCheckerSettingsTests.java

@@ -14,8 +14,6 @@
 import org.opensearch.common.unit.TimeValue;
 import org.opensearch.test.OpenSearchSingleNodeTestCase;
 
-import static org.hamcrest.Matchers.equalTo;
-import static org.opensearch.cluster.coordination.FollowersChecker.FOLLOWER_CHECK_RETRY_COUNT_SETTING;
 import static org.opensearch.cluster.coordination.FollowersChecker.FOLLOWER_CHECK_TIMEOUT_SETTING;
 import static org.opensearch.cluster.coordination.LeaderChecker.LEADER_CHECK_TIMEOUT_SETTING;
 import static org.opensearch.common.unit.TimeValue.timeValueSeconds;
@@ -68,52 +66,6 @@ public void testFollowerCheckTimeoutMinValue() {
         );
     }
 
-    public void testFollowerCheckRetryCountValueUpdate(){
-        Setting<Integer> setting1 = FOLLOWER_CHECK_RETRY_COUNT_SETTING;
-        Settings retrySettings = Settings.builder().put(setting1.getKey(), "5").build();
-        try {
-            ClusterUpdateSettingsResponse response = client().admin()
-                .cluster()
-                .prepareUpdateSettings()
-                .setPersistentSettings(retrySettings)
-                .execute()
-                .actionGet();
-
-            assertAcked(response);
-            assertThat(setting1.get(response.getPersistentSettings()), equalTo(5));
-        } finally {
-            // cleanup
-            retrySettings = Settings.builder().putNull(setting1.getKey()).build();
-            client().admin().cluster().prepareUpdateSettings().setPersistentSettings(retrySettings).execute().actionGet();
-        }
-    }
-
-    public void testFollowerCheckRetryCountMaxValue(){
-        Setting<Integer> setting1 = FOLLOWER_CHECK_RETRY_COUNT_SETTING;
-        Settings countSettings = Settings.builder().put(setting1.getKey(), "12").build();
-
-        assertThrows(
-            "failed to parse value [12] for setting [" + setting1.getKey() + "], must be <= [10]",
-            IllegalArgumentException.class,
-            () -> {
-                client().admin().cluster().prepareUpdateSettings().setPersistentSettings(countSettings).execute().actionGet();
-            }
-        );
-    }
-
-    public void testFollowerCheckRetryCountMinValue(){
-        Setting<Integer> setting1 = FOLLOWER_CHECK_RETRY_COUNT_SETTING;
-        Settings countSettings = Settings.builder().put(setting1.getKey(), "0").build();
-
-        assertThrows(
-            "failed to parse value [0] for setting [" + setting1.getKey() + "], must be >= [1]",
-            IllegalArgumentException.class,
-            () -> {
-                client().admin().cluster().prepareUpdateSettings().setPersistentSettings(countSettings).execute().actionGet();
-            }
-        );
-    }
-
     public void testLeaderCheckTimeoutValueUpdate() {
         Setting<TimeValue> setting1 = LEADER_CHECK_TIMEOUT_SETTING;
         Settings timeSettings1 = Settings.builder().put(setting1.getKey(), "60s").build();

server/src/test/java/org/opensearch/cluster/coordination/ElectionSchedulerFactoryTests.java

@@ -33,7 +33,6 @@
 package org.opensearch.cluster.coordination;
 
 import org.opensearch.common.lease.Releasable;
-import org.opensearch.common.settings.ClusterSettings;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.settings.Settings.Builder;
 import org.opensearch.common.unit.TimeValue;
@@ -166,10 +165,8 @@ public void testRetriesOnCorrectSchedule() {
         final long duration = ELECTION_DURATION_SETTING.get(settings).millis();
 
         final DeterministicTaskQueue deterministicTaskQueue = new DeterministicTaskQueue(settings, random());
-        final ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
         final ElectionSchedulerFactory electionSchedulerFactory = new ElectionSchedulerFactory(
             settings,
-            clusterSettings,
             random(),
             deterministicTaskQueue.getThreadPool()
         );
@@ -237,8 +234,7 @@ public void testSettingsValidation() {
             assertThat(ELECTION_INITIAL_TIMEOUT_SETTING.get(settings), is(TimeValue.timeValueMillis(initialTimeoutMillis)));
             assertThat(ELECTION_BACK_OFF_TIME_SETTING.get(settings), is(TimeValue.timeValueMillis(backOffMillis)));
             assertThat(ELECTION_MAX_TIMEOUT_SETTING.get(settings), is(TimeValue.timeValueMillis(maxTimeoutMillis)));
-            final ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
-            assertThat(new ElectionSchedulerFactory(settings, clusterSettings, random(), null), not(nullValue())); // doesn't throw an IAE
+            assertThat(new ElectionSchedulerFactory(settings, random(), null), not(nullValue())); // doesn't throw an IAE
         }
 
         {
@@ -249,10 +245,9 @@ public void testSettingsValidation() {
                 .put(ELECTION_INITIAL_TIMEOUT_SETTING.getKey(), initialTimeoutMillis + "ms")
                 .put(ELECTION_MAX_TIMEOUT_SETTING.getKey(), maxTimeoutMillis + "ms")
                 .build();
-            final ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
             IllegalArgumentException e = expectThrows(
                 IllegalArgumentException.class,
-                () -> new ElectionSchedulerFactory(settings, clusterSettings, random(), null)
+                () -> new ElectionSchedulerFactory(settings, random(), null)
             );
             assertThat(
                 e.getMessage(),

server/src/test/java/org/opensearch/discovery/PeerFinderTests.java

@@ -39,7 +39,6 @@
 import org.opensearch.cluster.node.DiscoveryNode;
 import org.opensearch.cluster.node.DiscoveryNodes;
 import org.opensearch.cluster.node.DiscoveryNodes.Builder;
-import org.opensearch.common.settings.ClusterSettings;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.core.action.ActionListener;
 import org.opensearch.core.common.io.stream.StreamInput;
@@ -170,13 +169,8 @@ class TestPeerFinder extends PeerFinder {
         DiscoveryNode discoveredClusterManagerNode;
         OptionalLong discoveredClusterManagerTerm = OptionalLong.empty();
 
-        TestPeerFinder(
-            Settings settings,
-            ClusterSettings clusterSettings,
-            TransportService transportService,
-            TransportAddressConnector transportAddressConnector
-        ) {
-            super(settings, clusterSettings, transportService, transportAddressConnector, PeerFinderTests.this::resolveConfiguredHosts);
+        TestPeerFinder(Settings settings, TransportService transportService, TransportAddressConnector transportAddressConnector) {
+            super(settings, transportService, transportAddressConnector, PeerFinderTests.this::resolveConfiguredHosts);
         }
 
         @Override
@@ -257,8 +251,7 @@ public void setup() {
         transportService.acceptIncomingRequests();
 
         lastAcceptedNodes = DiscoveryNodes.builder().localNodeId(localNode.getId()).add(localNode).build();
-        final ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
-        peerFinder = new TestPeerFinder(settings, clusterSettings, transportService, transportAddressConnector);
+        peerFinder = new TestPeerFinder(settings, transportService, transportAddressConnector);
         foundPeersFromNotification = emptyList();
     }