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

statistics: avoid large CMSketch affecting the latency of normal query #17542

Merged
merged 1 commit into from
Jun 1, 2020

Conversation

bobotu
Copy link
Contributor

@bobotu bobotu commented May 31, 2020

What problem does this PR solve?

Problem Summary:
We have captured a runtime trace of tidb-server and observed a large latency caused by GC sweep.
图片

图片

The root cause is the large amount 8K span referenced by CMSketch and there is a limitation of Go's allocator. In this case, 8K allocation issued by IndexLookUpExec will take a long time to scan memory spans used by CMSketch.

What is changed and how it works?

Memory allocation larger than 32K will never use memory span. The default configuration of CMSketch will use 40K memory, so use a single slice is a workaround.

This problem can be solved in Go 1.15, so this simple workaround is a good choice before Go 1.15 release.

图片

This test is using v3.0.14 TiDB compiled with Go 1.13.10, but I believe v4.0 and Go 1.14 have the same problem.

Release note

  • avoid large CMSketch affecting the latency of normal query

@bobotu bobotu requested review from lysu and zz-jason May 31, 2020 02:47
@bobotu bobotu requested a review from a team as a code owner May 31, 2020 02:47
@ghost ghost removed their request for review May 31, 2020 02:47
@sre-bot
Copy link
Contributor

sre-bot commented May 31, 2020

@bobotu
Copy link
Contributor Author

bobotu commented May 31, 2020

/rebuild

@codecov
Copy link

codecov bot commented May 31, 2020

Codecov Report

Merging #17542 into master will decrease coverage by 0.0231%.
The diff coverage is 100.0000%.

@@               Coverage Diff                @@
##             master     #17542        +/-   ##
================================================
- Coverage   79.8510%   79.8279%   -0.0232%     
================================================
  Files           520        520                
  Lines        140067     139743       -324     
================================================
- Hits         111845     111554       -291     
+ Misses        19275      19239        -36     
- Partials       8947       8950         +3     

@siddontang
Copy link
Member

good catch!!!

Copy link
Member

@ngaut ngaut 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 ngaut added the status/LGT1 Indicates that a PR has LGTM 1. label Jun 1, 2020
Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

@jackysp
Copy link
Member

jackysp commented Jun 1, 2020

/merge

@sre-bot sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jun 1, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Jun 1, 2020

/run-all-tests

@sre-bot sre-bot merged commit d3f1a7a into pingcap:master Jun 1, 2020
@bobotu
Copy link
Contributor Author

bobotu commented Jun 1, 2020

/run-cherry-picker

sre-bot pushed a commit to sre-bot/tidb that referenced this pull request Jun 1, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Jun 1, 2020

cherry pick to release-3.0 in PR #17543

sre-bot pushed a commit to sre-bot/tidb that referenced this pull request Jun 1, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Jun 1, 2020

cherry pick to release-3.1 in PR #17544

sre-bot pushed a commit to sre-bot/tidb that referenced this pull request Jun 1, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Jun 1, 2020

cherry pick to release-4.0 in PR #17545

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/statistics status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1. type/performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The large CMSketch may affecting the latency of normal query
6 participants