Make INLINESTATS (and subplans) work with CCS#134323
Make INLINESTATS (and subplans) work with CCS#134323smalyshev merged 12 commits intoelastic:mainfrom
Conversation
|
Hi @smalyshev, I've created a changelog YAML for you. |
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlExecutionInfo.java
Show resolved
Hide resolved
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
|
Pinging @elastic/es-search-foundations (Team:Search Foundations) |
astefan
left a comment
There was a problem hiding this comment.
LGTM
Left some questions and small items, nothing major.
...ternalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterAsyncQueryStopIT.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| // What happens here is when we run stop on subquery, the main query will still finish. | ||
| // TODO: should we have a way to stop the main query as well? |
There was a problem hiding this comment.
This is an interesting question.
I've looked at the other tests in this class and, to me, this looks like we are already doing the right thing in this test, meaning if the query is stopped while the remote is running, the "rest" of the query (the one that takes the sub-query results, puts them in the original query and then continues) is running until it finishes. And this is in line with what happens in test method testStopQuery in this class.
By this question you imply that when we stop the query while the remote sub-query runs, we should also stop the "rest" of the query?
OR you are referring to stopping the query AFTER the remote successfully finished (and it correctly and fully computed total value), meaning during the final execution? In which case, the results will be incomplete, but with a total value attached to them.
There was a problem hiding this comment.
I've created a separate JIRA for this, and if it needs adjustments, we'll handle it in separate pull.
By this question you imply that when we stop the query while the remote sub-query runs, we should also stop the "rest" of the query?
I think this would be the best way to go, but we need to discuss it properly and make a decision either way. Here I'm just noting this is an open question which we don't solve with this pull.
| @@ -24,6 +24,7 @@ | |||
| import java.util.Map; | |||
There was a problem hiding this comment.
I found we have a similar test for ENRICH in CrossClusterAsyncEnrichStopIT, but is there any stopping query IT test anywhere that uses a lookup join command?
There was a problem hiding this comment.
Not sure, but Lookup Join doesn't do anything really special when executing AFAIK, so not sure if it will give us more.
...ternalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterAsyncQueryStopIT.java
Show resolved
Hide resolved
* Make INLINESTATS (and subplans) work with CCS
* Make INLINESTATS (and subplans) work with CCS
INLINESTATSworks by running query subplans, which are exactly like regular query plans, except at the end of it the query does not end but moves on to another subplan - or to the main plan.In order to make it work with CCS, we need to:
Closes #124748