From 1ceb86deb3b094da8fe7e1e644dd609a23b9b110 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 15 Mar 2021 20:41:06 +0100 Subject: [PATCH] Enable Setting Master Node Timeout in Watcher Start/Stop Requests 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. --- .../docs/en/rest-api/watcher/start.asciidoc | 9 ++++- x-pack/docs/en/rest-api/watcher/stop.asciidoc | 9 ++++- .../rest/action/RestWatchServiceAction.java | 7 ++-- .../TransportWatcherServiceAction.java | 38 +++++++------------ 4 files changed, 32 insertions(+), 31 deletions(-) diff --git a/x-pack/docs/en/rest-api/watcher/start.asciidoc b/x-pack/docs/en/rest-api/watcher/start.asciidoc index b153410ed2901..c565ca8693331 100644 --- 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 <>. //[[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, <>) 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 index f0e733df39792..a981b4ccb0f69 100644 --- 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 <>. //[[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, <>) 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/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/rest/action/RestWatchServiceAction.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/rest/action/RestWatchServiceAction.java index 4bc335bd23f6a..3cdf828374ce5 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/rest/action/RestWatchServiceAction.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/rest/action/RestWatchServiceAction.java @@ -49,9 +49,10 @@ public String getName() { } @Override - public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) { - return channel -> - client.execute(WatcherServiceAction.INSTANCE, new WatcherServiceRequest().stop(), new RestToXContentListener<>(channel)); + public RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) { + final WatcherServiceRequest request = new WatcherServiceRequest().stop(); + request.masterNodeTimeout(restRequest.paramAsTime("master_timeout", request.masterNodeTimeout())); + return channel -> client.execute(WatcherServiceAction.INSTANCE, request, new RestToXContentListener<>(channel)); } } } diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/transport/actions/TransportWatcherServiceAction.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/transport/actions/TransportWatcherServiceAction.java index f187e67241b9b..a2573e56fd606 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/transport/actions/TransportWatcherServiceAction.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/transport/actions/TransportWatcherServiceAction.java @@ -36,18 +36,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, @@ -59,20 +47,22 @@ public TransportWatcherServiceAction(TransportService transportService, ClusterS @Override protected void masterOperation(Task task, WatcherServiceRequest request, ClusterState state, ActionListener 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 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);