Browse Source

Deprecate the 'local' parameter of /_cat/nodes (#50499)

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
Oleg 5 years ago
parent
commit
1c2492136d

+ 10 - 0
docs/reference/cat/nodes.asciidoc

@@ -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::[8.0,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]
 

+ 5 - 1
rest-api-spec/src/main/resources/rest-api-spec/api/cat.nodes.json

@@ -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":"8.0.0",
+          "description":"This parameter does not cause this API to act locally."
+        }
       },
       "master_timeout":{
         "type":"time",

+ 9 - 0
server/src/main/java/org/elasticsearch/rest/action/cat/RestNodesAction.java

@@ -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;
@@ -33,6 +34,7 @@ import org.elasticsearch.cluster.node.DiscoveryNodeRole;
 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;
@@ -67,6 +69,10 @@ import java.util.stream.Collectors;
 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);
@@ -86,6 +92,9 @@ public class RestNodesAction extends AbstractCatAction {
     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);

+ 15 - 0
server/src/test/java/org/elasticsearch/rest/action/cat/RestNodesActionTests.java

@@ -23,13 +23,16 @@ import org.elasticsearch.Version;
 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;
 
@@ -65,4 +68,16 @@ public class RestNodesActionTests extends ESTestCase {
 
         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);
+    }
 }