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: Speed up unit tests #14357

Merged
merged 6 commits into from
Jan 6, 2020
Merged

executor: Speed up unit tests #14357

merged 6 commits into from
Jan 6, 2020

Conversation

zyxbest
Copy link
Contributor

@zyxbest zyxbest commented Jan 6, 2020 •
edited
Loading

What problem does this PR solve?

Speed up executor unit test .

What is changed and how it works?

Change the suite distribution of slow tests.
See the statistic report:
image
Most testSuites ended in 30 seconds.
Some testSuites are too large such as testSuite3 and testSuite4 in y-axis, since the tests in one testSuite run one by one.
It's ok to move some tests from "Slow Suite" to "Quick Suite"

@zyxbest zyxbest changed the title split tests from testSuite3 and testSuite4 executor :speed up unit tests Jan 6, 2020
@zyxbest zyxbest changed the title executor :speed up unit tests [DNM] executor :speed up unit tests Jan 6, 2020
@zyxbest
Copy link
Contributor Author

zyxbest commented Jan 6, 2020

/run-check_dev

@zyxbest
Copy link
Contributor Author

zyxbest commented Jan 6, 2020

As a result:
before:

ok  	github.com/pingcap/tidb/executor	106.273s

after:

ok      github.com/pingcap/tidb/executor    88.846s

The new picture looks better
image

@zyxbest zyxbest changed the title [DNM] executor :speed up unit tests executor: Speed up unit tests Jan 6, 2020
@zyxbest
Copy link
Contributor Author

zyxbest commented Jan 6, 2020

PTAL @SunRunAway @bb7133

SunRunAway
SunRunAway reviewed Jan 6, 2020
View reviewed changes
Copy link
Contributor

@SunRunAway SunRunAway left a comment

Choose a reason for hiding this comment

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

LGTM

@ngaut
Copy link
Member

ngaut commented Jan 6, 2020

Cool, How did you get the image? Is there any tools can generate it?

@SunRunAway SunRunAway added sig/execution SIG execution component/test status/LGT1 Indicates that a PR has LGTM 1. labels Jan 6, 2020
@zyxbest
Copy link
Contributor Author

zyxbest commented Jan 6, 2020 •
edited
Loading

  1. I've craweled the log which is all we need from Jenkins website.
  2. Then convert the go test log into csv with pandas in Python.
  3. Plot them with Pcolor

Step 2 is the most annoying which relys on some data statistic tricks, but works for all kinds of unit test.

@ngaut

zz-jason
zz-jason approved these changes Jan 6, 2020
View reviewed changes
Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@zz-jason zz-jason added status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jan 6, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Jan 6, 2020

Your auto merge job has been accepted, waiting for 14200, 14035, 14275, 14229

@sre-bot
Copy link
Contributor

sre-bot commented Jan 6, 2020

/run-all-tests

@sre-bot sre-bot merged commit 39ed9f3 into pingcap:master Jan 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SunRunAway SunRunAway SunRunAway left review comments

@zz-jason zz-jason zz-jason approved these changes

Assignees
No one assigned
Labels
component/test sig/execution SIG execution status/can-merge Indicates a PR has been approved by a committer. 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
@zyxbest @ngaut @sre-bot @SunRunAway @zz-jason