From 8cce9781a70feaa3ab3ecc8f3ec4efe1acf7c0e6 Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 6 Jan 2020 14:12:51 +0000 Subject: [PATCH] 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 --- docs/reference/cat/nodes.asciidoc | 12 ------------ docs/reference/migration/migrate_8_0.asciidoc | 2 ++ .../migration/migrate_8_0/api.asciidoc | 19 +++++++++++++++++++ .../rest-api-spec/api/cat.nodes.json | 8 -------- .../rest/action/cat/RestNodesAction.java | 12 +++--------- .../rest/action/cat/RestNodesActionTests.java | 12 ++++++------ 6 files changed, 30 insertions(+), 35 deletions(-) create mode 100644 docs/reference/migration/migrate_8_0/api.asciidoc diff --git a/docs/reference/cat/nodes.asciidoc b/docs/reference/cat/nodes.asciidoc index 506bc6f5c2721..a5aff2f8daf2d 100644 --- a/docs/reference/cat/nodes.asciidoc +++ b/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] diff --git a/docs/reference/migration/migrate_8_0.asciidoc b/docs/reference/migration/migrate_8_0.asciidoc index eff10899a2e03..9e4532b87ccbe 100644 --- a/docs/reference/migration/migrate_8_0.asciidoc +++ b/docs/reference/migration/migrate_8_0.asciidoc @@ -30,6 +30,7 @@ coming[8.0.0] * <> * <> * <> +* <> //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[] diff --git a/docs/reference/migration/migrate_8_0/api.asciidoc b/docs/reference/migration/migrate_8_0/api.asciidoc new file mode 100644 index 0000000000000..f3293a85d5621 --- /dev/null +++ b/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. diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/cat.nodes.json b/rest-api-spec/src/main/resources/rest-api-spec/api/cat.nodes.json index 753a6aab1d5f3..20de099e2e9cb 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/cat.nodes.json +++ b/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" diff --git a/server/src/main/java/org/elasticsearch/rest/action/cat/RestNodesAction.java b/server/src/main/java/org/elasticsearch/rest/action/cat/RestNodesAction.java index 849fbdac49224..ac26c43385004 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/cat/RestNodesAction.java +++ b/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.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 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 @@ 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); + 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(channel) { diff --git a/server/src/test/java/org/elasticsearch/rest/action/cat/RestNodesActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/cat/RestNodesActionTests.java index 05b14d6b1cfc4..38c4837468362 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/cat/RestNodesActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/cat/RestNodesActionTests.java @@ -40,6 +40,7 @@ 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 void testBuildTableDoesNotThrowGivenNullNodeInfoAndStats() { 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); } }