[ML] Make APIs cancellable for the ML and Transforms plugin

[benwtrent]

Many of the ML APIs should be cancellable as they should not continue executing once the caller disconnects the HTTP connection.

Here is the list:

* Denotes more important APIs to get done as they are heavier requests used by many automatic processes

For ML

• Get calendar events
• Get calendars actions
• Cat datafeeds
• Cat dataframes
• Cat jobs
• cat trained models
• Get datafeeds * ([ML] make more GET and heavier processing APIs cancellable #88142)
• Get datafeeds stats ([ML] make ML stats APIs cancellable #88030)
• Preview Datafeeds * ([ML] make more GET and heavier processing APIs cancellable #88142)
• Evaluate data frame analytics ([ML] make more GET and heavier processing APIs cancellable #88142)
• Explain data frame analytics ([ML] make more GET and heavier processing APIs cancellable #88142)
• Get data frame analytics ([ML] make trained model rest APIs cancellable #88009)
• Get data frame analytics stats ([ML] make ML stats APIs cancellable #88030)
• Preview data frame analytics ([ML] make more GET and heavier processing APIs cancellable #88142)
• Get filters ([ML] make trained model rest APIs cancellable #88009)
• Get trained models * ([ML] make trained model rest APIs cancellable #88009)
• Get trained models stats * ([ML] make trained model rest APIs cancellable #88009)
• Inter trained model (and deployment) ([ML] make trained model rest APIs cancellable #88009)
• Estimate model memory *
• Get jobs ([ML] make more GET and heavier processing APIs cancellable #88142)
• Get jobs stats ([ML] make ML stats APIs cancellable #88030)
• Get job model upgrade stats ([ML] make ML stats APIs cancellable #88030)
• Get model snapshots ([ML] make more GET and heavier processing APIs cancellable #88142)
• All the results APIs
• Validate detector
• Validate job config

For transforms

• Cat transform
• Get transforms * ([ML] make trained model rest APIs cancellable #88009)
• Get transforms stats * ([Transform] _stats should respect timeout parameter and be cancelable #91616)
• Preview transforms * ([Transform] Make preview calls cancellable and respect timeout #91286)

[elasticmachine]

Pinging @elastic/ml-core (Team:ML)

[pequolau]

Hi, can I work on this part?

• Estimate model memory

[droberts195]

@pequolau sure, if you could work on the estimate model memory endpoint that would be great.

[DaveCTurner]

Seems like a good plan, but if these things are heavy enough to need to be cancellable then they should also not be running on SAME (i.e. a transport worker, NB #97916). Also it's not enough to just add a RestCancellableNodeClient wrapper, you also have to explicitly check whether the task is cancelled throughout the execution. I looked at a handful of the changes linked from the OP and I didn't see any that did either of these things.

[droberts195]

but if these things are heavy enough to need to be cancellable then they should also not be running on SAME (i.e. a transport worker

Is that true even if the processing on SAME is to immediately call another async API? Many of these APIs will start off by doing a search. Even though the search may take a while, initiating it should be fast. And then the thread that initiated the search should be freed up again.

Also it's not enough to just add a RestCancellableNodeClient wrapper, you also have to explicitly check whether the task is cancelled throughout the execution.

If a task is cancelled does it automatically cancel its child tasks?

The idea behind a lot of these changes was that they boil down to a series of searches. So if a parent task that's cancelled automatically cancels its child tasks, and the search action explicitly checks whether it's cancelled throughout its execution, then this change would tie the dropping of the HTTP connection that called the ML endpoint to the cancellation functionality that already exists in search.

It's possible that we do still have a problem. I guess a cancelled task results in the onFailure handler of listeners being called? But there are a few places in the ML actions where we proceed to the next action in a chain of actions even if the onFailure handler of one action is called. In these cases we probably need to be checking whether the task is cancelled before proceeding to the next (now pointless) action in the chain and just clean up instead.

[DaveCTurner]

Is that true even if the processing on SAME is to immediately call another async API?

It Depends™ - often that's not going to cause noticeable problems, but there can still be nontrivial work involved within the initial phases of the next API called (which also doesn't fork if called via NodeClient, because #97916). I think the reasoning should be the other way round: we should reserve SAME just for those cases where the overhead of forking is quantifiably too much, and by default we should always pick a different pool.

If a task is cancelled does it automatically cancel its child tasks?

Ah good point, yes, as long as those tasks are explicitly registered as its child tasks. Which in many cases they are.

I came here from #98333 which definitely has both of these problems tho.

[akagra]

Hey,
I have started contributing to open source recently and I can start on this part as this problem contains "good first issue" tag. Should I start on this problem?

[akagra]

Hey @droberts195 shall I work on some of its feature? I am first time contributor and looking for a gate to enter into the open source contribution.

[vimalJD]

Is this project is ongoing? Let me know further (DOCS) to contribute on it as "good first issue".