diff --git a/core/src/main/java/org/elasticsearch/action/support/ThreadedActionListener.java b/core/src/main/java/org/elasticsearch/action/support/ThreadedActionListener.java index 6ed52e1ac46..1eec4c8d9e9 100644 --- a/core/src/main/java/org/elasticsearch/action/support/ThreadedActionListener.java +++ b/core/src/main/java/org/elasticsearch/action/support/ThreadedActionListener.java @@ -22,7 +22,6 @@ package org.elasticsearch.action.support; import org.elasticsearch.action.ActionListener; import org.elasticsearch.client.Client; import org.elasticsearch.client.transport.TransportClient; -import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.common.logging.ESLogger; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.AbstractRunnable; @@ -48,9 +47,9 @@ public final class ThreadedActionListener implements ActionListener ActionListener wrap(ActionListener listener) { diff --git a/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java b/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java index 87a8ca53efd..39d4a2f1e60 100644 --- a/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java +++ b/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java @@ -112,7 +112,10 @@ public class TransportClient extends AbstractClient { .put(NettyTransport.PING_SCHEDULE.getKey(), "5s") // enable by default the transport schedule ping interval .put(InternalSettingsPreparer.prepareSettings(settings)) .put(NetworkService.NETWORK_SERVER.getKey(), false) - .put(Node.NODE_CLIENT_SETTING.getKey(), true) + //nocommit not too sure if these settings are needed here... + .put(Node.NODE_MASTER_SETTING.getKey(), false) + .put(Node.NODE_DATA_SETTING.getKey(), false) + .put(Node.NODE_INGEST_SETTING.getKey(), false) .put(CLIENT_TYPE_SETTING_S.getKey(), CLIENT_TYPE); return new PluginsService(settingsBuilder.build(), null, null, pluginClasses); } diff --git a/core/src/main/java/org/elasticsearch/cluster/node/DiscoveryNode.java b/core/src/main/java/org/elasticsearch/cluster/node/DiscoveryNode.java index 03506643eeb..fdecfe39e45 100644 --- a/core/src/main/java/org/elasticsearch/cluster/node/DiscoveryNode.java +++ b/core/src/main/java/org/elasticsearch/cluster/node/DiscoveryNode.java @@ -48,7 +48,6 @@ public class DiscoveryNode implements Streamable, ToXContent { public static final String DATA_ATTR = "data"; public static final String MASTER_ATTR = "master"; - public static final String CLIENT_ATTR = "client"; public static final String INGEST_ATTR = "ingest"; public static boolean localNode(Settings settings) { @@ -69,25 +68,15 @@ public class DiscoveryNode implements Streamable, ToXContent { } public static boolean nodeRequiresLocalStorage(Settings settings) { - return (Node.NODE_CLIENT_SETTING.get(settings) || (Node.NODE_DATA_SETTING.get(settings) == false && Node.NODE_MASTER_SETTING.get(settings) == false)) == false; - } - - public static boolean clientNode(Settings settings) { - return Node.NODE_CLIENT_SETTING.get(settings); + return Node.NODE_DATA_SETTING.get(settings) || Node.NODE_MASTER_SETTING.get(settings); } public static boolean masterNode(Settings settings) { - if (Node.NODE_MASTER_SETTING.exists(settings)) { - return Node.NODE_MASTER_SETTING.get(settings); - } - return clientNode(settings) == false; + return Node.NODE_MASTER_SETTING.get(settings); } public static boolean dataNode(Settings settings) { - if (Node.NODE_DATA_SETTING.exists(settings)) { - return Node.NODE_DATA_SETTING.get(settings); - } - return clientNode(settings) == false; + return Node.NODE_DATA_SETTING.get(settings); } public static boolean ingestNode(Settings settings) { @@ -270,10 +259,7 @@ public class DiscoveryNode implements Streamable, ToXContent { */ public boolean dataNode() { String data = attributes.get(DATA_ATTR); - if (data == null) { - return !clientNode(); - } - return Booleans.parseBooleanExact(data); + return data == null ? true : Booleans.parseBooleanExact(data); } /** @@ -283,27 +269,12 @@ public class DiscoveryNode implements Streamable, ToXContent { return dataNode(); } - /** - * Is the node a client node or not. - */ - public boolean clientNode() { - String client = attributes.get(CLIENT_ATTR); - return client != null && Booleans.parseBooleanExact(client); - } - - public boolean isClientNode() { - return clientNode(); - } - /** * Can this node become master or not. */ public boolean masterNode() { - String master = attributes.get(MASTER_ATTR); - if (master == null) { - return !clientNode(); - } - return Booleans.parseBooleanExact(master); + String master = attributes.get(MASTER_ATTR); + return master == null ? true : Booleans.parseBooleanExact(master); } /** diff --git a/core/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodeService.java b/core/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodeService.java index 83f603d2890..c237b78e98e 100644 --- a/core/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodeService.java +++ b/core/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodeService.java @@ -22,6 +22,7 @@ package org.elasticsearch.cluster.node; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.node.Node; import java.util.HashMap; import java.util.List; @@ -45,16 +46,13 @@ public class DiscoveryNodeService extends AbstractComponent { } public Map buildAttributes() { - Map attributes = new HashMap<>(settings.getByPrefix("node.").getAsMap()); + Map attributes = new HashMap<>(Node.NODE_ATTRIBUTES.get(this.settings).getAsMap()); attributes.remove("name"); // name is extracted in other places if (attributes.containsKey("client")) { - if (attributes.get("client").equals("false")) { - attributes.remove("client"); // this is the default - } else { - // if we are client node, don't store data ... - attributes.put("data", "false"); - } + throw new IllegalArgumentException("node.client setting is no longer supported, use " + Node.NODE_MASTER_SETTING.getKey() + + ", " + Node.NODE_DATA_SETTING.getKey() + " and " + Node.NODE_INGEST_SETTING.getKey() + " explicitly instead"); } + //nocommit why don't we remove master as well if it's true? and ingest? if (attributes.containsKey("data")) { if (attributes.get("data").equals("true")) { attributes.remove("data"); @@ -79,7 +77,7 @@ public class DiscoveryNodeService extends AbstractComponent { return attributes; } - public static interface CustomAttributesProvider { + public interface CustomAttributesProvider { Map buildAttributes(); } diff --git a/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java index fa8b8c4ac41..9d7fc660cfc 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java @@ -350,7 +350,6 @@ public final class ClusterSettings extends AbstractScopedSettings { SearchService.KEEPALIVE_INTERVAL_SETTING, Node.WRITE_PORTS_FIELD_SETTING, Node.NODE_NAME_SETTING, - Node.NODE_CLIENT_SETTING, Node.NODE_DATA_SETTING, Node.NODE_MASTER_SETTING, Node.NODE_LOCAL_SETTING, 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 fb0f7a61966..ab5d61e7aa0 100644 --- a/core/src/main/java/org/elasticsearch/discovery/zen/ZenDiscovery.java +++ b/core/src/main/java/org/elasticsearch/discovery/zen/ZenDiscovery.java @@ -853,10 +853,11 @@ public class ZenDiscovery extends AbstractLifecycleComponent implemen List pingResponses = new ArrayList<>(); for (ZenPing.PingResponse pingResponse : fullPingResponses) { DiscoveryNode node = pingResponse.node(); - if (masterElectionFilterClientNodes && (node.clientNode() || (!node.masterNode() && !node.dataNode()))) { - // filter out the client node, which is a client node, or also one that is not data and not master (effectively, client) - } else if (masterElectionFilterDataNodes && (!node.masterNode() && node.dataNode())) { - // filter out data node that is not also master + //nocommit we should rename this and its setting, also we ignore node.ingest, but maybe it's ok here + if (masterElectionFilterClientNodes && node.masterNode() == false && node.dataNode() == false) { + // filter out nodes that don't hold data and are not master eligible + } else if (masterElectionFilterDataNodes && node.masterNode() == false && node.dataNode()) { + // filter out dedicated data nodes } else { pingResponses.add(pingResponse); } diff --git a/core/src/main/java/org/elasticsearch/node/Node.java b/core/src/main/java/org/elasticsearch/node/Node.java index d115fbb8773..a78a08ef32a 100644 --- a/core/src/main/java/org/elasticsearch/node/Node.java +++ b/core/src/main/java/org/elasticsearch/node/Node.java @@ -130,14 +130,13 @@ import static org.elasticsearch.common.settings.Settings.settingsBuilder; public class Node implements Closeable { public static final Setting WRITE_PORTS_FIELD_SETTING = Setting.boolSetting("node.portsfile", false, false, Setting.Scope.CLUSTER); - public static final Setting NODE_CLIENT_SETTING = Setting.boolSetting("node.client", false, false, Setting.Scope.CLUSTER); public static final Setting NODE_DATA_SETTING = Setting.boolSetting("node.data", true, false, Setting.Scope.CLUSTER); public static final Setting NODE_MASTER_SETTING = Setting.boolSetting("node.master", true, false, Setting.Scope.CLUSTER); public static final Setting NODE_LOCAL_SETTING = Setting.boolSetting("node.local", false, false, Setting.Scope.CLUSTER); public static final Setting NODE_MODE_SETTING = new Setting<>("node.mode", "network", Function.identity(), false, Setting.Scope.CLUSTER); public static final Setting NODE_INGEST_SETTING = Setting.boolSetting("node.ingest", true, false, Setting.Scope.CLUSTER); public static final Setting NODE_NAME_SETTING = Setting.simpleString("node.name", false, Setting.Scope.CLUSTER); - // this sucks that folks can mistype client etc and get away with it. + // this sucks that folks can mistype data, master or ingest and get away with it. // TODO: we should move this to node.attribute.${name} = ${value} instead. public static final Setting NODE_ATTRIBUTES = Setting.groupSetting("node.", false, Setting.Scope.CLUSTER); diff --git a/core/src/main/java/org/elasticsearch/tribe/TribeService.java b/core/src/main/java/org/elasticsearch/tribe/TribeService.java index bf66cce1b9e..6f30a4931ab 100644 --- a/core/src/main/java/org/elasticsearch/tribe/TribeService.java +++ b/core/src/main/java/org/elasticsearch/tribe/TribeService.java @@ -107,7 +107,9 @@ public class TribeService extends AbstractLifecycleComponent { } // its a tribe configured node..., force settings Settings.Builder sb = Settings.builder().put(settings); - sb.put(Node.NODE_CLIENT_SETTING.getKey(), true); // this node should just act as a node client + sb.put(Node.NODE_MASTER_SETTING.getKey(), false); + sb.put(Node.NODE_DATA_SETTING.getKey(), false); + sb.put(Node.NODE_INGEST_SETTING.getKey(), false); sb.put(DiscoveryModule.DISCOVERY_TYPE_SETTING.getKey(), "local"); // a tribe node should not use zen discovery // nothing is going to be discovered, since no master will be elected sb.put(DiscoverySettings.INITIAL_STATE_TIMEOUT_SETTING.getKey(), 0); @@ -177,7 +179,9 @@ public class TribeService extends AbstractLifecycleComponent { if (sb.get("http.enabled") == null) { sb.put("http.enabled", false); } - sb.put(Node.NODE_CLIENT_SETTING.getKey(), true); + sb.put(Node.NODE_DATA_SETTING.getKey(), false); + sb.put(Node.NODE_MASTER_SETTING.getKey(), false); + sb.put(Node.NODE_INGEST_SETTING.getKey(), false); nodes.add(new TribeClientNode(sb.build())); } diff --git a/core/src/test/java/org/elasticsearch/action/ListenerActionIT.java b/core/src/test/java/org/elasticsearch/action/ListenerActionIT.java index a9bb96a0a9f..3976f665df9 100644 --- a/core/src/test/java/org/elasticsearch/action/ListenerActionIT.java +++ b/core/src/test/java/org/elasticsearch/action/ListenerActionIT.java @@ -23,7 +23,6 @@ import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.action.index.IndexResponse; import org.elasticsearch.client.Client; import org.elasticsearch.client.transport.TransportClient; -import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.test.ESIntegTestCase; import java.util.concurrent.CountDownLatch; @@ -61,7 +60,7 @@ public class ListenerActionIT extends ESIntegTestCase { latch.await(); - boolean shouldBeThreaded = DiscoveryNode.clientNode(client.settings()) || TransportClient.CLIENT_TYPE.equals(Client.CLIENT_TYPE_SETTING_S.get(client.settings())); + boolean shouldBeThreaded = TransportClient.CLIENT_TYPE.equals(Client.CLIENT_TYPE_SETTING_S.get(client.settings())); if (shouldBeThreaded) { assertTrue(threadName.get().contains("listener")); } else { diff --git a/core/src/test/java/org/elasticsearch/cluster/node/DiscoveryNodeServiceTests.java b/core/src/test/java/org/elasticsearch/cluster/node/DiscoveryNodeServiceTests.java new file mode 100644 index 00000000000..fda2b6a3f47 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/cluster/node/DiscoveryNodeServiceTests.java @@ -0,0 +1,63 @@ +/* + * 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.node; + +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.test.ESTestCase; + +import java.util.HashMap; +import java.util.Map; + +import static org.hamcrest.CoreMatchers.equalTo; + +public class DiscoveryNodeServiceTests extends ESTestCase { + + public void testClientNodeSettingIsProhibited() { + Settings settings = Settings.builder().put("node.client", randomBoolean()).build(); + try { + new DiscoveryNodeService(settings).buildAttributes(); + fail("build attributes should have failed"); + } catch(IllegalArgumentException e) { + assertThat(e.getMessage(), equalTo("node.client setting is no longer supported, use node.master, " + + "node.data and node.ingest explicitly instead")); + } + } + + public void testBuildAttributesWithCustomAttributeServiceProvider() { + Map expectedAttributes = new HashMap<>(); + int numCustomSettings = randomIntBetween(0, 5); + Settings.Builder builder = Settings.builder(); + for (int i = 0; i < numCustomSettings; i++) { + builder.put("node.attr" + i, "value" + i); + expectedAttributes.put("attr" + i, "value" + i); + } + DiscoveryNodeService discoveryNodeService = new DiscoveryNodeService(builder.build()); + int numCustomAttributes = randomIntBetween(0, 5); + Map customAttributes = new HashMap<>(); + for (int i = 0; i < numCustomAttributes; i++) { + customAttributes.put("custom-" + randomAsciiOfLengthBetween(5, 10), randomAsciiOfLengthBetween(1, 10)); + } + expectedAttributes.putAll(customAttributes); + discoveryNodeService.addCustomAttributeProvider(() -> customAttributes); + + Map attributes = discoveryNodeService.buildAttributes(); + assertThat(attributes, equalTo(expectedAttributes)); + } +} diff --git a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java index 04548eb85c9..51fab2d2f4f 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java @@ -656,19 +656,20 @@ public final class InternalTestCluster extends TestCluster { */ public synchronized Client clientNodeClient() { ensureOpen(); - NodeAndClient randomNodeAndClient = getRandomNodeAndClient(new ClientNodePredicate()); + NodeAndClient randomNodeAndClient = getRandomNodeAndClient(new NoDataNoMasterNodePredicate()); if (randomNodeAndClient != null) { return randomNodeAndClient.client(random); } int nodeId = nextNodeId.getAndIncrement(); Settings settings = getSettings(nodeId, random.nextLong(), Settings.EMPTY); startNodeClient(settings); - return getRandomNodeAndClient(new ClientNodePredicate()).client(random); + return getRandomNodeAndClient(new NoDataNoMasterNodePredicate()).client(random); } public synchronized Client startNodeClient(Settings settings) { ensureOpen(); // currently unused - Builder builder = settingsBuilder().put(settings).put(Node.NODE_CLIENT_SETTING.getKey(), true); + Builder builder = settingsBuilder().put(settings).put(Node.NODE_MASTER_SETTING.getKey(), false) + .put(Node.NODE_DATA_SETTING.getKey(), false); if (size() == 0) { // if we are the first node - don't wait for a state builder.put(DiscoverySettings.INITIAL_STATE_TIMEOUT_SETTING.getKey(), 0); @@ -952,7 +953,8 @@ public final class InternalTestCluster extends TestCluster { NodeAndClient nodeAndClient = nodes.get(buildNodeName); if (nodeAndClient == null) { changed = true; - Builder clientSettingsBuilder = Settings.builder().put(Node.NODE_CLIENT_SETTING.getKey(), true); + Builder clientSettingsBuilder = Settings.builder().put(Node.NODE_MASTER_SETTING.getKey(), false) + .put(Node.NODE_DATA_SETTING.getKey(), false); nodeAndClient = buildNode(i, sharedNodesSeeds[i], clientSettingsBuilder.build(), Version.CURRENT); nodeAndClient.node.start(); logger.info("Start Shared Node [{}] not shared", nodeAndClient.name); @@ -1677,10 +1679,11 @@ public final class InternalTestCluster extends TestCluster { } } - private static final class ClientNodePredicate implements Predicate { + private static final class NoDataNoMasterNodePredicate implements Predicate { @Override public boolean test(NodeAndClient nodeAndClient) { - return DiscoveryNode.clientNode(nodeAndClient.node.settings()); + return DiscoveryNode.masterNode(nodeAndClient.node.settings()) == false && + DiscoveryNode.dataNode(nodeAndClient.node.settings()) == false; } }