-
Notifications
You must be signed in to change notification settings - Fork 2.3k
[BugFix] Fix result queue capacity of result sink #59153
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
Signed-off-by: zihe.liu <ziheliu1024@gmail.com>
[Java-Extensions Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[FE Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[BE Incremental Coverage Report]✅ pass : 3 / 3 (100.00%) file detail
|
alvin-celerdata
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
Could you elaborate a little more on the influence of query in the commit message? Will the performance be influenced?
I'm not very sure about the consequence of one slot in the buffer.
|
@alvin-celerdata Added a performance test in the description. |
|
@Mergifyio backport branch-3.3 |
|
@Mergifyio backport branch-3.5 |
|
@Mergifyio backport branch-3.4 |
✅ Backports have been createdDetails
|
✅ Backports have been createdDetails
|
✅ Backports have been createdDetails
|
Signed-off-by: zihe.liu <ziheliu1024@gmail.com> (cherry picked from commit 0ccc306) # Conflicts: # be/src/exec/data_sink.cpp
Signed-off-by: zihe.liu <ziheliu1024@gmail.com> (cherry picked from commit 0ccc306)
Signed-off-by: zihe.liu <ziheliu1024@gmail.com> (cherry picked from commit 0ccc306)
Signed-off-by: zihe.liu <ziheliu1024@gmail.com> Signed-off-by: AntiTopQuark <AntiTopQuark1350@outlook.com>
Why I'm doing:
Introduced by #30386.
The result queue capacity is
dop*2.However, this dop is get by
state->query_options().pipeline_dop, which is the raw value of the session variablepipeline_dop(0 by default).This results that the queue capacity is 0, which means the queue will be full after putting one result batch. And therefore, result sink can only buffer one chunk.
Test
16Core 64GB Memory BE.
ResultDeliverTime:
ResultDeliverTime is decreased, but the total execution time between these two versions are the same.
What I'm doing:
Use actual DOP to set queue capacity.
Fixes #issue
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check: