From 04a9845a49d76e9548cc43e59dab6a60477baffd 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 acda1a3dced..af1ed146c41 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 d32e7cb5f1c..684dee05021 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 4c28f0bec5d..16bf099081c 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 bc12b8f0431..d99a8e1deeb 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 368d82da203..8baed3ea4dd 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();