-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Make INLINESTATS (and subplans) work with CCS #134323
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
|
|
||
| import java.util.Iterator; | ||
| import java.util.List; | ||
| import java.util.Map; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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