Skip to content

Commit

Permalink
Add cluster_manager_timemout in Cat indices api
Browse files Browse the repository at this point in the history 
Signed-off-by: Tianli Feng <ftianli@amazon.com>
  • Loading branch information
Tianli Feng committed Mar 22, 2022
1 parent 92b48f3 commit 006da61
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 57 deletions.
10 changes: 9 additions & 1 deletion rest-api-spec/src/main/resources/rest-api-spec/api/cat.indices.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,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"
},
"h":{
"type":"list",
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/opensearch/rest/action/cat/RestAllocationAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ private Table buildTable(RestRequest request, final ClusterStateResponse state,
private void parseDeprecatedMasterTimeoutParameter(ClusterStateRequest clusterStateRequest, RestRequest request) {
final String deprecatedTimeoutParam = "master_timeout";
if (request.hasParam(deprecatedTimeoutParam)) {
deprecationLogger.deprecate("cat_nodes_master_timeout_parameter", MASTER_TIMEOUT_DEPRECATED_MESSAGE);
deprecationLogger.deprecate("cat_allocation_master_timeout_parameter", MASTER_TIMEOUT_DEPRECATED_MESSAGE);
request.validateParamValuesAreEqual(deprecatedTimeoutParam, "cluster_manager_timeout");
clusterStateRequest.masterNodeTimeout(request.paramAsTime(deprecatedTimeoutParam, clusterStateRequest.masterNodeTimeout()));
}
Expand Down
134 changes: 84 additions & 50 deletions server/src/main/java/org/opensearch/rest/action/cat/RestIndicesAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.common.Strings;
import org.opensearch.common.Table;
import org.opensearch.common.logging.DeprecationLogger;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.time.DateFormatter;
import org.opensearch.common.unit.TimeValue;
Expand Down Expand Up @@ -82,6 +83,9 @@
public class RestIndicesAction extends AbstractCatAction {

private static final DateFormatter STRICT_DATE_TIME_FORMATTER = DateFormatter.forPattern("strict_date_time");
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestIndicesAction.class);
public 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.";

@Override
public List<Route> routes() {
Expand Down Expand Up @@ -109,7 +113,9 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli
final String[] indices = Strings.splitStringByCommaToArray(request.param("index"));
final IndicesOptions indicesOptions = IndicesOptions.fromRequest(request, IndicesOptions.strictExpand());
final boolean local = request.paramAsBoolean("local", false);
final TimeValue masterNodeTimeout = request.paramAsTime("master_timeout", DEFAULT_MASTER_NODE_TIMEOUT);
TimeValue clusterManagerTimeout = request.paramAsTime("cluster_manager_timeout", DEFAULT_MASTER_NODE_TIMEOUT);
TimeValue deprecatedMasterTimeout = parseDeprecatedMasterTimeoutParameter(request);
final TimeValue clusterManagerNodeTimeout = deprecatedMasterTimeout == null ? clusterManagerTimeout : deprecatedMasterTimeout;
final boolean includeUnloadedSegments = request.paramAsBoolean("include_unloaded_segments", false);

return channel -> {
Expand All @@ -120,56 +126,66 @@ public RestResponse buildResponse(final Table table) throws Exception {
}
});

sendGetSettingsRequest(indices, indicesOptions, local, masterNodeTimeout, client, new ActionListener<GetSettingsResponse>() {
@Override
public void onResponse(final GetSettingsResponse getSettingsResponse) {
final GroupedActionListener<ActionResponse> groupedListener = createGroupedListener(request, 4, listener);
groupedListener.onResponse(getSettingsResponse);

// The list of indices that will be returned is determined by the indices returned from the Get Settings call.
// All the other requests just provide additional detail, and wildcards may be resolved differently depending on the
// type of request in the presence of security plugins (looking at you, ClusterHealthRequest), so
// force the IndicesOptions for all the sub-requests to be as inclusive as possible.
final IndicesOptions subRequestIndicesOptions = IndicesOptions.lenientExpandHidden();

// Indices that were successfully resolved during the get settings request might be deleted when the subsequent cluster
// state, cluster health and indices stats requests execute. We have to distinguish two cases:
// 1) the deleted index was explicitly passed as parameter to the /_cat/indices request. In this case we want the
// subsequent requests to fail.
// 2) the deleted index was resolved as part of a wildcard or _all. In this case, we want the subsequent requests not to
// fail on the deleted index (as we want to ignore wildcards that cannot be resolved).
// This behavior can be ensured by letting the cluster state, cluster health and indices stats requests re-resolve the
// index names with the same indices options that we used for the initial cluster state request (strictExpand).
sendIndicesStatsRequest(
indices,
subRequestIndicesOptions,
includeUnloadedSegments,
client,
ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure)
);
sendClusterStateRequest(
indices,
subRequestIndicesOptions,
local,
masterNodeTimeout,
client,
ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure)
);
sendClusterHealthRequest(
indices,
subRequestIndicesOptions,
local,
masterNodeTimeout,
client,
ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure)
);
sendGetSettingsRequest(
indices,
indicesOptions,
local,
clusterManagerNodeTimeout,
client,
new ActionListener<GetSettingsResponse>() {
@Override
public void onResponse(final GetSettingsResponse getSettingsResponse) {
final GroupedActionListener<ActionResponse> groupedListener = createGroupedListener(request, 4, listener);
groupedListener.onResponse(getSettingsResponse);

// The list of indices that will be returned is determined by the indices returned from the Get Settings call.
// All the other requests just provide additional detail, and wildcards may be resolved differently depending on the
// type of request in the presence of security plugins (looking at you, ClusterHealthRequest), so
// force the IndicesOptions for all the sub-requests to be as inclusive as possible.
final IndicesOptions subRequestIndicesOptions = IndicesOptions.lenientExpandHidden();

// Indices that were successfully resolved during the get settings request might be deleted when the subsequent
// cluster
// state, cluster health and indices stats requests execute. We have to distinguish two cases:
// 1) the deleted index was explicitly passed as parameter to the /_cat/indices request. In this case we want the
// subsequent requests to fail.
// 2) the deleted index was resolved as part of a wildcard or _all. In this case, we want the subsequent requests
// not to
// fail on the deleted index (as we want to ignore wildcards that cannot be resolved).
// This behavior can be ensured by letting the cluster state, cluster health and indices stats requests re-resolve
// the
// index names with the same indices options that we used for the initial cluster state request (strictExpand).
sendIndicesStatsRequest(
indices,
subRequestIndicesOptions,
includeUnloadedSegments,
client,
ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure)
);
sendClusterStateRequest(
indices,
subRequestIndicesOptions,
local,
clusterManagerNodeTimeout,
client,
ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure)
);
sendClusterHealthRequest(
indices,
subRequestIndicesOptions,
local,
clusterManagerNodeTimeout,
client,
ActionListener.wrap(groupedListener::onResponse, groupedListener::onFailure)
);
}

@Override
public void onFailure(final Exception e) {
listener.onFailure(e);
}
}

@Override
public void onFailure(final Exception e) {
listener.onFailure(e);
}
});
);
};
}

