From a809e4ad7b7f89856859eb52cdf32e52dec25037 Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Tue, 10 Nov 2020 07:46:07 -0600 Subject: [PATCH 1/2] Remove the deprecated local parameter for _cat/shards --- docs/reference/cat/shards.asciidoc | 5 ----- .../reference/migration/migrate_8_0/api.asciidoc | 16 ++++++++++++++++ .../resources/rest-api-spec/api/cat.shards.json | 8 -------- .../rest/action/cat/RestShardsAction.java | 10 ++++------ .../rest/action/cat/RestShardsActionTests.java | 10 ++++++---- 5 files changed, 26 insertions(+), 23 deletions(-) diff --git a/docs/reference/cat/shards.asciidoc b/docs/reference/cat/shards.asciidoc index 7a69cd4da5cce..12f046e26d8e9 100644 --- a/docs/reference/cat/shards.asciidoc +++ b/docs/reference/cat/shards.asciidoc @@ -273,11 +273,6 @@ Reason the shard is unassigned. Returned values are: include::{es-repo-dir}/rest-api/common-parms.asciidoc[tag=help] -`local`:: -(Optional, boolean) -+ -deprecated::[7.11.0,"This parameter does not affect the request. It will be removed in a future release."] - include::{es-repo-dir}/rest-api/common-parms.asciidoc[tag=master-timeout] include::{es-repo-dir}/rest-api/common-parms.asciidoc[tag=cat-s] diff --git a/docs/reference/migration/migrate_8_0/api.asciidoc b/docs/reference/migration/migrate_8_0/api.asciidoc index d8dc474ac873c..6611a680b6f13 100644 --- a/docs/reference/migration/migrate_8_0/api.asciidoc +++ b/docs/reference/migration/migrate_8_0/api.asciidoc @@ -24,6 +24,22 @@ Discontinue use of the `?local` query parameter. {ref}/cat-nodes.html[cat node API] requests that include this parameter will return an error. ==== +.The cat shard API's `local` query parameter has been removed. +[%collapsible] +==== +*Details* + +The `?local` parameter to the `GET _cat/shards` 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. + +*Impact* + +Discontinue use of the `?local` query parameter. {ref}/cat-shards.html[cat shards +API] requests that include this parameter will return an error. +==== + .The get field mapping API's `local` query parameter has been removed. [%collapsible] ==== diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/cat.shards.json b/rest-api-spec/src/main/resources/rest-api-spec/api/cat.shards.json index 576cba3fdcd49..565be0e730d33 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/cat.shards.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/cat.shards.json @@ -49,14 +49,6 @@ "pb" ] }, - "local":{ - "type":"boolean", - "description":"Return local information, do not retrieve the state from master node (default: false)", - "deprecated":{ - "version":"7.11.0", - "description":"This parameter does not affect the request. It will be removed in a future release." - } - }, "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/RestShardsAction.java b/server/src/main/java/org/elasticsearch/rest/action/cat/RestShardsAction.java index 7b141b05c0fb9..ed265ad2f2209 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/cat/RestShardsAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/cat/RestShardsAction.java @@ -19,6 +19,7 @@ package org.elasticsearch.rest.action.cat; +import org.elasticsearch.Version; import org.elasticsearch.action.admin.cluster.state.ClusterStateRequest; import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse; import org.elasticsearch.action.admin.indices.stats.CommonStats; @@ -30,7 +31,6 @@ import org.elasticsearch.cluster.routing.UnassignedInfo; import org.elasticsearch.common.Strings; import org.elasticsearch.common.Table; -import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.index.bulk.stats.BulkStats; import org.elasticsearch.index.cache.query.QueryCacheStats; @@ -61,8 +61,6 @@ import static org.elasticsearch.rest.RestRequest.Method.GET; public class RestShardsAction extends AbstractCatAction { - private static final DeprecationLogger DEPRECATION_LOGGER = DeprecationLogger.getLogger(RestShardsAction.class); - static final String LOCAL_DEPRECATED_MESSAGE = "The parameter [local] is deprecated and will be removed in a future release."; @Override public List routes() { @@ -90,10 +88,10 @@ protected void documentation(StringBuilder sb) { public RestChannelConsumer doCatRequest(final RestRequest request, final NodeClient client) { final String[] indices = Strings.splitStringByCommaToArray(request.param("index")); final ClusterStateRequest clusterStateRequest = new ClusterStateRequest(); - if (request.hasParam("local")) { - DEPRECATION_LOGGER.deprecate("local", LOCAL_DEPRECATED_MESSAGE); + // needed only in v8 to catch breaking usages and can be removed in v9 + if (request.hasParam("local") && Version.CURRENT.major == Version.V_7_0_0.major + 1) { + throw new IllegalArgumentException("parameter [local] is not supported"); } - clusterStateRequest.local(request.paramAsBoolean("local", clusterStateRequest.local())); clusterStateRequest.masterNodeTimeout(request.paramAsTime("master_timeout", clusterStateRequest.masterNodeTimeout())); clusterStateRequest.clear().nodes(true).routingTable(true).indices(indices); return channel -> client.admin().cluster().state(clusterStateRequest, new RestActionListener(channel) { diff --git a/server/src/test/java/org/elasticsearch/rest/action/cat/RestShardsActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/cat/RestShardsActionTests.java index bca0412ce8228..0676752561855 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/cat/RestShardsActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/cat/RestShardsActionTests.java @@ -49,6 +49,7 @@ import java.util.Map; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -130,14 +131,15 @@ public void testBuildTable() { } } - public void testCatNodesWithLocalDeprecationWarning() { + public void testCatShardsRejectsLocalParameter() { + assumeTrue("test is needed only in v8 and 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")); + request.params().put("local", randomFrom("", "true", "false", randomAlphaOfLength(10))); - action.doCatRequest(request, client); - assertWarnings(RestShardsAction.LOCAL_DEPRECATED_MESSAGE); + assertThat(expectThrows(IllegalArgumentException.class, () -> action.doCatRequest(request, client)).getMessage(), + is("parameter [local] is not supported")); terminate(threadPool); } From 26384068e89f06a84183e98d5e9ead4cf5628a18 Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Wed, 11 Nov 2020 11:04:26 -0600 Subject: [PATCH 2/2] remove redundant check --- .../rest/action/cat/RestShardsAction.java | 5 ---- .../action/cat/RestShardsActionTests.java | 25 ------------------- 2 files changed, 30 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/action/cat/RestShardsAction.java b/server/src/main/java/org/elasticsearch/rest/action/cat/RestShardsAction.java index ed265ad2f2209..2e882817ab0bb 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/cat/RestShardsAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/cat/RestShardsAction.java @@ -19,7 +19,6 @@ package org.elasticsearch.rest.action.cat; -import org.elasticsearch.Version; import org.elasticsearch.action.admin.cluster.state.ClusterStateRequest; import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse; import org.elasticsearch.action.admin.indices.stats.CommonStats; @@ -88,10 +87,6 @@ protected void documentation(StringBuilder sb) { public RestChannelConsumer doCatRequest(final RestRequest request, final NodeClient client) { final String[] indices = Strings.splitStringByCommaToArray(request.param("index")); final ClusterStateRequest clusterStateRequest = new ClusterStateRequest(); - // needed only in v8 to catch breaking usages and can be removed in v9 - if (request.hasParam("local") && Version.CURRENT.major == Version.V_7_0_0.major + 1) { - throw new IllegalArgumentException("parameter [local] is not supported"); - } clusterStateRequest.masterNodeTimeout(request.paramAsTime("master_timeout", clusterStateRequest.masterNodeTimeout())); clusterStateRequest.clear().nodes(true).routingTable(true).indices(indices); return channel -> client.admin().cluster().state(clusterStateRequest, new RestActionListener(channel) { diff --git a/server/src/test/java/org/elasticsearch/rest/action/cat/RestShardsActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/cat/RestShardsActionTests.java index 0676752561855..f661521376cf5 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/cat/RestShardsActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/cat/RestShardsActionTests.java @@ -25,7 +25,6 @@ import org.elasticsearch.action.admin.indices.stats.IndexStats; import org.elasticsearch.action.admin.indices.stats.IndicesStatsResponse; import org.elasticsearch.action.admin.indices.stats.ShardStats; -import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodes; @@ -34,12 +33,9 @@ import org.elasticsearch.cluster.routing.ShardRoutingState; import org.elasticsearch.cluster.routing.TestShardRouting; import org.elasticsearch.common.Table; -import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.shard.ShardPath; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.rest.FakeRestRequest; -import org.elasticsearch.threadpool.TestThreadPool; -import org.junit.Before; import java.nio.file.Path; import java.util.ArrayList; @@ -49,19 +45,11 @@ import java.util.Map; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.is; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; public class RestShardsActionTests extends ESTestCase { - private RestShardsAction action; - - @Before - public void setUpAction() { - action = new RestShardsAction(); - } - public void testBuildTable() { final int numShards = randomIntBetween(1, 5); DiscoveryNode localNode = new DiscoveryNode("local", buildNewFakeTransportAddress(), Version.CURRENT); @@ -130,17 +118,4 @@ public void testBuildTable() { assertThat(row.get(70).value, equalTo(shardStats.getStatePath())); } } - - public void testCatShardsRejectsLocalParameter() { - assumeTrue("test is needed only in v8 and 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", randomAlphaOfLength(10))); - - assertThat(expectThrows(IllegalArgumentException.class, () -> action.doCatRequest(request, client)).getMessage(), - is("parameter [local] is not supported")); - - terminate(threadPool); - } }