From 117d79b5e92dd962ffebf8fb1bf700d67cf93cc6 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 27 Oct 2020 12:47:14 -0400 Subject: [PATCH] Adjust defaults for tiered data roles (#64015) This commit adjusts the defaults for the tiered data roles so that they are enabled by default, or if the node has the legacy data role. This ensures that the default experience is that the tiered data roles are enabled. To fully specifiy the behavior for the tiered data roles then: - starting a new node with the defaults: enabled - starting a new node with node.roles configured: enabled if and only if the tiered data roles are explicitly configured, independently of the node having the data role - starting a new node with node.data enabled: enabled unless the tiered data roles are explicitly disabled - starting a new node with node.data disabled: disabled unless the tiered data roles are explicitly enabled --- .../resources/rest-api-spec/test/11_nodes.yml | 8 +-- docs/reference/cat/nodes.asciidoc | 8 +-- .../rest-api-spec/test/cat.nodes/10_basic.yml | 8 +-- .../elasticsearch/xpack/core/DataTier.java | 48 ++++++++++++++--- .../xpack/core/DataTierTests.java | 53 +++++++++++++++++++ 5 files changed, 106 insertions(+), 19 deletions(-) diff --git a/distribution/docker/src/test/resources/rest-api-spec/test/11_nodes.yml b/distribution/docker/src/test/resources/rest-api-spec/test/11_nodes.yml index acda1a3dcedf..af1ed146c41a 100644 --- a/distribution/docker/src/test/resources/rest-api-spec/test/11_nodes.yml +++ b/distribution/docker/src/test/resources/rest-api-spec/test/11_nodes.yml @@ -6,8 +6,8 @@ - match: $body: | - / #ip heap.percent ram.percent cpu load_1m load_5m load_15m node.role master name - ^ ((\d{1,3}\.){3}\d{1,3} \s+ \d+ \s+ \d* \s+ (-)?\d* \s+ ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)?\s+ ((-)?\d*(\.\d+)?)? \s+ (-|[dilmrt]{1,6}) \s+ [-*x] \s+ (\S+\s?)+ \n)+ $/ + / #ip heap.percent ram.percent cpu load_1m load_5m load_15m node.role master name + ^ ((\d{1,3}\.){3}\d{1,3} \s+ \d+ \s+ \d* \s+ (-)?\d* \s+ ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)?\s+ ((-)?\d*(\.\d+)?)? \s+ (-|[cdhilmrstvw]{1,11}) \s+ [-*x] \s+ (\S+\s?)+ \n)+ $/ - do: cat.nodes: @@ -15,8 +15,8 @@ - match: $body: | - /^ ip \s+ heap\.percent \s+ ram\.percent \s+ cpu \s+ load_1m \s+ load_5m \s+ load_15m \s+ node\.role \s+ master \s+ name \n - ((\d{1,3}\.){3}\d{1,3} \s+ \d+ \s+ \d* \s+ (-)?\d* \s+ ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)? \s+ (-|[dilmrt]{1,6}) \s+ [-*x] \s+ (\S+\s?)+ \n)+ $/ + /^ ip \s+ heap\.percent \s+ ram\.percent \s+ cpu \s+ load_1m \s+ load_5m \s+ load_15m \s+ node\.role \s+ master \s+ name \n + ((\d{1,3}\.){3}\d{1,3} \s+ \d+ \s+ \d* \s+ (-)?\d* \s+ ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)? \s+ (-|[cdhilmrstvw]{1,11}) \s+ [-*x] \s+ (\S+\s?)+ \n)+ $/ - do: cat.nodes: diff --git a/docs/reference/cat/nodes.asciidoc b/docs/reference/cat/nodes.asciidoc index 7a731deabdf0..92c0e3fa1431 100644 --- a/docs/reference/cat/nodes.asciidoc +++ b/docs/reference/cat/nodes.asciidoc @@ -42,9 +42,11 @@ Valid columns are: (Default) Used file descriptors percentage, such as `1`. `node.role`, `r`, `role`, `nodeRole`:: -(Default) Roles of the node. Returned values include `d` (data node), `i` -(ingest node), `m` (master-eligible node), `l` (machine learning node), `v` -(voting-only node), `t` ({transform} node), `r` (remote cluster client node), and `-` (coordinating node only). +(Default) Roles of the node. Returned values include `c` (cold node), `d` (data +node), `h` (hot node), `i` (ingest node), `l` (machine learning node), `m` +(master-eligible node), `r` (remote cluster client node), `s` (content node), +`t` ({transform} node), `v` (voting-only node), `w` (warm node) and `-` +(coordinating node only). + For example, `dim` indicates a master-eligible data and ingest node. See <>. diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/cat.nodes/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/cat.nodes/10_basic.yml index 4c28f0bec5df..16bf099081c6 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/cat.nodes/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/cat.nodes/10_basic.yml @@ -6,8 +6,8 @@ - match: $body: | - / #ip heap.percent ram.percent cpu load_1m load_5m load_15m node.role master name - ^ ((\d{1,3}\.){3}\d{1,3} \s+ \d+ \s+ \d* \s+ (-)?\d* \s+ ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)?\s+ ((-)?\d*(\.\d+)?)? \s+ (-|[dilmrt]{1,6}) \s+ [-*x] \s+ (\S+\s?)+ \n)+ $/ + / #ip heap.percent ram.percent cpu load_1m load_5m load_15m node.role master name + ^ ((\d{1,3}\.){3}\d{1,3} \s+ \d+ \s+ \d* \s+ (-)?\d* \s+ ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)?\s+ ((-)?\d*(\.\d+)?)? \s+ (-|[cdhilmrstvw]{1,11}) \s+ [-*x] \s+ (\S+\s?)+ \n)+ $/ - do: cat.nodes: @@ -15,8 +15,8 @@ - match: $body: | - /^ ip \s+ heap\.percent \s+ ram\.percent \s+ cpu \s+ load_1m \s+ load_5m \s+ load_15m \s+ node\.role \s+ master \s+ name \n - ((\d{1,3}\.){3}\d{1,3} \s+ \d+ \s+ \d* \s+ (-)?\d* \s+ ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)? \s+ (-|[dilmrt]{1,6}) \s+ [-*x] \s+ (\S+\s?)+ \n)+ $/ + /^ ip \s+ heap\.percent \s+ ram\.percent \s+ cpu \s+ load_1m \s+ load_5m \s+ load_15m \s+ node\.role \s+ master \s+ name \n + ((\d{1,3}\.){3}\d{1,3} \s+ \d+ \s+ \d* \s+ (-)?\d* \s+ ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)? \s+ (-|[cdhilmrstvw]{1,11}) \s+ [-*x] \s+ (\S+\s?)+ \n)+ $/ - do: cat.nodes: diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/DataTier.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/DataTier.java index bc12b8f04315..d99a8e1deeb7 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/DataTier.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/DataTier.java @@ -69,12 +69,20 @@ public class DataTier { public static DiscoveryNodeRole DATA_CONTENT_NODE_ROLE = new DiscoveryNodeRole("data_content", "s") { @Override public boolean isEnabledByDefault(final Settings settings) { - return false; + return DiscoveryNode.hasRole(settings, DiscoveryNodeRole.DATA_ROLE); } @Override public Setting legacySetting() { - return null; + // we do not register these settings, they're not intended to be used externally, only for proper defaults + return Setting.boolSetting( + "node.data_content", + settings -> + // Don't use DiscoveryNode#isDataNode(Settings) here, as it is called before all plugins are initialized + Boolean.toString(DiscoveryNode.hasRole(settings, DiscoveryNodeRole.DATA_ROLE)), + Setting.Property.Deprecated, + Setting.Property.NodeScope + ); } @Override @@ -91,12 +99,20 @@ public class DataTier { public static DiscoveryNodeRole DATA_HOT_NODE_ROLE = new DiscoveryNodeRole("data_hot", "h") { @Override public boolean isEnabledByDefault(final Settings settings) { - return false; + return DiscoveryNode.hasRole(settings, DiscoveryNodeRole.DATA_ROLE); } @Override public Setting legacySetting() { - return null; + // we do not register these settings, they're not intended to be used externally, only for proper defaults + return Setting.boolSetting( + "node.data_hot", + settings -> + // Don't use DiscoveryNode#isDataNode(Settings) here, as it is called before all plugins are initialized + Boolean.toString(DiscoveryNode.hasRole(settings, DiscoveryNodeRole.DATA_ROLE)), + Setting.Property.Deprecated, + Setting.Property.NodeScope + ); } @Override @@ -113,12 +129,20 @@ public class DataTier { public static DiscoveryNodeRole DATA_WARM_NODE_ROLE = new DiscoveryNodeRole("data_warm", "w") { @Override public boolean isEnabledByDefault(final Settings settings) { - return false; + return DiscoveryNode.hasRole(settings, DiscoveryNodeRole.DATA_ROLE); } @Override public Setting legacySetting() { - return null; + // we do not register these settings, they're not intended to be used externally, only for proper defaults + return Setting.boolSetting( + "node.data_warm", + settings -> + // Don't use DiscoveryNode#isDataNode(Settings) here, as it is called before all plugins are initialized + Boolean.toString(DiscoveryNode.hasRole(settings, DiscoveryNodeRole.DATA_ROLE)), + Setting.Property.Deprecated, + Setting.Property.NodeScope + ); } @Override @@ -135,12 +159,20 @@ public class DataTier { public static DiscoveryNodeRole DATA_COLD_NODE_ROLE = new DiscoveryNodeRole("data_cold", "c") { @Override public boolean isEnabledByDefault(final Settings settings) { - return false; + return DiscoveryNode.hasRole(settings, DiscoveryNodeRole.DATA_ROLE); } @Override public Setting legacySetting() { - return null; + // we do not register these settings, they're not intended to be used externally, only for proper defaults + return Setting.boolSetting( + "node.data_cold", + settings -> + // Don't use DiscoveryNode#isDataNode(Settings) here, as it is called before all plugins are initialized + Boolean.toString(DiscoveryNode.hasRole(settings, DiscoveryNodeRole.DATA_ROLE)), + Setting.Property.Deprecated, + Setting.Property.NodeScope + ); } @Override diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/DataTierTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/DataTierTests.java index 368d82da203b..8baed3ea4dd0 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/DataTierTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/DataTierTests.java @@ -12,6 +12,8 @@ import org.elasticsearch.cluster.node.DiscoveryNodeRole; import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.Setting; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.node.NodeRoleSettings; import org.elasticsearch.test.ESTestCase; import java.util.ArrayList; @@ -25,6 +27,8 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.StreamSupport; import static org.hamcrest.Matchers.arrayContainingInAnyOrder; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.not; public class DataTierTests extends ESTestCase { @@ -80,6 +84,55 @@ public class DataTierTests extends ESTestCase { assertThat(discoveryNodes.resolveNodes("data:true"), arrayContainingInAnyOrder(allTiers.toArray(Strings.EMPTY_ARRAY))); } + public void testDefaultRolesImpliesTieredDataRoles() { + DiscoveryNode.setAdditionalRoles( + Set.of(DataTier.DATA_CONTENT_NODE_ROLE, DataTier.DATA_HOT_NODE_ROLE, DataTier.DATA_WARM_NODE_ROLE, DataTier.DATA_COLD_NODE_ROLE) + ); + final DiscoveryNode node = DiscoveryNode.createLocal(Settings.EMPTY, buildNewFakeTransportAddress(), randomAlphaOfLength(8)); + assertThat(node.getRoles(), hasItem(DataTier.DATA_CONTENT_NODE_ROLE)); + assertThat(node.getRoles(), hasItem(DataTier.DATA_HOT_NODE_ROLE)); + assertThat(node.getRoles(), hasItem(DataTier.DATA_WARM_NODE_ROLE)); + assertThat(node.getRoles(), hasItem(DataTier.DATA_COLD_NODE_ROLE)); + } + + public void testDataRoleDoesNotImplyTieredDataRoles() { + DiscoveryNode.setAdditionalRoles( + Set.of(DataTier.DATA_CONTENT_NODE_ROLE, DataTier.DATA_HOT_NODE_ROLE, DataTier.DATA_WARM_NODE_ROLE, DataTier.DATA_COLD_NODE_ROLE) + ); + final Settings settings = Settings.builder().put(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), "data").build(); + final DiscoveryNode node = DiscoveryNode.createLocal(settings, buildNewFakeTransportAddress(), randomAlphaOfLength(8)); + assertThat(node.getRoles(), not(hasItem(DataTier.DATA_CONTENT_NODE_ROLE))); + assertThat(node.getRoles(), not(hasItem(DataTier.DATA_HOT_NODE_ROLE))); + assertThat(node.getRoles(), not(hasItem(DataTier.DATA_WARM_NODE_ROLE))); + assertThat(node.getRoles(), not(hasItem(DataTier.DATA_COLD_NODE_ROLE))); + } + + public void testLegacyDataRoleImpliesTieredDataRoles() { + DiscoveryNode.setAdditionalRoles( + Set.of(DataTier.DATA_CONTENT_NODE_ROLE, DataTier.DATA_HOT_NODE_ROLE, DataTier.DATA_WARM_NODE_ROLE, DataTier.DATA_COLD_NODE_ROLE) + ); + final Settings settings = Settings.builder().put(DiscoveryNodeRole.DATA_ROLE.legacySetting().getKey(), true).build(); + final DiscoveryNode node = DiscoveryNode.createLocal(settings, buildNewFakeTransportAddress(), randomAlphaOfLength(8)); + assertThat(node.getRoles(), hasItem(DataTier.DATA_CONTENT_NODE_ROLE)); + assertThat(node.getRoles(), hasItem(DataTier.DATA_HOT_NODE_ROLE)); + assertThat(node.getRoles(), hasItem(DataTier.DATA_WARM_NODE_ROLE)); + assertThat(node.getRoles(), hasItem(DataTier.DATA_COLD_NODE_ROLE)); + assertSettingDeprecationsAndWarnings(new Setting[]{DiscoveryNodeRole.DATA_ROLE.legacySetting()}); + } + + public void testDisablingLegacyDataRoleDisablesTieredDataRoles() { + DiscoveryNode.setAdditionalRoles( + Set.of(DataTier.DATA_CONTENT_NODE_ROLE, DataTier.DATA_HOT_NODE_ROLE, DataTier.DATA_WARM_NODE_ROLE, DataTier.DATA_COLD_NODE_ROLE) + ); + final Settings settings = Settings.builder().put(DiscoveryNodeRole.DATA_ROLE.legacySetting().getKey(), false).build(); + final DiscoveryNode node = DiscoveryNode.createLocal(settings, buildNewFakeTransportAddress(), randomAlphaOfLength(8)); + assertThat(node.getRoles(), not(hasItem(DataTier.DATA_CONTENT_NODE_ROLE))); + assertThat(node.getRoles(), not(hasItem(DataTier.DATA_HOT_NODE_ROLE))); + assertThat(node.getRoles(), not(hasItem(DataTier.DATA_WARM_NODE_ROLE))); + assertThat(node.getRoles(), not(hasItem(DataTier.DATA_COLD_NODE_ROLE))); + assertSettingDeprecationsAndWarnings(new Setting[]{DiscoveryNodeRole.DATA_ROLE.legacySetting()}); + } + private static DiscoveryNodes buildDiscoveryNodes() { int numNodes = randomIntBetween(3, 15); DiscoveryNodes.Builder discoBuilder = DiscoveryNodes.builder();