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

executor: Chunk size control test for Limit+TableScan and Limit+IndexScan #9574

Merged
merged 12 commits into from
Mar 7, 2019
Merged

executor: Chunk size control test for Limit+TableScan and Limit+IndexScan #9574

merged 12 commits into from
Mar 7, 2019

Conversation

qw4990
Copy link
Contributor

@qw4990 qw4990 commented Mar 6, 2019 •
edited
Loading

What problem does this PR solve?

Add some unit tests to prove that chunk size control works in these cases.
Following up #9512, this PR is a subtask of #9166.

What is changed and how it works?

  1. Use mocktikv.Cluster to construct some specified cases of data distribution.
  2. Introduce testSlowClient to mock delay on some specified regions.
  3. Add unit tests for Limit+TableScan and Limit+IndexScan.

Check List

Tests

  • Unit test

@qw4990 qw4990 added the sig/execution SIG execution label Mar 6, 2019
@codecov-io
Copy link

codecov-io commented Mar 6, 2019 •
edited
Loading

Codecov Report

Merging #9574 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #9574      +/-   ##
=========================================
+ Coverage   67.28%   67.3%   +0.01%     
=========================================
  Files         376     376              
  Lines       78963   78963              
=========================================
+ Hits        53134   53149      +15     
+ Misses      21066   21056      -10     
+ Partials     4763    4758       -5
Impacted Files Coverage Δ
executor/aggregate.go 78.05% <0%> (-0.85%) ⬇️
executor/executor.go 68.57% <0%> (+0.4%) ⬆️
expression/schema.go 94.53% <0%> (+0.78%) ⬆️
executor/distsql.go 74.25% <0%> (+2.06%) ⬆️
ddl/delete_range.go 78.3% <0%> (+3.17%) ⬆️

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 b3d49c2...a70474a. Read the comment docs.

lysu
lysu reviewed Mar 6, 2019
View reviewed changes
Copy link
Contributor

@lysu lysu left a comment

Choose a reason for hiding this comment

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

LGTM

eurekaka
eurekaka reviewed Mar 7, 2019
View reviewed changes
Copy link
Contributor

@eurekaka eurekaka left a comment

Choose a reason for hiding this comment

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

LGTM, if license is added.

@qw4990 qw4990 force-pushed the chunkSizeControl_test branch from 2214bcd to a9c5366 Compare March 7, 2019 03:49
@qw4990 qw4990 added the status/LGT2 Indicates that a PR has LGTM 2. label Mar 7, 2019
@qw4990
Copy link
Contributor Author

qw4990 commented Mar 7, 2019

/run-all-tests

alivxxx
alivxxx previously approved these changes Mar 7, 2019
View reviewed changes
@qw4990
Copy link
Contributor Author

qw4990 commented Mar 7, 2019

/run-all-tests

1 similar comment
@qw4990
Copy link
Contributor Author

qw4990 commented Mar 7, 2019

/run-all-tests

@qw4990
Copy link
Contributor Author

qw4990 commented Mar 7, 2019

/run-all-tests

@qw4990
Copy link
Contributor Author

qw4990 commented Mar 7, 2019

/run-all-tests

2 similar comments
@qw4990
Copy link
Contributor Author

qw4990 commented Mar 7, 2019

/run-all-tests

@qw4990
Copy link
Contributor Author

qw4990 commented Mar 7, 2019

/run-all-tests

@qw4990
Copy link
Contributor Author

qw4990 commented Mar 7, 2019

/run-all-tests

1 similar comment
@qw4990
Copy link
Contributor Author

qw4990 commented Mar 7, 2019

/run-all-tests

@qw4990 qw4990 merged commit 2d89e3b into pingcap:master Mar 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eurekaka eurekaka eurekaka left review comments

@lysu lysu lysu approved these changes

@alivxxx alivxxx alivxxx left review comments

@zz-jason zz-jason Awaiting requested review from zz-jason

@winoros winoros Awaiting requested review from winoros

@XuHuaiyu XuHuaiyu Awaiting requested review from XuHuaiyu

Assignees
No one assigned
Labels
sig/execution SIG execution status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@qw4990 @codecov-io @lysu @eurekaka @alivxxx