executor: control Chunk size for Selection&Projection #9398

qw4990 · 2019-02-21T06:45:10Z

What problem does this PR solve?

Control the number of rows in chunks returned by Selection & Projection.
Following up #9364, this PR is a subtask of #9166.

What is changed and how it works?

Make Selection support chunk size control.
Make Projection support chunk size control.
Add unit tests.

Check List

Tests

Unit test

winoros · 2019-02-21T06:54:00Z

executor/projection.go

@@ -166,6 +166,8 @@ func (e *ProjectionExec) isUnparallelExec() bool {
 }

 func (e *ProjectionExec) unParallelExecute(ctx context.Context, chk *chunk.Chunk) error {
+	// transmit the requiredRows
+	e.childResult.SetRequiredRows(chk.RequiredRows(), e.maxChunkSize)


Selection's childResult needs the same change?

Why not?
It makes the child return as quickly as possible and also guarantees enough rows to return to Projection's parent.

codecov-io · 2019-02-21T06:57:38Z

Codecov Report

Merging #9398 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #9398      +/-   ##
==========================================
+ Coverage    67.3%   67.31%   +0.01%     
==========================================
  Files         373      373              
  Lines       78442    78448       +6     
==========================================
+ Hits        52796    52809      +13     
+ Misses      20909    20905       -4     
+ Partials     4737     4734       -3

Impacted Files	Coverage Δ
executor/projection.go	`76.76% <100%> (+1.02%)`	⬆️
executor/executor.go	`68.16% <100%> (+0.27%)`	⬆️
util/systimemon/systime_mon.go	`80% <0%> (-20%)`	⬇️
executor/index_lookup_join.go	`77.42% <0%> (-0.63%)`	⬇️
executor/join.go	`79.58% <0%> (+0.51%)`	⬆️
ddl/delete_range.go	`79.36% <0%> (+4.23%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9afbda9...5303a60. Read the comment docs.

qw4990 · 2019-02-21T07:13:09Z

Projection becomes a pipeline breaker when it in parallel mode, and according to its implementation, it's hard to support chunk size control in this mode, so I only make it work in unparallel mode.

Selection only supports chunk size control upward to its parent now.
While calling the child's Next, it can't determine the RequiredRows because the selectivity is unknown.
If we have a estimated selectivity, maybe we can set RequiredRows of child to (RequiredRows from parent) / selectivity?

How do you think?

lysu · 2019-02-21T09:43:30Z

Projection becomes a pipeline breaker when it in parallel mode, and according to its implementation, it's useless to support chunk size control in this mode, so I only make it work in unparallel mode.

but in parallel mode, projection's child executor still get benefits from known "Require Not More than this info"?

qw4990 · 2019-02-21T11:02:59Z

Projection becomes a pipeline breaker when it in parallel mode, and according to its implementation, it's useless to support chunk size control in this mode, so I only make it work in unparallel mode.

but in parallel mode, projection's child executor still get benefits from known "Require Not More than this info"?

Yes, but in parallel mode, all worker goroutines work in the background and Projection fetches data from its child asynchronously, so it's hard to determine the value of RequiredRows passed to child.

qw4990 · 2019-02-22T06:32:07Z

Projection in parallel mode supports chunk size control now.
I introduce requiredRowsNow into Projection and update it when each call of Next.
Then the background fetcher uses this value as its own chunks' RequiredRows while fetching data from child.

PTAL~ @lysu

executor/executor.go

executor/projection.go

zz-jason

LGTM

zz-jason · 2019-02-25T09:05:17Z

/run-all-tests

qw4990 · 2019-02-25T12:09:46Z

/run-all-tests

qw4990 · 2019-02-25T12:41:37Z

/run-all-tests

qw4990 · 2019-02-25T12:59:06Z

/run-all-tests

qw4990 · 2019-02-26T02:39:56Z

/run-all-tests

alivxxx

LGTM

Unit test failed

qw4990 · 2019-02-26T03:02:40Z

/run-all-tests

qw4990 · 2019-02-26T03:19:44Z

/run-all-tests

cwen0 · 2019-02-26T03:27:56Z

/rebuild

cwen0 · 2019-02-26T03:36:26Z

/run-unit-test

qw4990 · 2019-02-26T05:41:35Z

/run-all-tests

qw4990 · 2019-02-26T05:53:52Z

/run-all-tests

alivxxx

LGTM

qw4990 added the sig/execution SIG execution label Feb 21, 2019

qw4990 requested review from lysu and zz-jason February 21, 2019 06:45

zz-jason changed the title ~~make Selection and Projection support chunk size control~~ executormake Selection and Projection support chunk size control Feb 21, 2019

zz-jason changed the title ~~executormake Selection and Projection support chunk size control~~ executor: control Chunk size for Selection&Projection Feb 21, 2019

winoros reviewed Feb 21, 2019

View reviewed changes

zz-jason reviewed Feb 22, 2019

View reviewed changes

executor/executor.go Show resolved Hide resolved

zz-jason reviewed Feb 22, 2019

View reviewed changes

executor/projection.go Show resolved Hide resolved

zz-jason reviewed Feb 22, 2019

View reviewed changes

executor/projection.go Outdated Show resolved Hide resolved

executor/projection.go Outdated Show resolved Hide resolved

qw4990 force-pushed the chunkSizeControl_Sel/Proj branch from 9249589 to f67fa59 Compare February 24, 2019 12:51

alivxxx reviewed Feb 25, 2019

View reviewed changes

executor/projection.go Outdated Show resolved Hide resolved

zz-jason reviewed Feb 25, 2019

View reviewed changes

zz-jason added the status/LGT1 Indicates that a PR has LGTM 1. label Feb 25, 2019

zz-jason requested a review from alivxxx February 25, 2019 09:05

qw4990 mentioned this pull request Feb 25, 2019

executor: control Chunk size for TableReader&IndexReader&IndexLookup #9452

Merged

qw4990 force-pushed the chunkSizeControl_Sel/Proj branch from 725cb81 to 102f98c Compare February 25, 2019 12:41

qw4990 force-pushed the chunkSizeControl_Sel/Proj branch from 102f98c to 47e0f8e Compare February 25, 2019 12:53

qw4990 added 4 commits February 26, 2019 10:39

make Selection and Projection support chunk size control

5223a0f

make Projection support chunk size control on parallel mode

0948a61

update comments

3be44fe

address comments

d66de4c

qw4990 added 2 commits February 26, 2019 10:39

update comments

17c0c83

fix CI

5294f93

qw4990 force-pushed the chunkSizeControl_Sel/Proj branch from 47e0f8e to 5294f93 Compare February 26, 2019 02:39

alivxxx previously approved these changes Feb 26, 2019

View reviewed changes

alivxxx added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Feb 26, 2019

fix CI

100e44c

fix CI problem

fa7ee0e

fix CI problem

1bb767c

alivxxx approved these changes Feb 26, 2019

View reviewed changes

Merge branch 'master' into chunkSizeControl_Sel/Proj

5303a60

alivxxx added the status/all tests passed label Feb 26, 2019

qw4990 merged commit 3e0f51b into pingcap:master Feb 26, 2019

qw4990 mentioned this pull request Apr 10, 2019

executor: control Chunk size for Selection&Projection #10110

Merged

qw4990 added a commit to qw4990/tidb that referenced this pull request Apr 11, 2019

executor: control Chunk size for Selection&Projection (pingcap#9398)

ccd9b51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

executor: control Chunk size for Selection&Projection #9398

executor: control Chunk size for Selection&Projection #9398

qw4990 commented Feb 21, 2019 •

edited

Loading

winoros Feb 21, 2019

qw4990 Feb 21, 2019

codecov-io commented Feb 21, 2019 •

edited

Loading

qw4990 commented Feb 21, 2019 •

edited

Loading

lysu commented Feb 21, 2019

qw4990 commented Feb 21, 2019

qw4990 commented Feb 22, 2019

zz-jason left a comment

zz-jason commented Feb 25, 2019

qw4990 commented Feb 25, 2019

qw4990 commented Feb 25, 2019

qw4990 commented Feb 25, 2019

qw4990 commented Feb 26, 2019

alivxxx left a comment

qw4990 commented Feb 26, 2019

qw4990 commented Feb 26, 2019

cwen0 commented Feb 26, 2019

cwen0 commented Feb 26, 2019

qw4990 commented Feb 26, 2019

qw4990 commented Feb 26, 2019

alivxxx left a comment

executor: control Chunk size for Selection&Projection #9398

executor: control Chunk size for Selection&Projection #9398

Conversation

qw4990 commented Feb 21, 2019 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

winoros Feb 21, 2019

Choose a reason for hiding this comment

qw4990 Feb 21, 2019

Choose a reason for hiding this comment

codecov-io commented Feb 21, 2019 • edited Loading

Codecov Report

qw4990 commented Feb 21, 2019 • edited Loading

lysu commented Feb 21, 2019

qw4990 commented Feb 21, 2019

qw4990 commented Feb 22, 2019

zz-jason left a comment

Choose a reason for hiding this comment

zz-jason commented Feb 25, 2019

qw4990 commented Feb 25, 2019

qw4990 commented Feb 25, 2019

qw4990 commented Feb 25, 2019

qw4990 commented Feb 26, 2019

alivxxx left a comment

Choose a reason for hiding this comment

qw4990 commented Feb 26, 2019

qw4990 commented Feb 26, 2019

cwen0 commented Feb 26, 2019

cwen0 commented Feb 26, 2019

qw4990 commented Feb 26, 2019

qw4990 commented Feb 26, 2019

alivxxx left a comment

Choose a reason for hiding this comment

qw4990 commented Feb 21, 2019 •

edited

Loading

codecov-io commented Feb 21, 2019 •

edited

Loading

qw4990 commented Feb 21, 2019 •

edited

Loading