config: add `region-max-size` property #4635

bufferflies · 2022-02-11T06:15:28Z

Signed-off-by: bufferflies 1045931706@qq.com

What problem does this PR solve?

Issue Number: Close #4459

What is changed and how it works?

config.PersistOptions should save the initial config and not update it .

Check List

Tests

Unit test

Code changes

Side effects

Related changes

Release note

config adds`region-max-size ` property

Signed-off-by: bufferflies <1045931706@qq.com>

ti-chi-bot · 2022-02-11T06:15:29Z

[REVIEW NOTIFICATION]

This pull request has not been approved.

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

server/config/persist_options.go

rleungx · 2022-02-11T06:42:09Z

Please add a unit test to check if changing the parameter in the config file works.

Signed-off-by: bufferflies <1045931706@qq.com>

codecov · 2022-02-11T09:46:16Z

Codecov Report

Merging #4635 (9b5f342) into master (4b6f00c) will decrease coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4635      +/-   ##
==========================================
- Coverage   74.75%   74.70%   -0.06%     
==========================================
  Files         284      285       +1     
  Lines       27816    27835      +19     
==========================================
  Hits        20795    20795              
- Misses       5149     5160      +11     
- Partials     1872     1880       +8

Flag	Coverage Δ
unittests	`74.70% <100.00%> (-0.06%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/mock/mockcluster/mockcluster.go	`94.98% <100.00%> (+0.02%)`	⬆️
server/cluster/cluster.go	`84.82% <100.00%> (+0.44%)`	⬆️
server/config/config.go	`77.94% <100.00%> (+0.09%)`	⬆️
server/config/immutable_options.go	`100.00% <100.00%> (ø)`
server/config/util.go	`87.50% <100.00%> (+1.29%)`	⬆️
server/schedule/checker/merge_checker.go	`74.40% <100.00%> (+0.84%)`	⬆️
server/server.go	`74.87% <100.00%> (-0.29%)`	⬇️
pkg/tempurl/tempurl.go	`45.00% <0.00%> (-15.00%)`	⬇️
server/tso/tso.go	`62.50% <0.00%> (-2.85%)`	⬇️
server/storage/hot_region_storage.go	`79.88% <0.00%> (-2.30%)`	⬇️
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b6f00c...9b5f342. Read the comment docs.

Signed-off-by: bufferflies <1045931706@qq.com>

…pd into feature/add-region-max-size

nolouch · 2022-02-22T03:54:39Z

How this config influence the TiKV?

conf/config.toml

disksing · 2022-02-22T06:24:57Z

pkg/mock/mockcluster/mockcluster.go

+		IDAllocator:     mockid.NewIDAllocator(),
+		HotStat:         statistics.NewHotStat(ctx),
+		PersistOptions:  opts,
+		ImmutableConfig: config.NewTestImmutableOptions(),


Does immutable means that it cannot be updated by pd-ctl?

yes, pd will reload config after restarting.

rleungx · 2022-02-22T07:10:17Z

pkg/mock/mockcluster/mockcluster.go

@@ -78,6 +80,11 @@ func (mc *Cluster) GetOpts() *config.PersistOptions {
 	return mc.PersistOptions
 }

+// GetConfig returns the cluster immutable configuration.
+func (mc *Cluster) GetConfig() *config.ImmutableConfig {


Prefer to change the name.

fix to GetImmutableCfg

rleungx · 2022-02-22T07:11:16Z

server/config/immutable_options.go

+// ImmutableConfig is a readonly config.
+type ImmutableConfig struct {
+	maxRegionSize uint64
+	leaderLease   int64


Why only leaderLease?

leaderLease will be remove and add max-split-size.

server/config/immutable_options.go

server/schedule/operator/builder.go

bufferflies · 2022-02-23T04:13:26Z

How this config influence the TiKV?

the configuration will not influence the TiKVs, maybe can consider this pr:#4308

Signed-off-by: bufferflies <1045931706@qq.com>

…pd into feature/add-region-max-size

Signed-off-by: bufferflies <1045931706@qq.com>

bufferflies · 2022-03-03T03:40:19Z

It is not graceful to add some tikv config into the config file, so I will use call back tikv to get this config.

add region-max-size to config

123c580

Signed-off-by: bufferflies <1045931706@qq.com>

ti-chi-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Feb 11, 2022

ti-chi-bot requested review from disksing and rleungx February 11, 2022 06:15

rleungx reviewed Feb 11, 2022

View reviewed changes

server/config/persist_options.go Outdated Show resolved Hide resolved

resolv config

66fb908

Signed-off-by: bufferflies <1045931706@qq.com>

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 11, 2022

bufferflies force-pushed the feature/add-region-max-size branch from 4d6f139 to 29a8db6 Compare February 11, 2022 09:43

add unit test

f9df236

Signed-off-by: bufferflies <1045931706@qq.com>

bufferflies force-pushed the feature/add-region-max-size branch from 29a8db6 to f9df236 Compare February 11, 2022 09:50

bufferflies added 5 commits February 14, 2022 17:07

create new interface to get immutable config

0688424

Signed-off-by: bufferflies <1045931706@qq.com>

Merge branch 'master' into feature/add-region-max-size

81ba30c

Merge branch 'master' into feature/add-region-max-size

ff97cc9

pass ut

33e84c1

Signed-off-by: bufferflies <1045931706@qq.com>

Merge branch 'feature/add-region-max-size' of github.com:bufferflies/…

5f32377

…pd into feature/add-region-max-size

bufferflies requested a review from rleungx February 18, 2022 08:35

Merge branch 'master' into feature/add-region-max-size

dc2c9f7

disksing reviewed Feb 22, 2022

View reviewed changes

rleungx reviewed Feb 22, 2022

View reviewed changes

bufferflies mentioned this pull request Feb 23, 2022

region may be splited after the region is merged due to some wrong configuration. #4659

Closed

address comment

de13935

Signed-off-by: bufferflies <1045931706@qq.com>

bufferflies mentioned this pull request Feb 23, 2022

api: validate config changes before it updates. #4672

Closed

bufferflies force-pushed the feature/add-region-max-size branch 2 times, most recently from 0471d73 to 41f7f79 Compare February 23, 2022 11:22

bufferflies force-pushed the feature/add-region-max-size branch 3 times, most recently from 3a01315 to 2fbeb24 Compare February 24, 2022 08:58

Merge branch 'feature/add-region-max-size' of github.com:bufferflies/…

3dcf9e1

…pd into feature/add-region-max-size

bufferflies force-pushed the feature/add-region-max-size branch from 2fbeb24 to 3dcf9e1 Compare February 24, 2022 09:10

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 24, 2022

deadlock

d119e2f

Signed-off-by: bufferflies <1045931706@qq.com>

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 24, 2022

deadlock

dc5f07a

Signed-off-by: bufferflies <1045931706@qq.com>

bufferflies force-pushed the feature/add-region-max-size branch from 0cdd667 to dc5f07a Compare February 24, 2022 09:27

Merge branch 'master' into feature/add-region-max-size

9b5f342

bufferflies closed this Mar 3, 2022

bufferflies mentioned this pull request Mar 31, 2022

schedule: avoid that the smallest region of the spliting regions can be merged again. #4779

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

config: add `region-max-size` property #4635

config: add `region-max-size` property #4635

bufferflies commented Feb 11, 2022

ti-chi-bot commented Feb 11, 2022

rleungx commented Feb 11, 2022

codecov bot commented Feb 11, 2022 •

edited

Loading

nolouch commented Feb 22, 2022

disksing Feb 22, 2022

bufferflies Feb 23, 2022

rleungx Feb 22, 2022

bufferflies Feb 24, 2022

rleungx Feb 22, 2022

bufferflies Feb 24, 2022

bufferflies commented Feb 23, 2022

bufferflies commented Mar 3, 2022

config: add region-max-size property #4635

config: add region-max-size property #4635

Conversation

bufferflies commented Feb 11, 2022

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Feb 11, 2022

rleungx commented Feb 11, 2022

codecov bot commented Feb 11, 2022 • edited Loading

Codecov Report

nolouch commented Feb 22, 2022

disksing Feb 22, 2022

Choose a reason for hiding this comment

bufferflies Feb 23, 2022

Choose a reason for hiding this comment

rleungx Feb 22, 2022

Choose a reason for hiding this comment

bufferflies Feb 24, 2022

Choose a reason for hiding this comment

rleungx Feb 22, 2022

Choose a reason for hiding this comment

bufferflies Feb 24, 2022

Choose a reason for hiding this comment

bufferflies commented Feb 23, 2022

bufferflies commented Mar 3, 2022

config: add `region-max-size` property #4635

config: add `region-max-size` property #4635

codecov bot commented Feb 11, 2022 •

edited

Loading