Expand Down Expand Up @@ -895,4 +911,22 @@ Table buildTable(
private static <A extends ActionResponse> A extractResponse(final Collection<? extends ActionResponse> responses, Class<A> c) {
return (A) responses.stream().filter(c::isInstance).findFirst().get();
}

/**
* 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
private TimeValue parseDeprecatedMasterTimeoutParameter(RestRequest request) {
final String deprecatedTimeoutParam = "master_timeout";
TimeValue clusterManagerTimeout = null;
if (request.hasParam(deprecatedTimeoutParam)) {
deprecationLogger.deprecate("cat_indices_master_timeout_parameter", MASTER_TIMEOUT_DEPRECATED_MESSAGE);
request.validateParamValuesAreEqual(deprecatedTimeoutParam, "cluster_manager_timeout");
clusterManagerTimeout = request.paramAsTime(deprecatedTimeoutParam, DEFAULT_MASTER_NODE_TIMEOUT);
}
return clusterManagerTimeout;
}
}
24 changes: 19 additions & 5 deletions server/src/test/java/org/opensearch/action/RenamedTimeoutRequestParameterTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,37 +8,51 @@

package org.opensearch.action;

import org.junit.AfterClass;
import org.opensearch.OpenSearchParseException;
import org.opensearch.client.node.NodeClient;
import org.opensearch.common.settings.Settings;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.action.cat.RestAllocationAction;
import org.opensearch.rest.action.cat.RestIndicesAction;
import org.opensearch.test.OpenSearchTestCase;
import org.opensearch.test.rest.FakeRestRequest;
import org.opensearch.threadpool.TestThreadPool;

import static org.hamcrest.CoreMatchers.containsString;

/**
* As of 2.0, the request parameter 'master_timeout' in all applicable REST APIs is deprecated,
* As of 2.0, the request parameter 'master_timeout' in all applicable REST APIs is deprecated,
* and alternative parameter 'cluster_manager_timeout' is added.
* The tests are used to validate the behavior about the renamed request parameter.
* Remove the test after removing MASTER_ROLE and 'master_timeout'.
*/
public class RenamedTimeoutRequestParameterTests extends OpenSearchTestCase {
private static final String PARAM_VALUE_ERROR_MESSAGE = "[master_timeout, cluster_manager_timeout] are required to be equal";
private static final TestThreadPool threadPool = new TestThreadPool(RenamedTimeoutRequestParameterTests.class.getName());
private final NodeClient client = new NodeClient(Settings.EMPTY, threadPool);
private final RestRequest request = getFakeRestRequestWithBothOldAndNewTimeoutParam();

@AfterClass
public static void terminateThreadPool() {
terminate(threadPool);
}

/**
* Validate both cluster_manager_timeout and its predecessor can be parsed correctly.
*/
public void testCatAllocationTimeoutErrorAndWarning() {
RestAllocationAction action = new RestAllocationAction();
TestThreadPool threadPool = new TestThreadPool(RenamedTimeoutRequestParameterTests.class.getName());
NodeClient client = new NodeClient(Settings.EMPTY, threadPool);
FakeRestRequest request = getFakeRestRequestWithBothOldAndNewTimeoutParam();
Exception e = assertThrows(OpenSearchParseException.class, () -> action.doCatRequest(request, client));
assertThat(e.getMessage(), containsString(PARAM_VALUE_ERROR_MESSAGE));
assertWarnings(RestAllocationAction.MASTER_TIMEOUT_DEPRECATED_MESSAGE);
terminate(threadPool);
}

public void testCatIndicesTimeoutErrorAndWarning() {
RestIndicesAction action = new RestIndicesAction();
Exception e = assertThrows(OpenSearchParseException.class, () -> action.doCatRequest(request, client));
assertThat(e.getMessage(), containsString(PARAM_VALUE_ERROR_MESSAGE));
assertWarnings(RestIndicesAction.MASTER_TIMEOUT_DEPRECATED_MESSAGE);
}

private FakeRestRequest getFakeRestRequestWithBothOldAndNewTimeoutParam() {
Expand Down

0 comments on commit 006da61

Please sign in to comment.