ESQL: List/get query API #124832

GalLalouche · 2025-03-13T21:48:56Z

This PR adds two new REST endpoints, for listing queries and getting information on a current query.

Resolves ESQL: API for listing running queries #124827
Related to ESQL: Create an API for getting information for a specific running query #124828 (initial work)

Changes from the API specified in the above issues:

The get API is pretty initial, as we don't have a way of fetching the memory used or number of rows processed.

List queries response:

GET /_query/queries
// returns for each of the running queries
// query_id, start_time, running_time, query

{ "queries" : {
 "abc:1234": {
  "node": "abc",
  "id": 1234,
  "start_time_millis": 14585858875292,
  "running_time_nanos": 762794,
  "query": "FROM logs* | STATS BY hostname"
  },
 "abd:1729": {
  "node": "abd",
  "id":1729,
  "start_time_millis": 14585858823573,
  "running_time_nanos": 90231,
  "query": "FROM orders | LOOKUP country_code ON country"
  }
 } 
}

Get query response:

GET /_query/queries/abc:1234

{
  "id": 1234,
  "node": "abc",
  "start_time_millis": 14585858875292,
  "running_time_nanos": 762794,
  "query": "FROM logs* | STATS BY hostname"
  "coordinating_node": "oTUltX4IQMOUUVeiohTt8A"
  "data_nodes" : [ "DwrYwfytxthse49X4", "i5msnbUyWlpe86e7"]
}

elasticsearchmachine · 2025-03-18T16:12:55Z

Hi @GalLalouche, I've created a changelog YAML for you.

elasticsearchmachine · 2025-03-19T09:33:49Z

Pinging @elastic/es-analytical-engine (Team:Analytics)

idegtiarenko · 2025-03-19T12:28:56Z

...plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RestEsqlListQueriesAction.java

