Enable Setting Master Node Timeout in Watcher Start/Stop Requests (#7…

…0425) (#70434) It's in the title, we have to be able to set this timeout. Otherwise, it's impossible to deactive/active watcher or a slow master node. In the worst case scenario, Wacher may be at fault for making the master slow and it becomes impossible to deactive it.
elastic · Mar 16, 2021 · 822016c · 822016c
1 parent b901073
commit 822016c
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 30 deletions.
diff --git a/x-pack/docs/en/rest-api/watcher/start.asciidoc b/x-pack/docs/en/rest-api/watcher/start.asciidoc
@@ -24,8 +24,13 @@ information, see <<security-privileges>>.
 //[[watcher-api-start-path-params]]
 //==== {api-path-parms-title}
 
-//[[watcher-api-start-query-params]]
-//==== {api-query-parms-title}
+[[watcher-api-start-query-params]]
+==== {api-query-parms-title}
+
+`master_timeout`::
+(Optional, <<time-units, time units>>) Specifies the period of time to wait for
+a connection to the master node. If no response is received before the timeout
+expires, the request fails and returns an error. Defaults to `30s`.
 
 //[[watcher-api-start-request-body]]
 //==== {api-request-body-title}

diff --git a/x-pack/docs/en/rest-api/watcher/stop.asciidoc b/x-pack/docs/en/rest-api/watcher/stop.asciidoc
@@ -24,8 +24,13 @@ information, see <<security-privileges>>.
 //[[watcher-api-stop-path-params]]
 //==== {api-path-parms-title}
 
-//[[watcher-api-stop-query-params]]
-//==== {api-query-parms-title}
+[[watcher-api-stop-query-params]]
+==== {api-query-parms-title}
+
+`master_timeout`::
+(Optional, <<time-units, time units>>) Specifies the period of time to wait for
+a connection to the master node. If no response is received before the timeout
+expires, the request fails and returns an error. Defaults to `30s`.
 
 //[[watcher-api-stop-request-body]]
 //==== {api-request-body-title}

diff --git a/...her/src/main/java/org/elasticsearch/xpack/watcher/rest/action/RestWatchServiceAction.java b/...her/src/main/java/org/elasticsearch/xpack/watcher/rest/action/RestWatchServiceAction.java
@@ -59,8 +59,10 @@ public String getName() {
         }
 
         @Override
-        public RestChannelConsumer doPrepareRequest(RestRequest request, WatcherClient client) {
-            return channel -> client.watcherService(new WatcherServiceRequest().stop(), new RestToXContentListener<>(channel));
+        public RestChannelConsumer doPrepareRequest(RestRequest restRequest, WatcherClient client) {
+            final WatcherServiceRequest request = new WatcherServiceRequest().stop();
+            request.masterNodeTimeout(restRequest.paramAsTime("master_timeout", request.masterNodeTimeout()));
+            return channel -> client.watcherService(request, new RestToXContentListener<>(channel));
         }
     }
 }
diff --git a/...java/org/elasticsearch/xpack/watcher/transport/actions/TransportWatcherServiceAction.java b/...java/org/elasticsearch/xpack/watcher/transport/actions/TransportWatcherServiceAction.java
@@ -35,18 +35,6 @@ public class TransportWatcherServiceAction extends AcknowledgedTransportMasterNo
 
     private static final Logger logger = LogManager.getLogger(TransportWatcherServiceAction.class);
 
-    private static final AckedRequest ackedRequest = new AckedRequest() {
-        @Override
-        public TimeValue ackTimeout() {
-            return AcknowledgedRequest.DEFAULT_ACK_TIMEOUT;
-        }
-
-        @Override
-        public TimeValue masterNodeTimeout() {
-            return AcknowledgedRequest.DEFAULT_ACK_TIMEOUT;
-        }
-    };
-
     @Inject
     public TransportWatcherServiceAction(TransportService transportService, ClusterService clusterService,
                                          ThreadPool threadPool, ActionFilters actionFilters,
@@ -58,20 +46,22 @@ public TransportWatcherServiceAction(TransportService transportService, ClusterS
     @Override
     protected void masterOperation(WatcherServiceRequest request, ClusterState state,
                                    ActionListener<AcknowledgedResponse> listener) {
-        switch (request.getCommand()) {
-            case STOP:
-                setWatcherMetadataAndWait(true, listener);
-                break;
-            case START:
-                setWatcherMetadataAndWait(false, listener);
-                break;
-        }
-    }
+        final boolean manuallyStopped = request.getCommand() == WatcherServiceRequest.Command.STOP;
+        final String source = manuallyStopped ? "update_watcher_manually_stopped" : "update_watcher_manually_started";
 
-    private void setWatcherMetadataAndWait(boolean manuallyStopped, final ActionListener<AcknowledgedResponse> listener) {
-        String source = manuallyStopped ? "update_watcher_manually_stopped" : "update_watcher_manually_started";
+        // TODO: make WatcherServiceRequest a real AckedRequest so that we have both a configurable timeout and master node timeout like
+        //       we do elsewhere
+        clusterService.submitStateUpdateTask(source, new AckedClusterStateUpdateTask(new AckedRequest() {
+            @Override
+            public TimeValue ackTimeout() {
+                return AcknowledgedRequest.DEFAULT_ACK_TIMEOUT;
+            }
 
-        clusterService.submitStateUpdateTask(source, new AckedClusterStateUpdateTask(ackedRequest, listener) {
+            @Override
+            public TimeValue masterNodeTimeout() {
+                return request.masterNodeTimeout();
+            }
+        }, listener) {
             @Override
             public ClusterState execute(ClusterState clusterState) {
                 XPackPlugin.checkReadyForXPackCustomMetadata(clusterState);