Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[2.x] Add request parameter 'cluster_manager_timeout' and deprecate 'master_timeout' in Cluster APIs (#2658) #2754

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,15 @@
},
"master_timeout":{
"type":"time",
"description":"Explicit operation timeout for connection to master node"
"description":"Explicit operation timeout for connection to master node",
"deprecated":{
"version":"2.0.0",
"description":"To promote inclusive language, use 'cluster_manager_timeout' instead."
}
},
"cluster_manager_timeout":{
"type":"time",
"description":"Explicit operation timeout for connection to cluster-manager node"
},
"timeout":{
"type":"time",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,15 @@
},
"master_timeout":{
"type":"time",
"description":"Explicit operation timeout for connection to master node"
"description":"Explicit operation timeout for connection to master node",
"deprecated":{
"version":"2.0.0",
"description":"To promote inclusive language, use 'cluster_manager_timeout' instead."
}
},
"cluster_manager_timeout":{
"type":"time",
"description":"Explicit operation timeout for connection to cluster-manager node"
},
"timeout":{
"type":"time",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,15 @@
},
"master_timeout":{
"type":"time",
"description":"Specify timeout for connection to master"
"description":"Specify timeout for connection to master",
"deprecated":{
"version":"2.0.0",
"description":"To promote inclusive language, use 'cluster_manager_timeout' instead."
}
},
"cluster_manager_timeout":{
"type":"time",
"description":"Specify timeout for connection to cluster-manager node"
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,15 @@
},
"master_timeout":{
"type":"time",
"description":"Explicit operation timeout for connection to master node"
"description":"Explicit operation timeout for connection to master node",
"deprecated":{
"version":"2.0.0",
"description":"To promote inclusive language, use 'cluster_manager_timeout' instead."
}
},
"cluster_manager_timeout":{
"type":"time",
"description":"Explicit operation timeout for connection to cluster-manager node"
},
"timeout":{
"type":"time",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,15 @@
},
"master_timeout":{
"type":"time",
"description":"Explicit operation timeout for connection to master node"
"description":"Explicit operation timeout for connection to master node",
"deprecated":{
"version":"2.0.0",
"description":"To promote inclusive language, use 'cluster_manager_timeout' instead."
}
},
"cluster_manager_timeout":{
"type":"time",
"description":"Explicit operation timeout for connection to cluster-manager node"
},
"timeout":{
"type":"time",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,15 @@
},
"master_timeout":{
"type":"time",
"description":"Specify timeout for connection to master"
"description":"Specify timeout for connection to master",
"deprecated":{
"version":"2.0.0",
"description":"To promote inclusive language, use 'cluster_manager_timeout' instead."
}
},
"cluster_manager_timeout":{
"type":"time",
"description":"Specify timeout for connection to cluster-manager node"
},
"flat_settings":{
"type":"boolean",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.opensearch.client.Requests;
import org.opensearch.client.node.NodeClient;
import org.opensearch.cluster.ClusterState;
import org.opensearch.common.logging.DeprecationLogger;
import org.opensearch.common.settings.ClusterSettings;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.settings.SettingsFilter;
Expand All @@ -59,6 +60,8 @@

public class RestClusterGetSettingsAction extends BaseRestHandler {

private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestClusterGetSettingsAction.class);

private final Settings settings;
private final ClusterSettings clusterSettings;
private final SettingsFilter settingsFilter;
Expand All @@ -84,7 +87,8 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
ClusterStateRequest clusterStateRequest = Requests.clusterStateRequest().routingTable(false).nodes(false);
final boolean renderDefaults = request.paramAsBoolean("include_defaults", false);
clusterStateRequest.local(request.paramAsBoolean("local", clusterStateRequest.local()));
clusterStateRequest.masterNodeTimeout(request.paramAsTime("master_timeout", clusterStateRequest.masterNodeTimeout()));
clusterStateRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", clusterStateRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(clusterStateRequest, request, deprecationLogger, getName());
return channel -> client.admin().cluster().state(clusterStateRequest, new RestBuilderListener<ClusterStateResponse>(channel) {
@Override
public RestResponse buildResponse(ClusterStateResponse response, XContentBuilder builder) throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import org.opensearch.cluster.health.ClusterHealthStatus;
import org.opensearch.common.Priority;
import org.opensearch.common.Strings;
import org.opensearch.common.logging.DeprecationLogger;
import org.opensearch.rest.BaseRestHandler;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.action.RestStatusToXContentListener;
Expand All @@ -56,6 +57,8 @@

public class RestClusterHealthAction extends BaseRestHandler {

private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestClusterHealthAction.class);

@Override
public List<Route> routes() {
return unmodifiableList(asList(new Route(GET, "/_cluster/health"), new Route(GET, "/_cluster/health/{index}")));
Expand All @@ -81,7 +84,8 @@ public static ClusterHealthRequest fromRequest(final RestRequest request) {
final ClusterHealthRequest clusterHealthRequest = clusterHealthRequest(Strings.splitStringByCommaToArray(request.param("index")));
clusterHealthRequest.indicesOptions(IndicesOptions.fromRequest(request, clusterHealthRequest.indicesOptions()));
clusterHealthRequest.local(request.paramAsBoolean("local", clusterHealthRequest.local()));
clusterHealthRequest.masterNodeTimeout(request.paramAsTime("master_timeout", clusterHealthRequest.masterNodeTimeout()));
clusterHealthRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", clusterHealthRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(clusterHealthRequest, request, deprecationLogger, "cluster_health");
clusterHealthRequest.timeout(request.paramAsTime("timeout", clusterHealthRequest.timeout()));
String waitForStatus = request.param("wait_for_status");
if (waitForStatus != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public RestClusterRerouteAction(SettingsFilter settingsFilter) {
}

// TODO: Remove the DeprecationLogger after removing MASTER_ROLE.
// It's used to log deprecation when request parameter 'metric' contains 'master_node'.
// It's used to log deprecation when request parameter 'metric' contains 'master_node', or request parameter 'master_timeout' is used.
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestClusterRerouteAction.class);
private static final String DEPRECATED_MESSAGE_MASTER_NODE =
"Deprecated value [master_node] used for parameter [metric]. To promote inclusive language, please use [cluster_manager_node] instead. It will be unsupported in a future major version.";
Expand Down Expand Up @@ -143,7 +143,8 @@ public static ClusterRerouteRequest createRequest(RestRequest request) throws IO
clusterRerouteRequest.explain(request.paramAsBoolean("explain", clusterRerouteRequest.explain()));
clusterRerouteRequest.timeout(request.paramAsTime("timeout", clusterRerouteRequest.timeout()));
clusterRerouteRequest.setRetryFailed(request.paramAsBoolean("retry_failed", clusterRerouteRequest.isRetryFailed()));
clusterRerouteRequest.masterNodeTimeout(request.paramAsTime("master_timeout", clusterRerouteRequest.masterNodeTimeout()));
clusterRerouteRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", clusterRerouteRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(clusterRerouteRequest, request, deprecationLogger, "cluster_reroute");
request.applyContentParser(parser -> PARSER.parse(parser, clusterRerouteRequest, null));
return clusterRerouteRequest;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public RestClusterStateAction(SettingsFilter settingsFilter) {
}

// TODO: Remove the DeprecationLogger after removing MASTER_ROLE.
// It's used to log deprecation when request parameter 'metric' contains 'master_node'.
// It's used to log deprecation when request parameter 'metric' contains 'master_node', or request parameter 'master_timeout' is used.
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestClusterStateAction.class);
private static final String DEPRECATED_MESSAGE_MASTER_NODE =
"Deprecated value [master_node] used for parameter [metric]. To promote inclusive language, please use [cluster_manager_node] instead. It will be unsupported in a future major version.";
Expand Down Expand Up @@ -104,7 +104,8 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
final ClusterStateRequest clusterStateRequest = Requests.clusterStateRequest();
clusterStateRequest.indicesOptions(IndicesOptions.fromRequest(request, clusterStateRequest.indicesOptions()));
clusterStateRequest.local(request.paramAsBoolean("local", clusterStateRequest.local()));
clusterStateRequest.masterNodeTimeout(request.paramAsTime("master_timeout", clusterStateRequest.masterNodeTimeout()));
clusterStateRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", clusterStateRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(clusterStateRequest, request, deprecationLogger, getName());
if (request.hasParam("wait_for_metadata_version")) {
clusterStateRequest.waitForMetadataVersion(request.paramAsLong("wait_for_metadata_version", 0));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.opensearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest;
import org.opensearch.client.Requests;
import org.opensearch.client.node.NodeClient;
import org.opensearch.common.logging.DeprecationLogger;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.xcontent.XContentParser;
import org.opensearch.rest.BaseRestHandler;
Expand All @@ -51,6 +52,8 @@

public class RestClusterUpdateSettingsAction extends BaseRestHandler {

private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestClusterUpdateSettingsAction.class);

private static final String PERSISTENT = "persistent";
private static final String TRANSIENT = "transient";

Expand All @@ -69,8 +72,9 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
final ClusterUpdateSettingsRequest clusterUpdateSettingsRequest = Requests.clusterUpdateSettingsRequest();
clusterUpdateSettingsRequest.timeout(request.paramAsTime("timeout", clusterUpdateSettingsRequest.timeout()));
clusterUpdateSettingsRequest.masterNodeTimeout(
request.paramAsTime("master_timeout", clusterUpdateSettingsRequest.masterNodeTimeout())
request.paramAsTime("cluster_manager_timeout", clusterUpdateSettingsRequest.masterNodeTimeout())
);
parseDeprecatedMasterTimeoutParameter(clusterUpdateSettingsRequest, request, deprecationLogger, getName());
Map<String, Object> source;
try (XContentParser parser = request.contentParser()) {
source = parser.map();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@

import org.opensearch.action.admin.cluster.tasks.PendingClusterTasksRequest;
import org.opensearch.client.node.NodeClient;
import org.opensearch.common.logging.DeprecationLogger;
import org.opensearch.rest.BaseRestHandler;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.action.RestToXContentListener;
Expand All @@ -46,6 +47,8 @@

public class RestPendingClusterTasksAction extends BaseRestHandler {

private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestPendingClusterTasksAction.class);

@Override
public List<Route> routes() {
return singletonList(new Route(GET, "/_cluster/pending_tasks"));
Expand All @@ -59,7 +62,10 @@ public String getName() {
@Override
public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
PendingClusterTasksRequest pendingClusterTasksRequest = new PendingClusterTasksRequest();
pendingClusterTasksRequest.masterNodeTimeout(request.paramAsTime("master_timeout", pendingClusterTasksRequest.masterNodeTimeout()));
pendingClusterTasksRequest.masterNodeTimeout(
request.paramAsTime("cluster_manager_timeout", pendingClusterTasksRequest.masterNodeTimeout())
);
parseDeprecatedMasterTimeoutParameter(pendingClusterTasksRequest, request, deprecationLogger, getName());
pendingClusterTasksRequest.local(request.paramAsBoolean("local", pendingClusterTasksRequest.local()));
return channel -> client.admin().cluster().pendingClusterTasks(pendingClusterTasksRequest, new RestToXContentListener<>(channel));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,18 @@
import org.opensearch.OpenSearchParseException;
import org.opensearch.action.support.master.MasterNodeRequest;
import org.opensearch.client.node.NodeClient;
import org.opensearch.common.bytes.BytesArray;
import org.opensearch.common.logging.DeprecationLogger;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.settings.SettingsFilter;
import org.opensearch.common.xcontent.NamedXContentRegistry;
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.rest.BaseRestHandler;
import org.opensearch.rest.action.admin.cluster.RestClusterGetSettingsAction;
import org.opensearch.rest.action.admin.cluster.RestClusterHealthAction;
import org.opensearch.rest.action.admin.cluster.RestClusterRerouteAction;
import org.opensearch.rest.action.admin.cluster.RestClusterStateAction;
import org.opensearch.rest.action.admin.cluster.RestClusterUpdateSettingsAction;
import org.opensearch.rest.action.cat.RestAllocationAction;
import org.opensearch.rest.action.cat.RestRepositoriesAction;
import org.opensearch.rest.action.cat.RestThreadPoolAction;
Expand All @@ -32,6 +41,9 @@
import org.opensearch.test.rest.FakeRestRequest;
import org.opensearch.threadpool.TestThreadPool;

import java.io.IOException;
import java.util.Collections;

import static org.hamcrest.Matchers.containsString;

/**
Expand Down Expand Up @@ -179,6 +191,68 @@ public void testCatSegments() {
assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE);
}

public void testClusterHealth() {
Exception e = assertThrows(
OpenSearchParseException.class,
() -> RestClusterHealthAction.fromRequest(getRestRequestWithBodyWithBothParams())
);
assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE));
assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE);
}

public void testClusterReroute() throws IOException {
final SettingsFilter filter = new SettingsFilter(Collections.singleton("foo.filtered"));
RestClusterRerouteAction action = new RestClusterRerouteAction(filter);
Exception e = assertThrows(
OpenSearchParseException.class,
() -> action.prepareRequest(getRestRequestWithBodyWithBothParams(), client)
);
assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE));
assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE);
}

public void testClusterState() throws IOException {
final SettingsFilter filter = new SettingsFilter(Collections.singleton("foo.filtered"));
RestClusterStateAction action = new RestClusterStateAction(filter);
Exception e = assertThrows(
OpenSearchParseException.class,
() -> action.prepareRequest(getRestRequestWithBodyWithBothParams(), client)
);
assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE));
assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE);
}

public void testClusterGetSettings() throws IOException {
final SettingsFilter filter = new SettingsFilter(Collections.singleton("foo.filtered"));
RestClusterGetSettingsAction action = new RestClusterGetSettingsAction(null, null, filter);
Exception e = assertThrows(
OpenSearchParseException.class,
() -> action.prepareRequest(getRestRequestWithBodyWithBothParams(), client)
);
assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE));
assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE);
}

public void testClusterUpdateSettings() throws IOException {
RestClusterUpdateSettingsAction action = new RestClusterUpdateSettingsAction();
Exception e = assertThrows(
OpenSearchParseException.class,
() -> action.prepareRequest(getRestRequestWithBodyWithBothParams(), client)
);
assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE));
assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE);
}

public void testClusterPendingTasks() {
RestPendingClusterTasksAction action = new RestPendingClusterTasksAction();
Exception e = assertThrows(
OpenSearchParseException.class,
() -> action.prepareRequest(getRestRequestWithBodyWithBothParams(), client)
);
assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE));
assertWarnings(MASTER_TIMEOUT_DEPRECATED_MESSAGE);
}

private MasterNodeRequest getMasterNodeRequest() {
return new MasterNodeRequest() {
@Override
Expand Down Expand Up @@ -206,4 +280,15 @@ private FakeRestRequest getRestRequestWithNewParam() {
request.params().put("cluster_manager_timeout", "2m");
return request;
}

private FakeRestRequest getRestRequestWithBodyWithBothParams() {
FakeRestRequest request = getFakeRestRequestWithBody();
request.params().put("cluster_manager_timeout", "2m");
request.params().put("master_timeout", "3s");
return request;
}

private FakeRestRequest getFakeRestRequestWithBody() {
return new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).withContent(new BytesArray("{}"), XContentType.JSON).build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ private RestRequest toRestRequest(ClusterRerouteRequest original) throws IOExcep
params.put("retry_failed", Boolean.toString(original.isRetryFailed()));
}
if (false == original.masterNodeTimeout().equals(MasterNodeRequest.DEFAULT_MASTER_NODE_TIMEOUT) || randomBoolean()) {
params.put("master_timeout", original.masterNodeTimeout().toString());
params.put("cluster_manager_timeout", original.masterNodeTimeout().toString());
}
if (original.getCommands() != null) {
hasBody = true;
Expand Down
Loading