Skip to content

Commit

Permalink
Add request parameter 'cluster_manager_timeout' as the alternative fo…
Browse files Browse the repository at this point in the history
…r 'master_timeout', and deprecate 'master_timeout' - in CAT APIs (#2557)

Apply the change of CAT Nodes API in PR #2435 to other applicable CAT APIs.

- Deprecate the request parameter `master_timeout` that used in many CAT APIs.
- Add alternative new request parameter `cluster_manager_timeout`.
- Add unit tests.

Signed-off-by: Tianli Feng <[email protected]>
(cherry picked from commit 78465b4)
  • Loading branch information
Tianli Feng authored and github-actions[bot] committed Apr 1, 2022
1 parent dcfa3fb commit 66fd5b3
Show file tree
Hide file tree
Showing 25 changed files with 347 additions and 76 deletions.
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
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,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
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
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,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
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,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
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,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
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,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
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,18 @@
"pb"
]
},
"master_timeout":{
"type":"time",
"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",
"description":"Comma-separated list of column names to display"
Expand Down
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
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,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
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,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
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,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
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.opensearch.cluster.routing.ShardRouting;
import org.opensearch.common.Strings;
import org.opensearch.common.Table;
import org.opensearch.common.logging.DeprecationLogger;
import org.opensearch.common.unit.ByteSizeValue;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.RestResponse;
Expand All @@ -58,6 +59,8 @@

public class RestAllocationAction extends AbstractCatAction {

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

@Override
public List<Route> routes() {
return unmodifiableList(asList(new Route(GET, "/_cat/allocation"), new Route(GET, "/_cat/allocation/{nodes}")));
Expand All @@ -79,7 +82,8 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli
final ClusterStateRequest clusterStateRequest = new ClusterStateRequest();
clusterStateRequest.clear().routingTable(true);
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 RestActionListener<ClusterStateResponse>(channel) {
@Override
Expand Down
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 All @@ -52,6 +53,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 +84,11 @@
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);
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 @@ -109,7 +116,16 @@ 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);
// 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);
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;
final boolean includeUnloadedSegments = request.paramAsBoolean("include_unloaded_segments", false);

return channel -> {
Expand All @@ -120,56 +136,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)
);
}

@Override
public void onFailure(final Exception e) {
listener.onFailure(e);
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);
}
}
});
);
};
}

Expand Down
Loading

0 comments on commit 66fd5b3

Please sign in to comment.