Skip to content

Commit

Permalink
Use set-based interface for NodesStatsRequest (#53637) (#54141)
Browse files Browse the repository at this point in the history 
The NodesStatsRequest class uses a set of strings for its internal
serialization. This commit updates the class's interface so that we
no longer use hard-coded getters and setters, but rather
methods that add strings directly. For example, the old way of
adding "os" metrics to a request would be to call request.os(true).
The new way of doing this is to call request.addMetric("os").

For the time being, the canonical list of metrics is an enum in
NodesStatsRequest. This will eventually be replaced with something
pluggable.
  • Loading branch information
@williamrandolph
williamrandolph authored Mar 26, 2020
1 parent 13368aa commit 14204f8
Show file tree
Hide file tree
Showing 13 changed files with 189 additions and 270 deletions.
4 changes: 2 additions & 2 deletions build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,8 @@ task verifyVersions {
* after the backport of the backcompat code is complete.
*/

boolean bwc_tests_enabled = true
final String bwc_tests_disabled_issue = "" /* place a PR link here when committing bwc changes */
boolean bwc_tests_enabled = false
final String bwc_tests_disabled_issue = "https://github.com/elastic/elasticsearch/pull/54141" /* place a PR link here when committing bwc changes */
if (bwc_tests_enabled == false) {
if (bwc_tests_disabled_issue.isEmpty()) {
throw new GradleException("bwc_tests_disabled_issue must be set when bwc_tests_enabled == false")
Expand Down
222 changes: 57 additions & 165 deletions ...er/src/main/java/org/elasticsearch/action/admin/cluster/node/stats/NodesStatsRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,12 @@
import org.elasticsearch.action.support.nodes.BaseNodesRequest;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;

import java.io.IOException;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;
import java.util.SortedSet;
import java.util.TreeSet;
import java.util.stream.Collectors;

/**
Expand All @@ -49,19 +50,19 @@ public NodesStatsRequest(StreamInput in) throws IOException {
indices = new CommonStatsFlags(in);
requestedMetrics.clear();
if (in.getVersion().before(Version.V_7_7_0)) {
addOrRemoveMetric(in.readBoolean(), Metric.OS.metricName());
addOrRemoveMetric(in.readBoolean(), Metric.PROCESS.metricName());
addOrRemoveMetric(in.readBoolean(), Metric.JVM.metricName());
addOrRemoveMetric(in.readBoolean(), Metric.THREAD_POOL.metricName());
addOrRemoveMetric(in.readBoolean(), Metric.FS.metricName());
addOrRemoveMetric(in.readBoolean(), Metric.TRANSPORT.metricName());
addOrRemoveMetric(in.readBoolean(), Metric.HTTP.metricName());
addOrRemoveMetric(in.readBoolean(), Metric.BREAKER.metricName());
addOrRemoveMetric(in.readBoolean(), Metric.SCRIPT.metricName());
addOrRemoveMetric(in.readBoolean(), Metric.DISCOVERY.metricName());
addOrRemoveMetric(in.readBoolean(), Metric.INGEST.metricName());
optionallyAddMetric(in.readBoolean(), Metric.OS.metricName());
optionallyAddMetric(in.readBoolean(), Metric.PROCESS.metricName());
optionallyAddMetric(in.readBoolean(), Metric.JVM.metricName());
optionallyAddMetric(in.readBoolean(), Metric.THREAD_POOL.metricName());
optionallyAddMetric(in.readBoolean(), Metric.FS.metricName());
optionallyAddMetric(in.readBoolean(), Metric.TRANSPORT.metricName());
optionallyAddMetric(in.readBoolean(), Metric.HTTP.metricName());
optionallyAddMetric(in.readBoolean(), Metric.BREAKER.metricName());
optionallyAddMetric(in.readBoolean(), Metric.SCRIPT.metricName());
optionallyAddMetric(in.readBoolean(), Metric.DISCOVERY.metricName());
optionallyAddMetric(in.readBoolean(), Metric.INGEST.metricName());
if (in.getVersion().onOrAfter(Version.V_6_1_0)) {
addOrRemoveMetric(in.readBoolean(), Metric.ADAPTIVE_SELECTION.metricName());
optionallyAddMetric(in.readBoolean(), Metric.ADAPTIVE_SELECTION.metricName());
}
} else {
requestedMetrics.addAll(in.readStringList());
Expand Down Expand Up @@ -94,10 +95,21 @@ public NodesStatsRequest clear() {
return this;
}

/**
* Get indices. Handles separately from other metrics because it may or
* may not have submetrics.
* @return flags indicating which indices stats to return
*/
public CommonStatsFlags indices() {
return indices;
}

/**
* Set indices. Handles separately from other metrics because it may or
* may not involve submetrics.
* @param indices flags indicating which indices stats to return
* @return This object, for request chaining.
*/
public NodesStatsRequest indices(CommonStatsFlags indices) {
this.indices = indices;
return this;
Expand All @@ -116,178 +128,58 @@ public NodesStatsRequest indices(boolean indices) {
}

/**
* Should the node OS be returned.
*/
public boolean os() {
return Metric.OS.containedIn(requestedMetrics);
}

/**
* Should the node OS be returned.
*/
public NodesStatsRequest os(boolean os) {
addOrRemoveMetric(os, Metric.OS.metricName());
return this;
}

/**
* Should the node Process be returned.
*/
public boolean process() {
return Metric.PROCESS.containedIn(requestedMetrics);
}

/**
* Should the node Process be returned.
*/
public NodesStatsRequest process(boolean process) {
addOrRemoveMetric(process, Metric.PROCESS.metricName());
return this;
}

/**
* Should the node JVM be returned.
*/
public boolean jvm() {
return Metric.JVM.containedIn(requestedMetrics);
}

/**
* Should the node JVM be returned.
*/
public NodesStatsRequest jvm(boolean jvm) {
addOrRemoveMetric(jvm, Metric.JVM.metricName());
return this;
}

/**
* Should the node Thread Pool be returned.
*/
public boolean threadPool() {
return Metric.THREAD_POOL.containedIn(requestedMetrics);
}

/**
* Should the node Thread Pool be returned.
*/
public NodesStatsRequest threadPool(boolean threadPool) {
addOrRemoveMetric(threadPool, Metric.THREAD_POOL.metricName());
return this;
}

/**
* Should the node file system stats be returned.
* Get the names of requested metrics, excluding indices, which are
* handled separately.
*/
public boolean fs() {
return Metric.FS.containedIn(requestedMetrics);
public Set<String> requestedMetrics() {
return new HashSet<>(requestedMetrics);
}

/**
* Should the node file system stats be returned.
* Add metric
*/
public NodesStatsRequest fs(boolean fs) {
addOrRemoveMetric(fs, Metric.FS.metricName());
return this;
}

/**
* Should the node Transport be returned.
*/
public boolean transport() {
return Metric.TRANSPORT.containedIn(requestedMetrics);
}

/**
* Should the node Transport be returned.
*/
public NodesStatsRequest transport(boolean transport) {
addOrRemoveMetric(transport, Metric.TRANSPORT.metricName());
return this;
}

/**
* Should the node HTTP be returned.
*/
public boolean http() {
return Metric.HTTP.containedIn(requestedMetrics);
}

/**
* Should the node HTTP be returned.
*/
public NodesStatsRequest http(boolean http) {
addOrRemoveMetric(http, Metric.HTTP.metricName());
return this;
}

public boolean breaker() {
return Metric.BREAKER.containedIn(requestedMetrics);
}

/**
* Should the node's circuit breaker stats be returned.
*/
public NodesStatsRequest breaker(boolean breaker) {
addOrRemoveMetric(breaker, Metric.BREAKER.metricName());
return this;
}

public boolean script() {
return Metric.SCRIPT.containedIn(requestedMetrics);
}

public NodesStatsRequest script(boolean script) {
addOrRemoveMetric(script, Metric.SCRIPT.metricName());
return this;
}


public boolean discovery() {
return Metric.DISCOVERY.containedIn(requestedMetrics);
}

/**
* Should the node's discovery stats be returned.
*/
public NodesStatsRequest discovery(boolean discovery) {
addOrRemoveMetric(discovery, Metric.DISCOVERY.metricName());
public NodesStatsRequest addMetric(String metric) {
if (Metric.allMetrics().contains(metric) == false) {
throw new IllegalStateException("Used an illegal metric: " + metric);
}
requestedMetrics.add(metric);
return this;
}

public boolean ingest() {
return Metric.INGEST.containedIn(requestedMetrics);
}

/**
* Should ingest statistics be returned.
* Add an array of metric names
*/
public NodesStatsRequest ingest(boolean ingest) {
addOrRemoveMetric(ingest, Metric.INGEST.metricName());
public NodesStatsRequest addMetrics(String... metrics) {
// use sorted set for reliable ordering in error messages
SortedSet<String> metricsSet = new TreeSet<>(Arrays.asList(metrics));
if (Metric.allMetrics().containsAll(metricsSet) == false) {
metricsSet.removeAll(Metric.allMetrics());
String plural = metricsSet.size() == 1 ? "" : "s";
throw new IllegalStateException("Used illegal metric" + plural + ": " + metricsSet);
}
requestedMetrics.addAll(metricsSet);
return this;
}

public boolean adaptiveSelection() {
return Metric.ADAPTIVE_SELECTION.containedIn(requestedMetrics);
}

/**
* Should adaptiveSelection statistics be returned.
* Remove metric
*/
public NodesStatsRequest adaptiveSelection(boolean adaptiveSelection) {
addOrRemoveMetric(adaptiveSelection, Metric.ADAPTIVE_SELECTION.metricName());
public NodesStatsRequest removeMetric(String metric) {
if (Metric.allMetrics().contains(metric) == false) {
throw new IllegalStateException("Used an illegal metric: " + metric);
}
requestedMetrics.remove(metric);
return this;
}

/**
* Helper method for adding and removing metrics.
* Helper method for adding metrics during deserialization.
* @param includeMetric Whether or not to include a metric.
* @param metricName Name of the metric to include or remove.
* @param metricName Name of the metric to add.
*/
private void addOrRemoveMetric(boolean includeMetric, String metricName) {
private void optionallyAddMetric(boolean includeMetric, String metricName) {
if (includeMetric) {
requestedMetrics.add(metricName);
} else {
requestedMetrics.remove(metricName);
}
}

Expand Down Expand Up @@ -319,7 +211,7 @@ public void writeTo(StreamOutput out) throws IOException {
* An enumeration of the "core" sections of metrics that may be requested
* from the nodes stats endpoint. Eventually this list will be pluggable.
*/
private enum Metric {
public enum Metric {
OS("os"),
PROCESS("process"),
JVM("jvm"),
Expand All @@ -331,15 +223,15 @@ private enum Metric {
SCRIPT("script"),
DISCOVERY("discovery"),
INGEST("ingest"),
ADAPTIVE_SELECTION("adaptiveSelection");
ADAPTIVE_SELECTION("adaptive_selection");

private String metricName;

Metric(String name) {
this.metricName = name;
}

String metricName() {
public String metricName() {
return this.metricName;
}

Expand Down
Loading

0 comments on commit 14204f8

Please sign in to comment.