Skip to content

Commit

Permalink
Merge branch 'main' into cluster-manager-timeout-cat
Browse files Browse the repository at this point in the history
Signed-off-by: Tianli Feng <ftianli@amazon.com>

# Conflicts:
#	server/src/main/java/org/opensearch/rest/BaseRestHandler.java
#	server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java
#	server/src/test/java/org/opensearch/action/RenamedTimeoutRequestParameterTests.java
  • Loading branch information
Tianli Feng committed Apr 1, 2022
2 parents f97d74b + e1ee222 commit dc30f42
Show file tree
Hide file tree
Showing 22 changed files with 133 additions and 197 deletions.
1 change: 1 addition & 0 deletions .ci/bwcVersions
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,5 @@ BWC_VERSION:
- "1.2.5"
- "1.3.0"
- "1.3.1"
- "1.3.2"
- "1.4.0"
Binary file modified gradle/wrapper/gradle-wrapper.jar
Binary file not shown.
4 changes: 2 additions & 2 deletions gradle/wrapper/gradle-wrapper.properties
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-7.4.1-all.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-7.4.2-all.zip
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
distributionSha256Sum=a9a7b7baba105f6557c9dcf9c3c6e8f7e57e6b49889c5f1d133f015d0727e4be
distributionSha256Sum=e6d864e3b5bc05cc62041842b306383fc1fefcec359e70cebb1d470a6094ca82
2 changes: 1 addition & 1 deletion qa/wildfly/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ testFixtures.useFixture()

