Skip to content

Commit

Permalink
Remove the 'local' parameter of /_cat/nodes (elastic#50594)
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 parameter was deprecated in 7.x in elastic#50499 and this commit removes it.

Relates elastic#50088
  • Loading branch information
DaveCTurner authored and SivagurunathanV committed Jan 21, 2020
1 parent 8a45624 commit 8cce978
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 35 deletions.
12 changes: 0 additions & 12 deletions docs/reference/cat/nodes.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
2 changes: 2 additions & 0 deletions docs/reference/migration/migrate_8_0.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 changes: 19 additions & 0 deletions docs/reference/migration/migrate_8_0/api.asciidoc
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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<ClusterStateResponse>(channel) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
}
}

0 comments on commit 8cce978

Please sign in to comment.