-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
util/paging: choose min paging size default value as 128, and max value as 8192 #36331
Conversation
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
|
B.T.W, copr-cache hit rate may account for that paging performance become better than non-paging, but it's just my speculation. |
For max paging size = 8192, we've tested that setting will not bring obvious performance regression comparing to non-paging, in some other AP testing scenarios not listed here. |
Code Coverage Details: https://codecov.io/github/pingcap/tidb/commit/8c106a4de20aed92e2175df82246b70ccacb3522 |
/merge |
This pull request has been accepted and is ready to merge. Commit hash: b06a101
|
/run-check_dev_2 |
/run-mysql-test |
1 similar comment
/run-mysql-test |
The mysql-test case is fixed in https://github.com/pingcap/tidb-test/pull/1892/files |
/run-mysql-test |
/run-mysql-test tidb-test=pr/1892 |
TiDB MergeCI notify🔴 Bad News! [2] CI still failing after this pr merged.
|
What problem does this PR solve?
Issue Number: close #36328
Problem Summary:
What is changed and how it works?
When min paging size is too small, we need more network round-trip.
But when it's too large, the packet size grow and consume more memory.
There are two variable, min paging size and max paging size to control behaviour.
After expirement, I'd like to choose the default min paging size to 128.
In the following picture, I expirement with:
The
DistSQL QPS
panel reflect the total qpsThe
Coprocessor Query Per Second
panel reflect the actual paging requests, one distsql query may result in several, they are in 1:N relationshipCoprocessor query for two reason:
For 2, we want to choose the best defalut value for
tidb_min_paging_size
to reduce network round-trip to get better performance.From the
Coprocessor Query Per Second
panel, it's obvious that size = 16 is too small,128, 256, 512 are basicaly the same...
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.