Skip to content

Commit

Permalink
Deprecate the 'local' parameter of /_cat/nodes (#50499)
Browse files Browse the repository at this point in the history
The cat nodes API performs a `ClusterStateAction` then a `NodesInfoAction`.
Today it accepts the `?local` parameter and passes this to the
`ClusterStateAction` but this parameter has no effect on the `NodesInfoAction`.
This is surprising, because `GET _cat/nodes?local` looks like it might be a
completely local call but in fact it still depends on every node in the
cluster.

This commit deprecates the `?local` parameter on this API so that it can be
removed in 8.0.

Relates #50088
  • Loading branch information
olegbonar authored and DaveCTurner committed Jan 2, 2020
1 parent 4ecabe4 commit 7539fbb
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 1 deletion.
10 changes: 10 additions & 0 deletions docs/reference/cat/nodes.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,16 @@ Number of suggest operations, such as `0`.
include::{docdir}/rest-api/common-parms.asciidoc[tag=help]

include::{docdir}/rest-api/common-parms.asciidoc[tag=local]
+
--
`local`::
(Optional, boolean) If `true`, the request computes the list of selected nodes
from the local cluster state. Defaults to `false`, which means the list of
selected nodes is computed from the cluster state on the master node. In either
case the coordinating node sends a request for further information to each
selected node. deprecated::[7.6,This parameter does not cause this API to act
locally. It will be removed in version 8.0.]
--

include::{docdir}/rest-api/common-parms.asciidoc[tag=master-timeout]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,11 @@
},
"local":{
"type":"boolean",
"description":"Return local information, do not retrieve the state from master node (default: false)"
"description":"Calculate the selected nodes using the local cluster state rather than the state from master node (default: false)",
"deprecated":{
"version":"7.6.0",
"description":"This parameter does not cause this API to act locally."
}
},
"master_timeout":{
"type":"time",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.elasticsearch.rest.action.cat;

import org.apache.logging.log4j.LogManager;
import org.elasticsearch.action.admin.cluster.node.info.NodeInfo;
import org.elasticsearch.action.admin.cluster.node.info.NodesInfoRequest;
import org.elasticsearch.action.admin.cluster.node.info.NodesInfoResponse;
Expand All @@ -33,6 +34,7 @@
import org.elasticsearch.cluster.node.DiscoveryNodes;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.Table;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.network.NetworkAddress;
import org.elasticsearch.common.transport.TransportAddress;
import org.elasticsearch.common.unit.ByteSizeValue;
Expand Down Expand Up @@ -67,6 +69,10 @@
import static org.elasticsearch.rest.RestRequest.Method.GET;

public class RestNodesAction extends AbstractCatAction {
private static final DeprecationLogger deprecationLogger = new DeprecationLogger(
LogManager.getLogger(RestNodesAction.class));
static final String LOCAL_DEPRECATED_MESSAGE = "Deprecated parameter [local] used. This parameter does not cause this API to act " +
"locally, and should not be used. It will be unsupported in version 8.0.";

public RestNodesAction(RestController controller) {
controller.registerHandler(GET, "/_cat/nodes", this);
Expand All @@ -86,6 +92,9 @@ protected void documentation(StringBuilder sb) {
public RestChannelConsumer doCatRequest(final RestRequest request, final NodeClient client) {
final ClusterStateRequest clusterStateRequest = new ClusterStateRequest();
clusterStateRequest.clear().nodes(true);
if (request.hasParam("local")) {
deprecationLogger.deprecated(LOCAL_DEPRECATED_MESSAGE);
}
clusterStateRequest.local(request.paramAsBoolean("local", clusterStateRequest.local()));
clusterStateRequest.masterNodeTimeout(request.paramAsTime("master_timeout", clusterStateRequest.masterNodeTimeout()));
final boolean fullId = request.paramAsBoolean("full_id", false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,16 @@
import org.elasticsearch.action.admin.cluster.node.info.NodesInfoResponse;
import org.elasticsearch.action.admin.cluster.node.stats.NodesStatsResponse;
import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse;
import org.elasticsearch.client.node.NodeClient;
import org.elasticsearch.cluster.ClusterName;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.cluster.node.DiscoveryNodes;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.rest.RestController;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.rest.FakeRestRequest;
import org.elasticsearch.threadpool.TestThreadPool;
import org.elasticsearch.usage.UsageService;
import org.junit.Before;

Expand Down Expand Up @@ -65,4 +68,16 @@ public void testBuildTableDoesNotThrowGivenNullNodeInfoAndStats() {

action.buildTable(false, new FakeRestRequest(), clusterStateResponse, nodesInfoResponse, nodesStatsResponse);
}

public void testCatNodesWithLocalDeprecationWarning() {
TestThreadPool threadPool = new TestThreadPool(RestNodesActionTests.class.getName());
NodeClient client = new NodeClient(Settings.EMPTY, threadPool);
FakeRestRequest request = new FakeRestRequest();
request.params().put("local", randomFrom("", "true", "false"));

action.doCatRequest(request, client);
assertWarnings(RestNodesAction.LOCAL_DEPRECATED_MESSAGE);

terminate(threadPool);
}
}

0 comments on commit 7539fbb

Please sign in to comment.