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) #17543

Merged
merged 1 commit into from
Jun 1, 2020

Conversation

sre-bot
Copy link
Contributor

@sre-bot sre-bot commented Jun 1, 2020

cherry-pick #17542 to release-3.0


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

Signed-off-by: sre-bot <sre-bot@pingcap.com>
@sre-bot
Copy link
Contributor Author

sre-bot commented Jun 1, 2020

/run-all-tests

@bobotu
Copy link
Contributor

bobotu commented Jun 1, 2020

/run-all-tests

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

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

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

@lysu lysu added the status/LGT2 Indicates that a PR has LGTM 2. label Jun 1, 2020
@bobotu
Copy link
Contributor

bobotu commented Jun 1, 2020

/merge

@sre-bot
Copy link
Contributor Author

sre-bot commented Jun 1, 2020

Sorry @bobotu, you don't have permission to trigger auto merge event on this branch.

@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 Author

sre-bot commented Jun 1, 2020

/run-all-tests

@sre-bot
Copy link
Contributor Author

sre-bot commented Jun 1, 2020

@sre-bot merge failed.

@zyxbest
Copy link

zyxbest commented Jun 1, 2020

/run-all-tests

1 similar comment
@zyxbest
Copy link

zyxbest commented Jun 1, 2020

/run-all-tests

@bobotu bobotu added the priority/release-blocker This issue blocks a release. Please solve it ASAP. label Jun 1, 2020
@jackysp
Copy link
Member

jackysp commented Jun 1, 2020

/merge

@sre-bot
Copy link
Contributor Author

sre-bot commented Jun 1, 2020

/run-all-tests

@sre-bot sre-bot merged commit 58fc5bf into pingcap:release-3.0 Jun 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/statistics priority/release-blocker This issue blocks a release. Please solve it ASAP. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/performance type/3.0-cherry-pick
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