Browse Source

Remove the 'local' parameter of /_cat/nodes (#50594)

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 parameter was deprecated in 7.x in #50499 and this commit removes it.

Relates #50088
David Turner 5 years ago
parent
commit
8c6f821c75

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

@@ -285,18 +285,6 @@ 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]
 
 include::{docdir}/rest-api/common-parms.asciidoc[tag=cat-s]

+ 2 - 0
docs/reference/migration/migrate_8_0.asciidoc

@@ -30,6 +30,7 @@ coming[8.0.0]
 * <<breaking_80_search_changes>>
 * <<breaking_80_settings_changes>>
 * <<breaking_80_indices_changes>>
+* <<breaking_80_api_changes>>
 
 //NOTE: The notable-breaking-changes tagged regions are re-used in the
 //Installation and Upgrade Guide
@@ -81,3 +82,4 @@ include::migrate_8_0/reindex.asciidoc[]
 include::migrate_8_0/search.asciidoc[]
 include::migrate_8_0/settings.asciidoc[]
 include::migrate_8_0/indices.asciidoc[]
+include::migrate_8_0/api.asciidoc[]

+ 19 - 0
docs/reference/migration/migrate_8_0/api.asciidoc

@@ -0,0 +1,19 @@
+[float]
+[[breaking_80_api_changes]]
+=== REST API changes
+
+//NOTE: The notable-breaking-changes tagged regions are re-used in the
+//Installation and Upgrade Guide
+//tag::notable-breaking-changes[]
+
+// end::notable-breaking-changes[]
+
+[float]
+==== Deprecated `?local` parameter removed from `GET _cat/nodes` API
+
+The `?local` parameter to the `GET _cat/nodes` API was deprecated in 7.x and is
+rejected in 8.0. This parameter caused the API to use the local cluster state
+to determine the nodes returned by the API rather than the cluster state from
+the master, but this API requests information from each selected node
+regardless of the `?local` parameter which means this API does not run in a
+fully node-local fashion.

+ 0 - 8
rest-api-spec/src/main/resources/rest-api-spec/api/cat.nodes.json

@@ -41,14 +41,6 @@
         "type":"boolean",
         "description":"Return the full node ID instead of the shortened version (default: false)"
       },
-      "local":{
-        "type":"boolean",
-        "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",
         "description":"Explicit operation timeout for connection to master node"

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

@@ -19,7 +19,7 @@
 
 package org.elasticsearch.rest.action.cat;
 
-import org.apache.logging.log4j.LogManager;
+import org.elasticsearch.Version;
 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;
@@ -34,7 +34,6 @@ 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;
@@ -69,10 +68,6 @@ 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);
@@ -92,10 +87,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);
+        if (request.hasParam("local") && Version.CURRENT.major == Version.V_7_0_0.major + 1) { // only needed in v8 to catch breaking usages
+            throw new IllegalArgumentException("parameter [local] is not supported");
         }
-        clusterStateRequest.local(request.paramAsBoolean("local", clusterStateRequest.local()));
         clusterStateRequest.masterNodeTimeout(request.paramAsTime("master_timeout", clusterStateRequest.masterNodeTimeout()));
         final boolean fullId = request.paramAsBoolean("full_id", false);
         return channel -> client.admin().cluster().state(clusterStateRequest, new RestActionListener<ClusterStateResponse>(channel) {

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

@@ -40,6 +40,7 @@ import java.util.Collections;
 
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.emptySet;
+import static org.hamcrest.Matchers.is;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -69,15 +70,14 @@ public class RestNodesActionTests extends ESTestCase {
         action.buildTable(false, new FakeRestRequest(), clusterStateResponse, nodesInfoResponse, nodesStatsResponse);
     }
 
-    public void testCatNodesWithLocalDeprecationWarning() {
+    public void testCatNodesRejectsLocalParameter() {
+        assumeTrue("test is only needed in v8, can be removed in v9", Version.CURRENT.major == Version.V_7_0_0.major + 1);
         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);
-
+        request.params().put("local", randomFrom("", "true", "false", randomAlphaOfLength(10)));
+        assertThat(expectThrows(IllegalArgumentException.class, () -> action.doCatRequest(request, client)).getMessage(),
+            is("parameter [local] is not supported"));
         terminate(threadPool);
     }
 }