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

Add OperatorSpillContext #7741

Merged
merged 32 commits into from Aug 1, 2023
Merged

Add OperatorSpillContext #7741

merged 32 commits into from Aug 1, 2023

Conversation

windtalker
Copy link
Contributor

@windtalker windtalker commented Jul 4, 2023 •
edited

What problem does this PR solve?

Issue Number: ref #7738

Problem Summary:

What is changed and how it works?

Code refine for #7738, the basic idea is to add OperatorSpillContext, and in the future, all OperatorSpillContext will be registered to some place(like MPPQueryTaskSet), in runtime, it will be used to automatically trigger spill if needed.
This pr

  • Add top level class OperatorSpillContext
  • Add AggSpillContext/SortSpillContext/HashJoinSpillContext
  • Adopt current agg/sort/join spill with SpillContext

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 do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 4, 2023
@ti-chi-bot ti-chi-bot bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 14, 2023
@windtalker windtalker changed the title [WIP] Add OperatorSpillContext and AggSpillContext Add OperatorSpillContext Jul 14, 2023
@ti-chi-bot ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 14, 2023
@SeaRise SeaRise self-requested a review July 14, 2023 03:28
SeaRise
SeaRise reviewed Jul 14, 2023
View reviewed changes
dbms/src/Interpreters/AggSpillContext.cpp Outdated Show resolved Hide resolved
dbms/src/Interpreters/Aggregator.cpp Outdated Show resolved Hide resolved
dbms/src/Interpreters/Aggregator.cpp Outdated Show resolved Hide resolved
dbms/src/Interpreters/AggSpillContext.cpp Outdated Show resolved Hide resolved
@windtalker windtalker requested a review from SeaRise July 17, 2023 10:15
@windtalker
Copy link
Contributor Author

/run-all-tests

SeaRise
SeaRise reviewed Jul 18, 2023
View reviewed changes
dbms/src/DataStreams/MergeSortingBlockInputStream.cpp Outdated Show resolved Hide resolved
dbms/src/Interpreters/Aggregator.cpp Outdated Show resolved Hide resolved
dbms/src/Interpreters/HashJoinSpillContext.cpp Outdated Show resolved Hide resolved
@windtalker
Copy link
Contributor Author

/run-all-tests

@windtalker
Copy link
Contributor Author

/run-unit-test

@windtalker windtalker requested a review from SeaRise July 19, 2023 04:03
@windtalker
Copy link
Contributor Author

/run-all-tests

1 similar comment
@windtalker
Copy link
Contributor Author

/run-all-tests

@SeaRise SeaRise self-requested a review July 19, 2023 06:15
SeaRise
SeaRise reviewed Jul 19, 2023
View reviewed changes
dbms/src/Interpreters/Join.cpp Outdated Show resolved Hide resolved
dbms/src/Interpreters/Join.cpp Outdated Show resolved Hide resolved
dbms/src/Interpreters/Join.cpp Outdated Show resolved Hide resolved
dbms/src/Interpreters/Join.cpp Outdated Show resolved Hide resolved
@windtalker
Copy link
Contributor Author

/run-all-tests

@windtalker
Copy link
Contributor Author

/run-unit-test

windtalker and others added 20 commits July 31, 2023 11:27
@windtalker
add ut
65faa3c 
Signed-off-by: xufei <xufei@pingcap.com>
@windtalker
save work
bd2200a 
Signed-off-by: xufei <xufei@pingcap.com>
@windtalker
refine
dd2ce25 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
fix build
fed49ef 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
save work
e6eba92 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
address comments
998dd12 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
save work
efc59b2 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
save work
a1ee3c1 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
make ut pass
f46a502 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
save work
ccf255e 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
address comments
f3df0c0 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
fix ut
48b9801 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
address comments
f868a02 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
fix build and address comments
4b92c9e 
Signed-off-by: xufei <xufei@pingcap.com>
@windtalker
refine
f4ca950 
Signed-off-by: xufei <xufei@pingcap.com>
@windtalker
refine
8cc1fc7 
Signed-off-by: xufei <xufei@pingcap.com>
@windtalker
fix
897145c 
Signed-off-by: xufei <xufei@pingcap.com>
@windtalker
refine
f06171f 
Signed-off-by: xufei <xufei@pingcap.com>
@windtalker
fix tests
4a28eca 
Signed-off-by: xufei <xufei@pingcap.com>
@windtalker
fix build
7ad9308 
Signed-off-by: xufei <xufei@pingcap.com>
@windtalker
Copy link
Contributor Author

/run-all-tests

@windtalker
Copy link
Contributor Author

/hold cancel

@ti-chi-bot ti-chi-bot bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 1, 2023
@windtalker
Copy link
Contributor Author

/run-all-tests

@ti-chi-bot ti-chi-bot bot merged commit 5de003a into pingcap:master Aug 1, 2023
6 checks passed
@windtalker windtalker deleted the add_operator_spill_context branch August 1, 2023 09:23
@SeaRise SeaRise mentioned this pull request Aug 2, 2023
Closed
@windtalker windtalker mentioned this pull request Aug 7, 2023
Merged
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SeaRise SeaRise SeaRise approved these changes

@yibin87 yibin87 yibin87 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 @SeaRise @yibin87