[Backport 2.x] Add request parameter 'cluster_manager_timeout' as the…

… alternative for 'master_timeout', and deprecate 'master_timeout' - in CAT APIs (#2716) 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)
opensearch-project · Apr 4, 2022 · 4e9be6a · 4e9be6a
1 parent dcfa3fb
commit 4e9be6a
Show file tree

Hide file tree

Showing 25 changed files with 346 additions and 76 deletions.
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/cat.allocation.json b/rest-api-spec/src/main/resources/rest-api-spec/api/cat.allocation.json
@@ -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",

diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/cat.cluster_manager.json b/rest-api-spec/src/main/resources/rest-api-spec/api/cat.cluster_manager.json
@@ -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",

diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/cat.indices.json b/rest-api-spec/src/main/resources/rest-api-spec/api/cat.indices.json
@@ -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",

diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/cat.nodeattrs.json b/rest-api-spec/src/main/resources/rest-api-spec/api/cat.nodeattrs.json
@@ -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",

diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/cat.pending_tasks.json b/rest-api-spec/src/main/resources/rest-api-spec/api/cat.pending_tasks.json
@@ -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",

diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/cat.plugins.json b/rest-api-spec/src/main/resources/rest-api-spec/api/cat.plugins.json
@@ -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",

diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/cat.repositories.json b/rest-api-spec/src/main/resources/rest-api-spec/api/cat.repositories.json
@@ -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",

diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/cat.segments.json b/rest-api-spec/src/main/resources/rest-api-spec/api/cat.segments.json
@@ -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"

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
@@ -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",

diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/cat.snapshots.json b/rest-api-spec/src/main/resources/rest-api-spec/api/cat.snapshots.json
@@ -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",

diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/cat.templates.json b/rest-api-spec/src/main/resources/rest-api-spec/api/cat.templates.json
@@ -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",

diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/cat.thread_pool.json b/rest-api-spec/src/main/resources/rest-api-spec/api/cat.thread_pool.json
@@ -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",

diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestAllocationAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestAllocationAction.java
@@ -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;
@@ -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}")));
@@ -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

diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestIndicesAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestIndicesAction.java
@@ -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;
@@ -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;
@@ -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() {
@@ -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 -> {
@@ -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);
+                    }
                 }
-            });
+            );
         };
     }