EQL: Adds an ability to start an asynchronous EQL search #56631
Conversation
Adds support for async searches to eql search API. This commit is limited to only submitting search API requests and doesn't provide APIs to get results nor delete the results. These functions will be added in follow up PRs. Relates to elastic#49638
imotov
added
>enhancement
:Analytics/EQL
|
Pinging @elastic/es-ql (:Query Languages/EQL) |
|
@elasticmachine run elasticsearch-ci/default-distro |
| @@ -46,6 +46,8 @@ | |||
| import java.util.function.Supplier; | |||
|
|
|||
| public class EqlPlugin extends Plugin implements ActionPlugin { | |||
| // We are going to reuse the same index as normal async search until system indices are implemented | |||
| public static final String INDEX = ".async-search"; | |||
There was a problem hiding this comment.
We should share this in core next to AsyncTaskMaintenanceService ?
I also wonder who should be responsible to start the AsyncTaskMaintenanceService since we don't need to start it twice ?
There was a problem hiding this comment.
Yes. Good point. Let's deal with it in the next PR when I start moving stuff around.
| if (request.hasParam("keep_alive")) { | ||
| eqlRequest.keepAlive(request.paramAsTime("keep_alive", eqlRequest.keepAlive())); | ||
| } | ||
| eqlRequest.keepOnCompletion(request.paramAsBoolean("keep_on_completion", eqlRequest.keepOnCompletion())); | ||
| } | ||
|
|
||
| return channel -> client.execute(EqlSearchAction.INSTANCE, eqlRequest, new RestResponseListener<>(channel) { |
There was a problem hiding this comment.
You should use RestCancellableNodeClient in order to get the automatic cancellation if the user closes the rest channel ?
There was a problem hiding this comment.
Good catch. I am going to open PR against master for that in a bit since it's not async execution-specific.
There was a problem hiding this comment.
Hmm, I need AbstractEqlBlockingIntegTestCase to do IT testing for it and it only exists in this branch for now. So, I think I will open a separate PR but against this branch.
| private final SecurityContext securityContext; | ||
| private final ClusterService clusterService; | ||
| private final PlanExecutor planExecutor; | ||
| private final ThreadPool threadPool; | ||
| private final AsyncTaskManagementService<EqlSearchRequest, EqlSearchResponse, EqlSearchTask> asyncTaskManagementService; | ||
|
|
||
| @Inject | ||
| public TransportEqlSearchAction(Settings settings, ClusterService clusterService, TransportService transportService, |
There was a problem hiding this comment.
PlanExecutor already has a Client and NamedWriteableRegistry injected - unless they are different, you could get them from there.
There was a problem hiding this comment.
They are the same, I just feel like having client as an explicit dependency is a bit cleaner. Otherwise, I would need to reach into PlanExecutor and possibly mock PlanExecutor in AsyncTaskManagementServiceTests. I also don't feel like providing access to client and writableRegistry shouldn't be a PlainExecutor's concern. Especially considering that writableRegistry is not even used by PlainExecutor at the moment.
Adds support for async searches to eql search API. This commit is limited to
only submitting search API requests and doesn't provide APIs to get results
nor delete the results. These functions will be added in follow up PRs.
Relates to #49638
Supersedes #56147