ES|QL: label drivers in profile info for FORK #128318
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ioanatia
added
>non-issue
Team:Analytics
Collaborator
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
23 tasks
nik9000
approved these changes
May 22, 2025
Member
nik9000
left a comment
nik9000
left a comment
There was a problem hiding this comment.
For sure an improvement! I'm not sure what you mean about sending physical plans. I imagine you have to send the qualifier to the data nodes regardless of what sort of plan you send, but y'all know your business better than I do.
idegtiarenko
approved these changes
May 23, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When we added streaming support for FORK, one piece of feedback was that we should check how we expose the profile info when executing FORK and add a test for it: #126705 (review)
I added labels for the drivers that get executed on the coordinator.
The
dataandnode_reduceones are not labelled with the sub plan number - I was thinking of changing that (although it's a bit more involved), but then remembered from #126705 that we might want to send the physical plans for the FORK sub plans in a single request. So I wasn't sure if I really needed to proceed, since we might change theDataNodeComputeHandlerand left it as it is for now.Anyway, we got profile info for all the FORK sub plans and a test to catch any regressions.