dependencies {
providedCompile 'javax.enterprise:cdi-api:1.2'
providedCompile 'org.jboss.spec.javax.annotation:jboss-annotations-api_1.2_spec:1.0.0.Final'
providedCompile 'org.jboss.spec.javax.annotation:jboss-annotations-api_1.2_spec:1.0.2.Final'
providedCompile 'org.jboss.spec.javax.ws.rs:jboss-jaxrs-api_2.0_spec:1.0.0.Final'
api('org.jboss.resteasy:resteasy-jackson2-provider:3.0.19.Final') {
exclude module: 'jackson-annotations'
Expand Down
1 change: 1 addition & 0 deletions server/src/main/java/org/opensearch/Version.java
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ public class Version implements Comparable<Version>, ToXContentFragment {
public static final Version V_1_2_5 = new Version(1020599, org.apache.lucene.util.Version.LUCENE_8_10_1);
public static final Version V_1_3_0 = new Version(1030099, org.apache.lucene.util.Version.LUCENE_8_10_1);
public static final Version V_1_3_1 = new Version(1030199, org.apache.lucene.util.Version.LUCENE_8_10_1);
public static final Version V_1_3_2 = new Version(1030299, org.apache.lucene.util.Version.LUCENE_8_10_1);
public static final Version V_1_4_0 = new Version(1040099, org.apache.lucene.util.Version.LUCENE_8_10_1);
public static final Version V_2_0_0 = new Version(2000099, org.apache.lucene.util.Version.LUCENE_9_1_0);
public static final Version CURRENT = V_2_0_0;
Expand Down
46 changes: 29 additions & 17 deletions server/src/main/java/org/opensearch/rest/BaseRestHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.apache.logging.log4j.Logger;
import org.apache.lucene.search.spell.LevenshteinDistance;
import org.apache.lucene.util.CollectionUtil;
import org.opensearch.OpenSearchParseException;
import org.opensearch.action.support.master.MasterNodeRequest;
import org.opensearch.client.node.NodeClient;
import org.opensearch.common.CheckedConsumer;
Expand Down Expand Up @@ -202,6 +203,34 @@ protected Set<String> responseParams() {
return Collections.emptySet();
}

/**
* Parse the deprecated request parameter 'master_timeout', and add deprecated log if the parameter is used.
* It also validates whether the two parameters 'master_timeout' and 'cluster_manager_timeout' are not assigned together.
* The method is temporarily added in 2.0 duing applying inclusive language. Remove the method along with MASTER_ROLE.
* @param mnr the action request
* @param request the REST request to handle
* @param logger the logger that logs deprecation notices
* @param logMsgKeyPrefix the key prefix of a deprecation message to avoid duplicate messages.
*/
public static void parseDeprecatedMasterTimeoutParameter(
MasterNodeRequest mnr,
RestRequest request,
DeprecationLogger logger,
String logMsgKeyPrefix
) {
final String MASTER_TIMEOUT_DEPRECATED_MESSAGE =
"Deprecated parameter [master_timeout] used. To promote inclusive language, please use [cluster_manager_timeout] instead. It will be unsupported in a future major version.";
final String DUPLICATE_PARAMETER_ERROR_MESSAGE =
"Please only use one of the request parameters [master_timeout, cluster_manager_timeout].";
if (request.hasParam("master_timeout")) {
logger.deprecate(logMsgKeyPrefix + "_master_timeout_parameter", MASTER_TIMEOUT_DEPRECATED_MESSAGE);
if (request.hasParam("cluster_manager_timeout")) {
throw new OpenSearchParseException(DUPLICATE_PARAMETER_ERROR_MESSAGE);
}
mnr.masterNodeTimeout(request.paramAsTime("master_timeout", mnr.masterNodeTimeout()));
}
}

public static class Wrapper extends BaseRestHandler {

protected final BaseRestHandler delegate;
Expand Down Expand Up @@ -260,21 +289,4 @@ public boolean allowSystemIndexAccessByDefault() {
return delegate.allowSystemIndexAccessByDefault();
}
}

/**
* Parse the deprecated request parameter 'master_timeout', and add deprecated log if the parameter is used.
* It also validates whether the value of 'master_timeout' is the same with 'cluster_manager_timeout'.
* Remove the method along with MASTER_ROLE.
* @deprecated As of 2.0, because promoting inclusive language.
*/
@Deprecated
protected static void parseDeprecatedMasterTimeoutParameter(DeprecationLogger logger, MasterNodeRequest mnr, RestRequest request) {
final String MASTER_TIMEOUT_DEPRECATED_MESSAGE =
"Deprecated parameter [master_timeout] used. To promote inclusive language, please use [cluster_manager_timeout] instead. It will be unsupported in a future major version.";
if (request.hasParam("master_timeout")) {
logger.deprecate("master_timeout_request_parameter", MASTER_TIMEOUT_DEPRECATED_MESSAGE);
request.validateParamValuesAreEqual("master_timeout", "cluster_manager_timeout");
mnr.masterNodeTimeout(request.paramAsTime("master_timeout", mnr.masterNodeTimeout()));
}
}
}
27 changes: 0 additions & 27 deletions server/src/main/java/org/opensearch/rest/RestRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
import java.io.IOException;
import java.io.InputStream;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -579,32 +578,6 @@ public static XContentType parseContentType(List<String> header) {
throw new IllegalArgumentException("empty Content-Type header");
}

/**
* The method is only used to validate whether the values of the 2 request parameters "master_timeout" and "cluster_manager_timeout" is the same value or not.
* If the 2 values are not the same, throw an {@link OpenSearchParseException}.
* @param keys Names of the request parameters.
* @deprecated The method will be removed along with the request parameter "master_timeout".
*/
@Deprecated
public void validateParamValuesAreEqual(String... keys) {
// Track the last seen value and ensure that every subsequent value matches it.
// The value to be tracked is the non-empty values of the parameters with the key.
String lastSeenValue = null;
for (String key : keys) {
String value = param(key);
if (!Strings.isNullOrEmpty(value)) {
if (lastSeenValue == null || value.equals(lastSeenValue)) {
lastSeenValue = value;
} else {
throw new OpenSearchParseException(
"The values of the request parameters: {} are required to be equal, otherwise please only assign value to one of the request parameters.",
Arrays.toString(keys)
);
}
}
}
}

public static class ContentTypeHeaderException extends RuntimeException {

ContentTypeHeaderException(final IllegalArgumentException cause) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli
clusterStateRequest.clear().routingTable(true);
clusterStateRequest.local(request.paramAsBoolean("local", clusterStateRequest.local()));
clusterStateRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", clusterStateRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(deprecationLogger, clusterStateRequest, request);
parseDeprecatedMasterTimeoutParameter(clusterStateRequest, request, deprecationLogger, getName());

return channel -> client.admin().cluster().state(clusterStateRequest, new RestActionListener<ClusterStateResponse>(channel) {
@Override
Expand Down Expand Up @@ -185,4 +185,5 @@ private Table buildTable(RestRequest request, final ClusterStateResponse state,

return table;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

package org.opensearch.rest.action.cat;

import org.opensearch.OpenSearchParseException;
import org.opensearch.action.ActionListener;
import org.opensearch.action.ActionResponse;
import org.opensearch.action.admin.cluster.health.ClusterHealthRequest;
Expand Down Expand Up @@ -86,6 +87,8 @@ public class RestIndicesAction extends AbstractCatAction {
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestIndicesAction.class);
private static final String MASTER_TIMEOUT_DEPRECATED_MESSAGE =
"Deprecated parameter [master_timeout] used. To promote inclusive language, please use [cluster_manager_timeout] instead. It will be unsupported in a future major version.";
private static final String DUPLICATE_PARAMETER_ERROR_MESSAGE =
"Please only use one of the request parameters [master_timeout, cluster_manager_timeout].";

@Override
public List<Route> routes() {
Expand Down Expand Up @@ -117,7 +120,9 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli
// Remove the if condition and statements inside after removing MASTER_ROLE.
if (request.hasParam("master_timeout")) {
deprecationLogger.deprecate("cat_indices_master_timeout_parameter", MASTER_TIMEOUT_DEPRECATED_MESSAGE);
request.validateParamValuesAreEqual("master_timeout", "cluster_manager_timeout");
if (request.hasParam("cluster_manager_timeout")) {
throw new OpenSearchParseException(DUPLICATE_PARAMETER_ERROR_MESSAGE);
}
clusterManagerTimeout = request.paramAsTime("master_timeout", DEFAULT_MASTER_NODE_TIMEOUT);
}
final TimeValue clusterManagerNodeTimeout = clusterManagerTimeout;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli
clusterStateRequest.clear().nodes(true);
clusterStateRequest.local(request.paramAsBoolean("local", clusterStateRequest.local()));
clusterStateRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", clusterStateRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(deprecationLogger, clusterStateRequest, request);
parseDeprecatedMasterTimeoutParameter(clusterStateRequest, request, deprecationLogger, getName());

return channel -> client.admin().cluster().state(clusterStateRequest, new RestResponseListener<ClusterStateResponse>(channel) {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli
clusterStateRequest.clear().nodes(true);
clusterStateRequest.local(request.paramAsBoolean("local", clusterStateRequest.local()));
clusterStateRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", clusterStateRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(deprecationLogger, clusterStateRequest, request);
parseDeprecatedMasterTimeoutParameter(clusterStateRequest, request, deprecationLogger, getName());

return channel -> client.admin().cluster().state(clusterStateRequest, new RestActionListener<ClusterStateResponse>(channel) {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli
}
clusterStateRequest.local(request.paramAsBoolean("local", clusterStateRequest.local()));
clusterStateRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", clusterStateRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(deprecationLogger, clusterStateRequest, request);
parseDeprecatedMasterTimeoutParameter(clusterStateRequest, request, deprecationLogger, getName());
final boolean fullId = request.paramAsBoolean("full_id", false);
return channel -> client.admin().cluster().state(clusterStateRequest, new RestActionListener<ClusterStateResponse>(channel) {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli
pendingClusterTasksRequest.masterNodeTimeout(
request.paramAsTime("cluster_manager_timeout", pendingClusterTasksRequest.masterNodeTimeout())
);
parseDeprecatedMasterTimeoutParameter(deprecationLogger, pendingClusterTasksRequest, request);
parseDeprecatedMasterTimeoutParameter(pendingClusterTasksRequest, request, deprecationLogger, getName());
pendingClusterTasksRequest.local(request.paramAsBoolean("local", pendingClusterTasksRequest.local()));
return channel -> client.admin()
.cluster()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli
clusterStateRequest.clear().nodes(true);
clusterStateRequest.local(request.paramAsBoolean("local", clusterStateRequest.local()));
clusterStateRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", clusterStateRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(deprecationLogger, clusterStateRequest, request);
parseDeprecatedMasterTimeoutParameter(clusterStateRequest, request, deprecationLogger, getName());

return channel -> client.admin().cluster().state(clusterStateRequest, new RestActionListener<ClusterStateResponse>(channel) {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public RestChannelConsumer doCatRequest(RestRequest request, NodeClient client)
getRepositoriesRequest.masterNodeTimeout(
request.paramAsTime("cluster_manager_timeout", getRepositoriesRequest.masterNodeTimeout())
);
parseDeprecatedMasterTimeoutParameter(deprecationLogger, getRepositoriesRequest, request);
parseDeprecatedMasterTimeoutParameter(getRepositoriesRequest, request, deprecationLogger, getName());

return channel -> client.admin()
.cluster()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli
final ClusterStateRequest clusterStateRequest = new ClusterStateRequest();
clusterStateRequest.local(request.paramAsBoolean("local", clusterStateRequest.local()));
clusterStateRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", clusterStateRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(deprecationLogger, clusterStateRequest, request);
parseDeprecatedMasterTimeoutParameter(clusterStateRequest, request, deprecationLogger, getName());
clusterStateRequest.clear().nodes(true).routingTable(true).indices(indices);

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 @@ -105,7 +105,7 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli
final ClusterStateRequest clusterStateRequest = new ClusterStateRequest();
clusterStateRequest.local(request.paramAsBoolean("local", clusterStateRequest.local()));
clusterStateRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", clusterStateRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(deprecationLogger, clusterStateRequest, request);
parseDeprecatedMasterTimeoutParameter(clusterStateRequest, request, deprecationLogger, getName());
clusterStateRequest.clear().nodes(true).routingTable(true).indices(indices);
return channel -> client.admin().cluster().state(clusterStateRequest, new RestActionListener<ClusterStateResponse>(channel) {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public RestChannelConsumer doCatRequest(final RestRequest request, NodeClient cl
getSnapshotsRequest.ignoreUnavailable(request.paramAsBoolean("ignore_unavailable", getSnapshotsRequest.ignoreUnavailable()));

getSnapshotsRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", getSnapshotsRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(deprecationLogger, getSnapshotsRequest, request);
parseDeprecatedMasterTimeoutParameter(getSnapshotsRequest, request, deprecationLogger, getName());

return channel -> client.admin()
.cluster()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public RestChannelConsumer doCatRequest(final RestRequest request, NodeClient cl
clusterStateRequest.clear().metadata(true);
clusterStateRequest.local(request.paramAsBoolean("local", clusterStateRequest.local()));
clusterStateRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", clusterStateRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(deprecationLogger, clusterStateRequest, request);
parseDeprecatedMasterTimeoutParameter(clusterStateRequest, request, deprecationLogger, getName());

return channel -> client.admin().cluster().state(clusterStateRequest, new RestResponseListener<ClusterStateResponse>(channel) {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli
clusterStateRequest.clear().nodes(true);
clusterStateRequest.local(request.paramAsBoolean("local", clusterStateRequest.local()));
clusterStateRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", clusterStateRequest.masterNodeTimeout()));
parseDeprecatedMasterTimeoutParameter(deprecationLogger, clusterStateRequest, request);
parseDeprecatedMasterTimeoutParameter(clusterStateRequest, request, deprecationLogger, getName());

return channel -> client.admin().cluster().state(clusterStateRequest, new RestActionListener<ClusterStateResponse>(channel) {
@Override
Expand Down
Loading

0 comments on commit dc30f42

Please sign in to comment.