Rename no-master-block setting (#38350)

Replaces `discovery.zen.no_master_block` with `cluster.no_master_block`. Any
value set for the old setting is now ignored.

Author: DaveCTurner

docs/reference/migration/migrate_7_0/discovery.asciidoc

@@ -38,3 +38,11 @@ file:
 - `discovery.seed_hosts`
 - `discovery.seed_providers`
 - `cluster.initial_master_nodes`
+
+[float]
+==== New name for `no_master_block` setting
+
+The `discovery.zen.no_master_block` setting is now known as
+`cluster.no_master_block`. Any value set for `discovery.zen.no_master_block` is
+now ignored. You should remove this setting and, if needed, set
+`cluster.no_master_block` appropriately after the upgrade.

docs/reference/modules/discovery/discovery-settings.asciidoc

@@ -134,7 +134,7 @@ Discovery and cluster formation are affected by the following settings:
     the addresses of seed nodes. By default, it is the
     <<settings-based-hosts-provider,settings-based hosts provider>>.
 
-[[no-master-block]]`discovery.zen.no_master_block`::
+[[no-master-block]]`cluster.no_master_block`::
 Specifies which operations are rejected when there is no active master in a
 cluster. This setting has two valid values:
 +
@@ -150,12 +150,16 @@ cluster.
 
 [NOTE]
 ===============================
-* The `discovery.zen.no_master_block` setting doesn't apply to nodes-based APIs
+* The `cluster.no_master_block` setting doesn't apply to nodes-based APIs
 (for example, cluster stats, node info, and node stats APIs). Requests to these
 APIs are not be blocked and can run on any available node.
   
 * For the cluster to be fully operational, it must have an active master.
 ===============================
+
+WARNING: This setting replaces the `discovery.zen.no_master_block` setting in
+earlier versions. The `discovery.zen.no_master_block` setting is ignored.
+
 --
 
 `discovery.seed_hosts`::

modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalResponseTests.java

@@ -24,6 +24,7 @@
 import org.elasticsearch.action.search.SearchPhaseExecutionException;
 import org.elasticsearch.action.search.ShardSearchFailure;
 import org.elasticsearch.cluster.block.ClusterBlockException;
+import org.elasticsearch.cluster.coordination.NoMasterBlockService;
 import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.breaker.CircuitBreaker;
 import org.elasticsearch.common.breaker.CircuitBreakingException;
@@ -37,7 +38,6 @@
 import org.elasticsearch.common.xcontent.XContentLocation;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentType;
-import org.elasticsearch.discovery.DiscoverySettings;
 import org.elasticsearch.index.shard.ShardId;
 import org.elasticsearch.search.SearchHit;
 import org.elasticsearch.search.SearchParseException;
@@ -64,7 +64,7 @@
 public class RankEvalResponseTests extends ESTestCase {
 
     private static final Exception[] RANDOM_EXCEPTIONS = new Exception[] {
-            new ClusterBlockException(singleton(DiscoverySettings.NO_MASTER_BLOCK_WRITES)),
+            new ClusterBlockException(singleton(NoMasterBlockService.NO_MASTER_BLOCK_WRITES)),
             new CircuitBreakingException("Data too large", 123, 456, CircuitBreaker.Durability.PERMANENT),
             new SearchParseException(new TestSearchContext(null), "Parse failure", new XContentLocation(12, 98)),
             new IllegalArgumentException("Closed resource", new RuntimeException("Resource")),

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

@@ -58,7 +58,6 @@
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.common.xcontent.json.JsonXContent;
 import org.elasticsearch.discovery.Discovery;
-import org.elasticsearch.discovery.DiscoverySettings;
 import org.elasticsearch.discovery.DiscoveryStats;
 import org.elasticsearch.discovery.HandshakingTransportAddressConnector;
 import org.elasticsearch.discovery.PeerFinder;
@@ -82,7 +81,7 @@
 import java.util.stream.Collectors;
 import java.util.stream.StreamSupport;
 
-import static org.elasticsearch.discovery.DiscoverySettings.NO_MASTER_BLOCK_ID;
+import static org.elasticsearch.cluster.coordination.NoMasterBlockService.NO_MASTER_BLOCK_ID;
 import static org.elasticsearch.gateway.ClusterStateUpdaters.hideStateIfNotRecovered;
 import static org.elasticsearch.gateway.GatewayService.STATE_NOT_RECOVERED_BLOCK;
 
@@ -103,7 +102,7 @@ public class Coordinator extends AbstractLifecycleComponent implements Discovery
     private final JoinHelper joinHelper;
     private final NodeRemovalClusterStateTaskExecutor nodeRemovalExecutor;
     private final Supplier<CoordinationState.PersistedState> persistedStateSupplier;
-    private final DiscoverySettings discoverySettings;
+    private final NoMasterBlockService noMasterBlockService;
     // TODO: the following field is package-private as some tests require access to it
     // These tests can be rewritten to use public methods once Coordinator is more feature-complete
     final Object mutex = new Object();
@@ -149,7 +148,7 @@ public Coordinator(String nodeName, Settings settings, ClusterSettings clusterSe
         this.joinHelper = new JoinHelper(settings, allocationService, masterService, transportService,
             this::getCurrentTerm, this::getStateForMasterService, this::handleJoinRequest, this::joinLeaderInTerm, this.onJoinValidators);
         this.persistedStateSupplier = persistedStateSupplier;
-        this.discoverySettings = new DiscoverySettings(settings, clusterSettings);
+        this.noMasterBlockService = new NoMasterBlockService(settings, clusterSettings);
         this.lastKnownLeader = Optional.empty();
         this.lastJoin = Optional.empty();
         this.joinAccumulator = new InitialJoinAccumulator();
@@ -632,7 +631,7 @@ protected void doStart() {
             ClusterState initialState = ClusterState.builder(ClusterName.CLUSTER_NAME_SETTING.get(settings))
                 .blocks(ClusterBlocks.builder()
                     .addGlobalBlock(STATE_NOT_RECOVERED_BLOCK)
-                    .addGlobalBlock(discoverySettings.getNoMasterBlock()))
+                    .addGlobalBlock(noMasterBlockService.getNoMasterBlock()))
                 .nodes(DiscoveryNodes.builder().add(getLocalNode()).localNodeId(getLocalNode().getId()))
                 .build();
             applierState = initialState;
@@ -934,7 +933,7 @@ private ClusterState clusterStateWithNoMasterBlock(ClusterState clusterState) {
             assert clusterState.blocks().hasGlobalBlockWithId(NO_MASTER_BLOCK_ID) == false :
                 "NO_MASTER_BLOCK should only be added by Coordinator";
             final ClusterBlocks clusterBlocks = ClusterBlocks.builder().blocks(clusterState.blocks()).addGlobalBlock(
-                discoverySettings.getNoMasterBlock()).build();
+                noMasterBlockService.getNoMasterBlock()).build();
             final DiscoveryNodes discoveryNodes = new DiscoveryNodes.Builder(clusterState.nodes()).masterNodeId(null).build();
             return ClusterState.builder(clusterState).blocks(clusterBlocks).nodes(discoveryNodes).build();
         } else {

server/src/main/java/org/elasticsearch/cluster/coordination/JoinTaskExecutor.java

@@ -30,7 +30,6 @@
 import org.elasticsearch.cluster.node.DiscoveryNodes;
 import org.elasticsearch.cluster.routing.allocation.AllocationService;
 import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.discovery.DiscoverySettings;
 import org.elasticsearch.discovery.zen.ElectMasterService;
 import org.elasticsearch.persistent.PersistentTasksCustomMetaData;
 
@@ -191,7 +190,7 @@ protected ClusterState.Builder becomeMasterAndTrimConflictingNodes(ClusterState
         // or removed by us above
         ClusterState tmpState = ClusterState.builder(currentState).nodes(nodesBuilder).blocks(ClusterBlocks.builder()
             .blocks(currentState.blocks())
-            .removeGlobalBlock(DiscoverySettings.NO_MASTER_BLOCK_ID))
+            .removeGlobalBlock(NoMasterBlockService.NO_MASTER_BLOCK_ID))
             .minimumMasterNodesOnPublishingMaster(minimumMasterNodesOnLocalNode)
             .build();
         logger.trace("becomeMasterAndTrimConflictingNodes: {}", tmpState.nodes());

server/src/main/java/org/elasticsearch/cluster/coordination/NoMasterBlockService.java

@@ -0,0 +1,73 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.elasticsearch.cluster.coordination;
+
+import org.elasticsearch.cluster.block.ClusterBlock;
+import org.elasticsearch.cluster.block.ClusterBlockLevel;
+import org.elasticsearch.common.settings.ClusterSettings;
+import org.elasticsearch.common.settings.Setting;
+import org.elasticsearch.common.settings.Setting.Property;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.rest.RestStatus;
+
+import java.util.EnumSet;
+
+public class NoMasterBlockService {
+    public static final int NO_MASTER_BLOCK_ID = 2;
+    public static final ClusterBlock NO_MASTER_BLOCK_WRITES = new ClusterBlock(NO_MASTER_BLOCK_ID, "no master", true, false, false,
+        RestStatus.SERVICE_UNAVAILABLE, EnumSet.of(ClusterBlockLevel.WRITE, ClusterBlockLevel.METADATA_WRITE));
+    public static final ClusterBlock NO_MASTER_BLOCK_ALL = new ClusterBlock(NO_MASTER_BLOCK_ID, "no master", true, true, false,
+        RestStatus.SERVICE_UNAVAILABLE, ClusterBlockLevel.ALL);
+
+    public static final Setting<ClusterBlock> LEGACY_NO_MASTER_BLOCK_SETTING =
+        new Setting<>("discovery.zen.no_master_block", "write", NoMasterBlockService::parseNoMasterBlock,
+            Property.Dynamic, Property.NodeScope, Property.Deprecated);
+    public static final Setting<ClusterBlock> NO_MASTER_BLOCK_SETTING =
+        new Setting<>("cluster.no_master_block", "write", NoMasterBlockService::parseNoMasterBlock,
+            Property.Dynamic, Property.NodeScope);
+
+    private volatile ClusterBlock noMasterBlock;
+
+    public NoMasterBlockService(Settings settings, ClusterSettings clusterSettings) {
+        this.noMasterBlock = NO_MASTER_BLOCK_SETTING.get(settings);
+        clusterSettings.addSettingsUpdateConsumer(NO_MASTER_BLOCK_SETTING, this::setNoMasterBlock);
+
+        LEGACY_NO_MASTER_BLOCK_SETTING.get(settings); // for deprecation warnings
+        clusterSettings.addSettingsUpdateConsumer(LEGACY_NO_MASTER_BLOCK_SETTING, b -> {}); // for deprecation warnings
+    }
+
+    private static ClusterBlock parseNoMasterBlock(String value) {
+        switch (value) {
+            case "all":
+                return NO_MASTER_BLOCK_ALL;
+            case "write":
+                return NO_MASTER_BLOCK_WRITES;
+            default:
+                throw new IllegalArgumentException("invalid no-master block [" + value + "], must be one of [all, write]");
+        }
+    }
+
+    public ClusterBlock getNoMasterBlock() {
+        return noMasterBlock;
+    }
+
+    private void setNoMasterBlock(ClusterBlock noMasterBlock) {
+        this.noMasterBlock = noMasterBlock;
+    }
+}

server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java

@@ -41,6 +41,7 @@
 import org.elasticsearch.cluster.coordination.JoinHelper;
 import org.elasticsearch.cluster.coordination.LagDetector;
 import org.elasticsearch.cluster.coordination.LeaderChecker;
+import org.elasticsearch.cluster.coordination.NoMasterBlockService;
 import org.elasticsearch.cluster.coordination.Reconfigurator;
 import org.elasticsearch.cluster.metadata.IndexGraveyard;
 import org.elasticsearch.cluster.metadata.MetaData;
@@ -232,7 +233,8 @@ public void apply(Settings value, Settings current, Settings previous) {
                     DiscoverySettings.PUBLISH_TIMEOUT_SETTING,
                     DiscoverySettings.PUBLISH_DIFF_ENABLE_SETTING,
                     DiscoverySettings.COMMIT_TIMEOUT_SETTING,
-                    DiscoverySettings.NO_MASTER_BLOCK_SETTING,
+                    NoMasterBlockService.NO_MASTER_BLOCK_SETTING,
+                    NoMasterBlockService.LEGACY_NO_MASTER_BLOCK_SETTING,
                     GatewayService.EXPECTED_DATA_NODES_SETTING,
                     GatewayService.EXPECTED_MASTER_NODES_SETTING,
                     GatewayService.EXPECTED_NODES_SETTING,

server/src/main/java/org/elasticsearch/discovery/DiscoverySettings.java

@@ -19,27 +19,17 @@
 
 package org.elasticsearch.discovery;
 
-import org.elasticsearch.cluster.block.ClusterBlock;
-import org.elasticsearch.cluster.block.ClusterBlockLevel;
 import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Setting.Property;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.TimeValue;
-import org.elasticsearch.rest.RestStatus;
-
-import java.util.EnumSet;
 
 /**
  * Exposes common discovery settings that may be supported by all the different discovery implementations
  */
 public class DiscoverySettings {
 
-    public static final int NO_MASTER_BLOCK_ID = 2;
-    public static final ClusterBlock NO_MASTER_BLOCK_ALL = new ClusterBlock(NO_MASTER_BLOCK_ID, "no master", true, true, false,
-        RestStatus.SERVICE_UNAVAILABLE, ClusterBlockLevel.ALL);
-    public static final ClusterBlock NO_MASTER_BLOCK_WRITES = new ClusterBlock(NO_MASTER_BLOCK_ID, "no master", true, false, false,
-        RestStatus.SERVICE_UNAVAILABLE, EnumSet.of(ClusterBlockLevel.WRITE, ClusterBlockLevel.METADATA_WRITE));
     /**
      * sets the timeout for a complete publishing cycle, including both sending and committing. the master
      * will continue to process the next cluster state update after this time has elapsed
@@ -56,26 +46,20 @@ public class DiscoverySettings {
         new Setting<>("discovery.zen.commit_timeout", PUBLISH_TIMEOUT_SETTING::getRaw,
             (s) -> TimeValue.parseTimeValue(s, TimeValue.timeValueSeconds(30), "discovery.zen.commit_timeout"),
             Property.Dynamic, Property.NodeScope, Property.Deprecated);
-    public static final Setting<ClusterBlock> NO_MASTER_BLOCK_SETTING =
-        new Setting<>("discovery.zen.no_master_block", "write", DiscoverySettings::parseNoMasterBlock,
-            Property.Dynamic, Property.NodeScope);
     public static final Setting<Boolean> PUBLISH_DIFF_ENABLE_SETTING =
         Setting.boolSetting("discovery.zen.publish_diff.enable", true, Property.Dynamic, Property.NodeScope, Property.Deprecated);
     public static final Setting<TimeValue> INITIAL_STATE_TIMEOUT_SETTING =
         Setting.positiveTimeSetting("discovery.initial_state_timeout", TimeValue.timeValueSeconds(30), Property.NodeScope);
 
-    private volatile ClusterBlock noMasterBlock;
     private volatile TimeValue publishTimeout;
 
     private volatile TimeValue commitTimeout;
     private volatile boolean publishDiff;
 
     public DiscoverySettings(Settings settings, ClusterSettings clusterSettings) {
-        clusterSettings.addSettingsUpdateConsumer(NO_MASTER_BLOCK_SETTING, this::setNoMasterBlock);
         clusterSettings.addSettingsUpdateConsumer(PUBLISH_DIFF_ENABLE_SETTING, this::setPublishDiff);
         clusterSettings.addSettingsUpdateConsumer(COMMIT_TIMEOUT_SETTING, this::setCommitTimeout);
         clusterSettings.addSettingsUpdateConsumer(PUBLISH_TIMEOUT_SETTING, this::setPublishTimeout);
-        this.noMasterBlock = NO_MASTER_BLOCK_SETTING.get(settings);
         this.publishTimeout = PUBLISH_TIMEOUT_SETTING.get(settings);
         this.commitTimeout = COMMIT_TIMEOUT_SETTING.get(settings);
         this.publishDiff = PUBLISH_DIFF_ENABLE_SETTING.get(settings);
@@ -92,14 +76,6 @@ public TimeValue getCommitTimeout() {
         return commitTimeout;
     }
 
-    public ClusterBlock getNoMasterBlock() {
-        return noMasterBlock;
-    }
-
-    private void setNoMasterBlock(ClusterBlock noMasterBlock) {
-        this.noMasterBlock = noMasterBlock;
-    }
-
     private void setPublishDiff(boolean publishDiff) {
         this.publishDiff = publishDiff;
     }
@@ -114,14 +90,4 @@ private void setCommitTimeout(TimeValue commitTimeout) {
 
     public boolean getPublishDiff() { return publishDiff;}
 
-    private static ClusterBlock parseNoMasterBlock(String value) {
-        switch (value) {
-            case "all":
-                return NO_MASTER_BLOCK_ALL;
-            case "write":
-                return NO_MASTER_BLOCK_WRITES;
-            default:
-                throw new IllegalArgumentException("invalid master block [" + value + "]");
-        }
-    }
 }