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.

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

executor, stats: fix fast analyze bugs #10680

Merged
merged 3 commits into from Jun 4, 2019

Conversation

Projects
None yet
3 participants
@lamxTyler
Copy link
Member

commented Jun 3, 2019

What problem does this PR solve?

Fast analyze panics during the analyze process.

What is changed and how it works?

  • Initialize the map before use.
  • Avoid divide by zero when calculating the default value in cm sketch.
  • Recalculate the row count when we partially retry the failed region.
  • When getting the region row count in mocktikv, we should use the decoded form of region start and end key, or the result will be incorrect.

Check List

Tests

  • Unit test

Code changes

  • Has exported function/method change

Side effects

  • None

Related changes

  • Need to cherry-pick to the release branch
@codecov

This comment has been minimized.

Copy link

commented Jun 3, 2019

Codecov Report

Merging #10680 into master will decrease coverage by 0.0066%.
The diff coverage is 90%.

@@               Coverage Diff                @@
##             master     #10680        +/-   ##
================================================
- Coverage   78.3515%   78.3448%   -0.0067%     
================================================
  Files           414        414                
  Lines         87706      87679        -27     
================================================
- Hits          68719      68692        -27     
+ Misses        13847      13845         -2     
- Partials       5140       5142         +2
Show resolved Hide resolved executor/analyze.go Outdated
@zz-jason
Copy link
Member

left a comment

LGTM

@zz-jason zz-jason added the status/LGT1 label Jun 4, 2019

@qw4990

qw4990 approved these changes Jun 4, 2019

Copy link
Contributor

left a comment

LGTM

@lamxTyler

This comment has been minimized.

Copy link
Member Author

commented Jun 4, 2019

/run-all-tests

@lamxTyler lamxTyler merged commit 89bfed3 into pingcap:master Jun 4, 2019

16 checks passed

ci/circleci Your tests passed on CircleCI!
Details
codecov/patch 90% of diff hit (target 0%)
Details
codecov/project Absolute coverage decreased by -0.0066% but relative coverage increased by +11.6484% compared to 8bc5553
Details
idc-jenkins-ci-tidb/build Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/build_check_race Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/check_dev Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/check_dev_2 Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/common-test job succeeded
Details
idc-jenkins-ci-tidb/integration-common-test Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/integration-compatibility-test Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/integration-ddl-test Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/mybatis-test job succeeded
Details
idc-jenkins-ci-tidb/sqllogic-test-1 Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/sqllogic-test-2 Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/unit-test Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details

@lamxTyler lamxTyler deleted the lamxTyler:fast-analyze branch Jun 4, 2019

lamxTyler added a commit to lamxTyler/tidb that referenced this pull request Jun 4, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.