+
+    @Override
+    public List<Route> routes() {
+        return List.of(new Route(GET, "/_query/queries/{id}"), new Route(GET, "/_query/queries"));


The repetition in the endpoint looks surprising to me. Why not /_queries or may be /_cluster/queries?

For the time being we're looking to keep the ESQL APIs under the ESQL prefix(_query) and avoid potential clashes with other rest APIs.
The decision hasn't been finalized though.

Let's sort this one out after this PR.

We can move this - we haven't finalized this API.

...k/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlListQueriesResponse.java

costin

Good first PR.
Add a yaml integration rest test with that checks the output for each endpoint, including working and failing tasks

Moving forward, please create a meta PR for this for follow-up issues:

query cancellation
filtering (my doc didn't include any, let's use the task API ones by filtering over nodes and id)
have a subtask to integrate the user in
double check that X-Opaque-Id /trace.id information is sent over (and displayed if available) - could be used for filtering also
follow-up item for memory tracking
follow-up item for bytes / records read

The meta issue will better reflect the overall status including tasks that are blocking and incoming feedback and keep the code get merged in.

costin · 2025-03-20T00:52:47Z

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlGetQueryAction.java

+
+public class EsqlGetQueryAction extends ActionType<EsqlGetQueryResponse> {
+    public static final EsqlGetQueryAction INSTANCE = new EsqlGetQueryAction();
+    public static final String NAME = "cluster:monitor/xpack/esql/get_queries";


As a follow-up, please reach out to the data store / security team to double check what type of security is needed (similar to cat/tasks API) both for listing and cancelling queries.
There might a need to add another permission or reuse an existing one.

costin · 2025-03-20T01:01:13Z

...plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RestEsqlListQueriesAction.java

+        return id != null
+            ? (channel -> client.execute(
+                EsqlGetQueryAction.INSTANCE,
+                new EsqlGetQueryRequest(new TaskId(id)),
+                new RestToXContentListener<>(channel)
+            ))
+            : (channel -> client.execute(
+                EsqlListQueriesAction.INSTANCE,
+                new EsqlListQueriesRequest(),
+                new RestToXContentListener<>(channel)
+            ));
+    }


An alternative is to move the conditional when creating the parameters and have only one consumer creation:

ActionType<?> action = id != null ? RestEsqlGetQueryAction.INSTANCE : RestEsqlListQueriesAction.INSTANCE; Object request = id != null ? new EsqlGetQueryRequest(new TaskId(id)) : new EsqlListQueriesRequest(); return channel -> client.execute( action, request, new RestToXContentListener<>(channel) );

costin · 2025-03-20T01:06:31Z

...plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RestEsqlListQueriesAction.java

+
+    @Override
+    public List<Route> routes() {
+        return List.of(new Route(GET, "/_query/queries/{id}"), new Route(GET, "/_query/queries"));


For the time being we're looking to keep the ESQL APIs under the ESQL prefix(_query) and avoid potential clashes with other rest APIs.
The decision hasn't been finalized though.

…ture/list_query_api

idegtiarenko · 2025-03-24T12:37:39Z

...ck/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlListQueriesRequest.java

+
+import java.io.IOException;
+
+public class EsqlListQueriesRequest extends ActionRequest {


I wonder if we should rely on org.elasticsearch.transport.EmptyRequest instead?

It looks like that's a test class... is it used in production?

nik9000 · 2025-03-24T19:24:08Z

test/framework/src/main/java/org/elasticsearch/test/IntegerMatcher.java

+/**
+ * A type-agnostic way of comparing integer values, not caring if it's a long or an integer.
+ */
+public abstract sealed class IntegerMatcher<T> extends BaseMatcher<T> {


Maybe IntOrLongMatcher.

nik9000 · 2025-03-24T19:25:14Z

...ugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java

+
+    public void testListApi_runningQuery_returnsQueriesObject() throws Exception {
+        bulkLoadTestData(1);
+        String query = fromIndex() + " | keep keyword, integer | where delay(10s) | limit 100 ";


Could you move this one to the PausableFieldPlugin stuff?

nik9000 · 2025-03-24T19:26:53Z

.../internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java

@@ -55,7 +55,7 @@ public void ensureExchangesAreReleased() throws Exception {
            ExchangeService exchangeService = esqlQueryAction.exchangeService();
            assertBusy(() -> {
                if (exchangeService.lifecycleState() == Lifecycle.State.STARTED) {
-                    assertTrue("Leftover exchanges " + exchangeService + " on node " + node, exchangeService.isEmpty());
+                    assertTrue("Leftover exchanges " + exchangeService + " on taskId " + node, exchangeService.isEmpty());


I think this is on the node, right?

Yes. At some point I renamed a field called node and intellij was kind enough to replace all "node" words with "taskId" in the entire repo. I thought I caught all of them but I guess a few slipped through.

nik9000 · 2025-03-24T19:27:34Z

...c/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlListQuerriesActionIT.java

+        String id = null;
+        try (var initialResponse = sendAsyncQuery()) {
+            id = initialResponse.asyncExecutionId().get();
+            // FIXME(gal, NOCOMMIT) more copy paste


Yes, but honest it's not worth the refactor in this case.

nik9000 · 2025-03-24T19:28:32Z

...c/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlListQuerriesActionIT.java

+        }
+    }
+
+    // FIXME(gal, NOCOMMIT) copy paste


More leftover?

Yes, but honest it's not worth the refactor in this case.

nik9000 · 2025-03-24T19:29:30Z

...c/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlListQuerriesActionIT.java

+        return EsqlQueryRequestBuilder.newAsyncEsqlQueryRequestBuilder(client()).query(QUERY).execute().actionGet(60, TimeUnit.SECONDS);
+    }
+
+    // FIXME(gal, NOCOMMIT) copy pasted from another place


Leftover? I guess we can move this to RestTestCase.

Yep, that's what I did :)

nik9000 · 2025-03-24T19:33:41Z

...k/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlListQueriesResponse.java

+
+    public record Query(org.elasticsearch.tasks.TaskId taskId, long startTimeMillis, long runningTimeNanos, String query)
+        implements
+            ToXContentObject {


This one is a ToXContentFragment because it's not a valid value - it's a key and a value.

nik9000 · 2025-03-24T19:35:11Z

...n/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlListQueriesAction.java

+
+    @Override
+    protected void doExecute(Task task, EsqlListQueriesRequest request, ActionListener<EsqlListQueriesResponse> listener) {
+        // FIXME(gal, NOCOMMIT) The + [a] hack needs a better solution.


I guess we can make the [a] dude a constant wherever it's used.

GalLalouche · 2025-03-26T13:23:55Z

Good first PR. Add a yaml integration rest test with that checks the output for each endpoint, including working and failing tasks

Moving forward, please create a meta PR for this for follow-up issues:

query cancellation

filtering (my doc didn't include any, let's use the task API ones by filtering over nodes and id)

have a subtask to integrate the user in

double check that X-Opaque-Id /trace.id information is sent over (and displayed if available) - could be used for filtering also

follow-up item for memory tracking

follow-up item for bytes / records read

The meta issue will better reflect the overall status including tasks that are blocking and incoming feedback and keep the code get merged in.

Done: #125662

jfreden

Thanks for the review request and sorry for the delay!

My understanding is that there are two new actions EsqlGetQueryAction and EsqlListQueriesAction that use the TransportListTasksAction under the hood and filters either on id or on the type of query (esql). From a least privilege principle point of view I think the existing monitor privilege is too wide, since we get more than we need with it. Adding a monitor_esql would probably make sense, in the same way we did for these features.

I think that would require a new esql origin and then register it here, to execute the actual TransportListTasksAction without the API caller having the monitor privilege.

WDYT?

GalLalouche · 2025-04-01T13:27:57Z

Thanks @jfreden! I've added the necessary privileges by following the other ones in the classes you mentioned. Could you PTAL?

jfreden · 2025-04-01T13:36:40Z

...n/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlListQueriesAction.java

+        new ListTasksRequestBuilder(nodeClient).setActions(
+            EsqlQueryAction.NAME,
+            EsqlQueryAction.NAME + AsyncTaskManagementService.ASYNC_ACTION_SUFFIX
+        ).setDetailed(true).execute(new ActionListener<>() {


If the caller only has monitor_esql I think this would fail because it doesn't have cluster:monitor/tasks/lists (or cluster:monitor*) that's required here. To work around that the executeAsyncWithOrigin can be used. It would be nice with an integration test that covers this. Maybe modify existing to only have the monitor_esql privilege?

Nice catch! I fixed it and added a couple of integration tests.

jfreden · 2025-04-01T13:57:16Z

The new privilege should also be added to:

jfreden · 2025-04-03T07:05:28Z

...ugin/esql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/esql/EsqlSecurityIT.java

@@ -761,6 +764,36 @@ public void testFromLookupIndexForbidden() throws Exception {
        assertThat(resp.getResponse().getStatusLine().getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST));
    }

+    public void testListQueryAllowed() throws Exception {


docs/reference/elasticsearch/security-privileges.md

jfreden · 2025-04-03T07:13:48Z

...ugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlGetQueryAction.java

+            nodeClient,
+            ESQL_ORIGIN,
+            TransportGetTaskAction.TYPE,
+            new GetTaskRequest().setTaskId(request.id()),


Is it possible to validate that this is for EsqlQueryAction or an async EsqlQueryAction? Otherwise you might be able to get any task here? Or maybe I'm missing additional validation?

Not properly, since we would need said task to have started but not yet finished by the time we ask for it. There are other tests that check that it works, but they use a special hack to inject a semaphore in the middle of executing the query. IMO, combining both checks (semaphore + custom privileges) would be too much of a hassle.

I think we could check the type of the returned task a few lines down. So if you ask for a non-ESQL task it'd reject it.

jfreden

Thanks for implementing the suggested changes! LGTM!

nik9000 · 2025-04-08T13:07:51Z

...plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RestEsqlListQueriesAction.java

+
+    @Override
+    public List<Route> routes() {
+        return List.of(new Route(GET, "/_query/queries/{id}"), new Route(GET, "/_query/queries"));


Let's sort this one out after this PR.

nik9000 · 2025-04-08T13:08:05Z

...plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/RestEsqlListQueriesAction.java

+
+    @Override
+    public List<Route> routes() {
+        return List.of(new Route(GET, "/_query/queries/{id}"), new Route(GET, "/_query/queries"));


We can move this - we haven't finalized this API.

...k/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlListQueriesResponse.java

nik9000 · 2025-04-08T13:14:36Z

...ugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlGetQueryAction.java

+            nodeClient,
+            ESQL_ORIGIN,
+            TransportGetTaskAction.TYPE,
+            new GetTaskRequest().setTaskId(request.id()),


I think we could check the type of the returned task a few lines down. So if you ask for a non-ESQL task it'd reject it.

elasticsearchmachine added the v9.1.0 label Mar 13, 2025

kkrik-es mentioned this pull request Mar 14, 2025

[TEST] Unmute LogsdbIndexingRollingUpgradeIT tests #124851

Merged

GalLalouche force-pushed the feature/list_query_api branch from d554936 to 58ccb18 Compare March 17, 2025 10:17

GalLalouche added 3 commits March 18, 2025 15:55

ESQL: List/get query API

d6b0a92

Added super simple test

d961e27

More tests

55e7625

GalLalouche force-pushed the feature/list_query_api branch from 58ccb18 to be048dc Compare March 18, 2025 13:55

Add operators to constants, rename

8ad4d6d

GalLalouche force-pushed the feature/list_query_api branch from be048dc to 8ad4d6d Compare March 18, 2025 14:35

GalLalouche requested review from costin and nik9000 March 18, 2025 16:12

GalLalouche added >feature Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) :Analytics/ES|QL AKA ESQL labels Mar 18, 2025

GalLalouche marked this pull request as ready for review March 19, 2025 09:33

GalLalouche and others added 5 commits March 19, 2025 11:33

ESQL: List/get query API

fd08751

Added super simple test

f1aab5d

More tests

af2b5ee

Add operators to constants, rename

1d95cbc

Update docs/changelog/124832.yaml

cb4ec47

GalLalouche force-pushed the feature/list_query_api branch from 87d1040 to cb4ec47 Compare March 19, 2025 09:33

idegtiarenko reviewed Mar 19, 2025

View reviewed changes

...k/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlListQueriesResponse.java Show resolved Hide resolved

idegtiarenko approved these changes Mar 19, 2025

View reviewed changes

costin reviewed Mar 20, 2025

View reviewed changes

GalLalouche added 3 commits March 23, 2025 14:15

Merge branch 'main' into feature/list_query_api

2fa0085

Moved test

ff0fe90

Merge remote-tracking branch 'origin/feature/list_query_api' into fea…

758319a

…ture/list_query_api

idegtiarenko reviewed Mar 24, 2025

View reviewed changes

nik9000 reviewed Mar 24, 2025

View reviewed changes

TEMP

894dad6

GalLalouche added 3 commits March 27, 2025 11:08

Merge branch 'main' into feature/list_query_api

30f5ef1

More CR fixes

9bcdb10

Merge branch 'main' into feature/list_query_api

6686702

GalLalouche force-pushed the feature/list_query_api branch from 9a159a4 to 6686702 Compare March 27, 2025 18:33

GalLalouche added 2 commits March 27, 2025 23:59

Add null URL to rest API

9847add

Merge branch 'main' into feature/list_query_api

1d0d15c

GalLalouche requested review from costin and nik9000 March 30, 2025 09:25

jfreden reviewed Apr 1, 2025

View reviewed changes

GalLalouche added 2 commits April 1, 2025 15:47

Merge branch 'main' into feature/list_query_api

9fe9243

Update privileges

f786222

GalLalouche requested a review from a team as a code owner April 1, 2025 13:25

jfreden reviewed Apr 1, 2025

View reviewed changes

GalLalouche added 2 commits April 2, 2025 19:48

Security fixes

6134a8e

Merge branch 'main' into feature/list_query_api

789362f

jfreden reviewed Apr 3, 2025

View reviewed changes

GalLalouche requested a review from jfreden April 3, 2025 11:49

jfreden approved these changes Apr 3, 2025

View reviewed changes

nik9000 approved these changes Apr 8, 2025

View reviewed changes

GalLalouche added 2 commits April 8, 2025 20:27

CR fixes

0cedb9a

Merge branch 'main' into feature/list_query_api

09dc300

GalLalouche removed the request for review from costin April 8, 2025 17:35

GalLalouche merged commit 953b9fb into elastic:main Apr 8, 2025
16 of 17 checks passed

GalLalouche mentioned this pull request Apr 10, 2025

ESQL: Add query monitoring API elastic/elasticsearch-specification#4260

Merged


		import java.io.IOException;

		public class EsqlListQueriesRequest extends ActionRequest {

ESQL: List/get query API #124832

ESQL: List/get query API #124832

Conversation

GalLalouche commented Mar 13, 2025 • edited Loading

elasticsearchmachine commented Mar 18, 2025

elasticsearchmachine commented Mar 19, 2025

idegtiarenko Mar 19, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

costin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GalLalouche commented Mar 26, 2025

jfreden left a comment

Choose a reason for hiding this comment

GalLalouche commented Apr 1, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jfreden commented Apr 1, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jfreden left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GalLalouche commented Mar 13, 2025 •

edited

Loading

idegtiarenko Mar 19, 2025 •

edited

Loading

jfreden commented Apr 1, 2025 •

edited

Loading