Skip to content

Introduce formal node ML role #45174

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Aug 6, 2019
Merged

Introduce formal node ML role #45174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0ce5add
Introduce formal node ML role
jasontedor Aug 3, 2019
564a5d2
Fix Docker test
jasontedor Aug 3, 2019
49e658f
Fix cat nodes test
jasontedor Aug 4, 2019
f2058d3
Remove accidental insert
jasontedor Aug 4, 2019
5d7d703
Merge branch 'master' into node-ml-role
jasontedor Aug 6, 2019
c7122e5
Feedback
jasontedor Aug 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions distribution/docker/src/test/resources/rest-api-spec/test/11_nodes.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@

- 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+ (-|[dmi]{1,3}) \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+ (-|[dilm]{1,4}) \s+ [-*x] \s+ (\S+\s?)+ \n)+ $/

- do:
cat.nodes:
v: true

- 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+ (-|[dmi]{1,3}) \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+ (-|[dilm]{1,4}) \s+ [-*x] \s+ (\S+\s?)+ \n)+ $/

- do:
cat.nodes:
Expand Down
2 changes: 1 addition & 1 deletion docs/reference/cat/nodes.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ ip heap.percent ram.percent cpu load_1m load_5m load_15m node.role master
--------------------------------------------------
// TESTRESPONSE[s/3.07/(\\d+\\.\\d+( \\d+\\.\\d+ (\\d+\\.\\d+)?)?)?/]
// TESTRESPONSE[s/65 99 42/\\d+ \\d+ \\d+/]
// TESTRESPONSE[s/[*]/[*]/ s/mJw06l1/.+/ non_json]
// TESTRESPONSE[s/dim/.+/ s/[*]/[*]/ s/mJw06l1/.+/ non_json]
Copy link

@droberts195 droberts195 Aug 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 101 of this file has the old mdi ordering.

Also, is it worth adding l to the docs on line 101 or is it deliberate to leave roles added by modules out of the table?

Copy link
Member Author

@jasontedor jasontedor Aug 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's leave tweaking that to a possible follow-up, as it would include adding voting-only nodes too.


The first few columns (`ip`, `heap.percent`, `ram.percent`, `cpu`, `load_*`) tell
you where your nodes live and give a quick picture of performance stats.
Expand Down
4 changes: 2 additions & 2 deletions docs/reference/getting-started.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,10 +247,10 @@ And the response:
[source,txt]
--------------------------------------------------
ip heap.percent ram.percent cpu load_1m load_5m load_15m node.role master name
127.0.0.1 10 5 5 4.46 dim * PB2SGZY
127.0.0.1 10 5 5 4.46 dim * PB2SGZY
--------------------------------------------------
// TESTRESPONSE[s/10 5 5 4.46/\\d+ \\d+ \\d+ (\\d+\\.\\d+)? (\\d+\\.\\d+)? (\\d+\.\\d+)?/]
// TESTRESPONSE[s/[*]/[*]/ s/PB2SGZY/.+/ non_json]
// TESTRESPONSE[s/dim/.+/ s/[*]/[*]/ s/PB2SGZY/.+/ non_json]

Here, we can see our one node named "PB2SGZY", which is the single node that is currently in our cluster.

Expand Down
8 changes: 4 additions & 4 deletions rest-api-spec/src/main/resources/rest-api-spec/test/cat.nodes/10_basic.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@

- 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+ (-|[dmi]{1,3}) \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+ (-|[dilm]{1,4}) \s+ [-*x] \s+ (\S+\s?)+ \n)+ $/

- do:
cat.nodes:
v: true

- 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+ (-|[dmi]{1,3}) \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+ (-|[dilm]{1,4}) \s+ [-*x] \s+ (\S+\s?)+ \n)+ $/

- do:
cat.nodes:
Expand Down
17 changes: 17 additions & 0 deletions x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.metadata.IndexTemplateMetaData;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.cluster.node.DiscoveryNodeRole;
import org.elasticsearch.cluster.node.DiscoveryNodes;
import org.elasticsearch.cluster.routing.UnassignedInfo;
import org.elasticsearch.cluster.service.ClusterService;
Expand Down Expand Up @@ -280,6 +281,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Supplier;
import java.util.function.UnaryOperator;

Expand All @@ -300,6 +302,21 @@ public class MachineLearning extends Plugin implements ActionPlugin, AnalysisPlu

public static final Setting<Boolean> ML_ENABLED =
Setting.boolSetting("node.ml", XPackSettings.MACHINE_LEARNING_ENABLED, Property.NodeScope);

public static final DiscoveryNodeRole ML_ROLE = new DiscoveryNodeRole("ml", "l") {

@Override
protected Setting<Boolean> roleSetting() {
return ML_ENABLED;
}

Copy link
Contributor

@przemekwitek przemekwitek Aug 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Excessive blank line

Copy link
Member Author

@jasontedor jasontedor Aug 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have very little in the way of style agreements, mostly what is enforced by checkstyle and a few loosely-enforced guidelines. We've discussed over the years adding an auto-formatter to avoid style issues once and for all, but we've not had the time to invest in it. Until then, we've mostly left style up to individual developers, although for particularly contentious issues we can have discussions amongst the teams that work in this codebase to see if we want to formalize a rule. Regarding this specific one, our codebase is inconsistently full of white space or not between the opening brace of a class and its body, and the closing brace of a class and its body. My preference is to have this whitespace.

Copy link
Contributor

@przemekwitek przemekwitek Aug 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation. +1 for the idea of automated formatting some day...

};

@Override
public Set<DiscoveryNodeRole> getRoles() {
return Collections.singleton(ML_ROLE);
}

// This is not used in v7 and higher, but users are still prevented from setting it directly to avoid confusion
private static final String PRE_V7_ML_ENABLED_NODE_ATTR = "ml.enabled";
public static final String MAX_OPEN_JOBS_NODE_ATTR = "ml.max_open_jobs";
Expand Down
4 changes: 0 additions & 4 deletions x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/JobNodeSelectorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,6 @@ public void testNodeNameAndMlAttributes() {

node = new DiscoveryNode(null, "_node_id1", ta, attributes, Collections.emptySet(), Version.CURRENT);
assertEquals("{_node_id1}{ml.machine_memory=5}", JobNodeSelector.nodeNameAndMlAttributes(node));

attributes.put("node.ml", "true");
node = new DiscoveryNode("_node_name1", "_node_id1", ta, attributes, Collections.emptySet(), Version.CURRENT);
assertEquals("{_node_name1}{ml.machine_memory=5}{node.ml=true}", JobNodeSelector.nodeNameAndMlAttributes(node));
}

public void testSelectLeastLoadedMlNode_byCount() {
Expand Down