*: enlarge the default value of max-merge-region-size.
#8445
Conversation
This pr is used to enlarge the region size from default value `96MB` to `256MB`, compatible to the requirement of the stability of large cluster. Signed-off-by: lucasliang <[email protected]>
ti-chi-bot
bot
added
do-not-merge/needs-linked-issue
release-note
pkg/schedule/config/config.go
Outdated
| @@ -30,7 +30,7 @@ const ( | |||
| DefaultMaxReplicas = 3 | |||
| defaultMaxSnapshotCount = 64 | |||
| defaultMaxPendingPeerCount = 64 | |||
| defaultMaxMergeRegionSize = 20 | |||
| defaultMaxMergeRegionSize = 96 | |||
There was a problem hiding this comment.
We need to add more comments about which version changed and the related issue.
There was a problem hiding this comment.
Will it cause lots of merge operations when upgrading?
There was a problem hiding this comment.
+1, 96 is too aggressive, it might cause a lot of merging after upgrading. We increase the default region size from 96MB to 256MB in TiKV side, it is 2.67x larger than before. But here 20 -> 96 is nearly 5x larger than before, probably 40MB is better.
There was a problem hiding this comment.
How about using 54MB(=> 20MB * 256 / 96) as the recommended value ?
There was a problem hiding this comment.
Will it cause lots of merge operations when upgrading?
Nope, the old value of max-merge-region-size is already persisted in the cluster and it will be reloaded after upgrading. Only the newly created cluster will use the new default value.
There was a problem hiding this comment.
Oh, right. But for those existing clusters, do we still use the old max-merge-region-size and let the user change the config? If so, then LGTM.
There was a problem hiding this comment.
IMO, it should be.
Since each user's cluster may have a unique self-defined max-merge-region-size, manual adjustments should be checked and executed if the user requires it.
LykxSassinator
changed the title
max-merge-region-size.
Signed-off-by: lucasliang <[email protected]>
ti-chi-bot
bot
added
size/S
|
@zhangjinpeng87: adding LGTM is restricted to approvers and reviewers in OWNERS files. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/approve |
ti-chi-bot
bot
added
lgtm
approved
and removed
needs-1-more-lgtm
Signed-off-by: lucasliang <[email protected]>
Signed-off-by: lucasliang <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: niubell, rleungx, zhangjinpeng87 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: lucasliang <[email protected]>
|
/test |
|
@LykxSassinator: The
The following commands are available to trigger optional jobs:
Use
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/test all |
…v#8445)" This reverts commit aa85b6c. Signed-off-by: lucasliang <[email protected]>
…)" (#8541) ref pingcap/tidb#55374 Revert the changes on the configuration of region-size. These changes will be delayed until v8.4. Signed-off-by: lucasliang <[email protected]>
This pr is used to enlarge the region size from default value
96MBto256MB, compatible to the requirement of the stability of large cluster.What problem does this PR solve?
Issue Number: close #8484, ref tikv/tikv#17309
What is changed and how does it work?
Check List
Tests
Code changes
Side effects
Related changes
pingcap/docs/pingcap/docs-cn:pingcap/tiup:Release note