Skip to content
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.

Sign up for GitHub

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

Jump to bottom

support Sort operator auto spill when query memory usage exceed threshold #7900

Merged
merged 18 commits into from Aug 11, 2023
Merged

support Sort operator auto spill when query memory usage exceed threshold #7900

merged 18 commits into from Aug 11, 2023

Conversation

windtalker
Copy link
Contributor

@windtalker windtalker commented Aug 4, 2023 •
edited

What problem does this PR solve?

Issue Number: ref #7738

Problem Summary:

What is changed and how it works?

Support sort operator auto spill when query memory usage exceeds the threshold. The basic idea is

  • MPPQuery will collect all OperatorSpillContext that support auto spill(currently, only SortSpillContext)
  • During complile time, a callback AutoSpillTrigger will be set to the executor(support both pull model and push model)
  • During runtime, for each iterator of each executor, it will call AutoSpillTrigger if current query memory usage exeeed the threshold
  • In AutoSpillTrigger, it check and mark operator to spill if possible
  • Each operator will be informed to spill in UpdateRevocableMemoryUsage, and begin to spill if needed

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Documentation

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

None

@ti-chi-bot ti-chi-bot bot added release-note-none size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Aug 4, 2023
@windtalker windtalker changed the title Sort auto spill support Sort operator auto spill when query memory usage exceed threshold Aug 4, 2023
@SeaRise SeaRise self-requested a review August 4, 2023 09:54
@SeaRise SeaRise self-requested a review August 7, 2023 05:51
SeaRise
SeaRise reviewed Aug 7, 2023
View reviewed changes
dbms/src/Flash/Coprocessor/DAGContext.h Outdated Show resolved Hide resolved
dbms/src/Flash/executeQuery.cpp Outdated Show resolved Hide resolved
@windtalker windtalker requested a review from SeaRise August 8, 2023 09:39
@windtalker
Copy link
Contributor Author

/run-all-tests

1 similar comment
@windtalker
Copy link
Contributor Author

/run-all-tests

@windtalker
Copy link
Contributor Author

/run-unit-test

SeaRise
SeaRise reviewed Aug 10, 2023
View reviewed changes
dbms/src/Core/AutoSpillTrigger.h Outdated Show resolved Hide resolved
dbms/src/Core/AutoSpillTrigger.h Show resolved Hide resolved
dbms/src/Core/TaskOperatorSpillContexts.h Show resolved Hide resolved
dbms/src/Core/QueryOperatorSpillContexts.h Outdated Show resolved Hide resolved
dbms/src/Core/TaskOperatorSpillContexts.h Outdated Show resolved Hide resolved
dbms/src/Interpreters/SortSpillContext.cpp Show resolved Hide resolved
SeaRise
SeaRise reviewed Aug 10, 2023
View reviewed changes
dbms/src/Core/AutoSpillTrigger.h Show resolved Hide resolved
dbms/src/Core/TaskOperatorSpillContexts.h Outdated Show resolved Hide resolved
dbms/src/Core/QueryOperatorSpillContexts.h Outdated Show resolved Hide resolved
@SeaRise SeaRise self-requested a review August 10, 2023 08:40
SeaRise
SeaRise reviewed Aug 10, 2023
View reviewed changes
Copy link
Contributor

@SeaRise SeaRise left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost LGTM

@ti-chi-bot ti-chi-bot bot added needs-1-more-lgtm approved needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Aug 10, 2023
@ti-chi-bot ti-chi-bot bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 11, 2023
@ti-chi-bot ti-chi-bot bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 11, 2023
yibin87
yibin87 approved these changes Aug 11, 2023
View reviewed changes
Copy link
Contributor

@yibin87 yibin87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Aug 11, 2023

[LGTM Timeline notifier]

Timeline:

  • 2023-08-10 10:00:46.848877105 +0000 UTC m=+193211.397893092: ☑️ agreed by SeaRise.
  • 2023-08-11 05:55:22.57163954 +0000 UTC m=+264887.120655512: ☑️ agreed by yibin87.

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Aug 11, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SeaRise, yibin87

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

windtalker and others added 18 commits August 11, 2023 14:04
@windtalker
support sort auto spill
7dfd890 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
save work
d5a511d 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
save work
2540da1 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
save work
fe93f37 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
save work
100ecd9 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
add ut
618e039 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
rename operatorspillcontexts to mpptaskoperatorspillcontexts
8467c5d 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
save work
2ea7ce1 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
address comments and add more tests
cca4f97 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
refine
ee21d5f 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
address comments
b447751 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
format
97f7b67 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker @SeaRise
Update dbms/src/Core/TaskOperatorSpillContexts.h
d9e96df 
Co-authored-by: SeaRise <hhssearise@foxmail.com>
@windtalker
add more log
d49d68c 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
refine
89bb012 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
fix bug
7e1b93c 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
add more ut
4aadbfd 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
format code
7496e48 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@ti-chi-bot ti-chi-bot bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 11, 2023
@windtalker
Copy link
Contributor Author

/run-all-tests

@ti-chi-bot ti-chi-bot bot merged commit a220251 into pingcap:master Aug 11, 2023
6 checks passed
@windtalker windtalker deleted the sort_auto_spill branch August 11, 2023 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yibin87 yibin87 yibin87 approved these changes

@SeaRise SeaRise SeaRise approved these changes

Assignees
No one assigned
Labels
approved lgtm release-note-none size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@windtalker @yibin87 @SeaRise