Task Manager - Long claiming duration breaks flow control

[kobelb]

When claiming tasks take a long time, it causes two independent behaviors that violate the intended flow controls:

1. When claiming cycles timeout, ongoing work should be cancelled and subsequent work should not be started. Each task-manager claiming cycle currently has 30 seconds to complete, and if they don't finish within that time period, the promise is rejected and we start the next claiming cycle. Even though the promise is being rejected after 30 seconds, task-manager isn't cancelling any ongoing work or preventing additional work associated with the timed out claiming cycle from completing.

2. When a claiming cycle takes a long time to complete, missed claiming intervals shouldn't "queue up" and immediately cause additional claims. Every 3 seconds, a new claim is triggered. However, the use of a concatMap will prevent these claims from running immediately if a prior claim is incomplete, which is great. However, this causes the claims to "queue up" and run immediately after the prior claim in line is complete.

[elasticmachine]

Pinging @elastic/response-ops (Team:ResponseOps)

[mikecote]

@pmuellr mentioned that it would be good to create and share tests exposing these symptoms to discuss what options we have and know if it would solve all the problems.

[mikecote]

I took a look on one option that we can approach this. Mainly after discovering how to cancel updateByQuery calls..

1. We could move away from the observable pattern to queue polling requests and instead use setTimeout at the end of a polling cycle
2. When the 30s timeout is hit, we should cancel any ongoing "search for claimed tasks" queries.
3. When the 30s timeout is hit, we should cancel any ongoing "update by query" request. If we really need to do this, I found a theoretical way to do so by use the wait_for_completion=false flag to detach from the request and use Elasticseach's Task Management APIs to wait_for_completion=true there (https://www.elastic.co/guide/en/elasticsearch/reference/master/tasks.html). This allows the updateByQuery call to return a taskId to monitor but also cancel if ever necessary. However, the API is beta and adds a lot of overhead when the majority of the time we will not need to cancel.. There might be a way to simply use the tasks API to cancel an ongoing update by query without having to detach.

I will keep digging on better approaches, I experimented with timeouts on updateByQuery without success. There might also be a way to set a header so we can query the tasks API independently and cancel the updateByQuery behind the scenes.

EDIT: Working on something that may not be as involved here: #153491

[mikecote]

I'm working on a POC (#153491). What I'm doing is the following:

• Write a custom poller that uses setTimeout instead of observables (solves the issue of polling requests piling up)
• Remove the 30s timeout (timeoutPromiseAfter) in favour of no timeout at all. This pushes the promise to rely on the Elasticsearch request timeouts instead which should properly cancel any ongoing and subsequent code.

A polling cycle would now look something like this:

1. Run update by query for main tasks (takes up to 30s)
2. If tasks got claimed, run a search query to find the claimed tasks (takes up to 30s)
3. Run update by query for reporting tasks (takes up to 30s)
4. If reporting tasks got claimed, run a search query to find the claimed reporting tasks (takes up to 30s)

The worst case polling cycle time jumps from 30s to 2m but we can adjust as needed. It should be quite rare that these requests take that long..

Internally, by relying on the Elasticsearch client requestTimeout and setting a maxRetries: 0, it ensures the requests don't take longer than the timeout to finish and do not retry if ever they do timeout.