config: fix the store limit cannot be persisted #3380

rleungx · 2021-01-22T03:43:36Z

What problem does this PR solve?

The store limit cannot persist due to we only change the value in the memory. Close #3382.

What is changed and how it works?

This PR persists the store limit once it changes and if persistence fails, we will roll back to the original one.

Check List

Tests

Unit test

Related changes

Need to cherry-pick to the release branch

Release note

Fix the issue that the store limit cannot be persisted

codecov · 2021-01-22T03:58:56Z

Codecov Report

Merging #3380 (b9c4583) into master (680a3a4) will decrease coverage by 0.17%.
The diff coverage is 58.33%.

@@            Coverage Diff             @@
##           master    #3380      +/-   ##
==========================================
- Coverage   75.06%   74.88%   -0.18%     
==========================================
  Files         243      243              
  Lines       23325    23353      +28     
==========================================
- Hits        17510    17489      -21     
- Misses       4256     4292      +36     
- Partials     1559     1572      +13

Flag	Coverage Δ
unittests	`74.88% <58.33%> (-0.18%)`	⬇️

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

Impacted Files	Coverage Δ
server/config/persist_options.go	`91.33% <50.00%> (ø)`
server/cluster/cluster.go	`82.33% <54.83%> (-1.36%)`	⬇️
server/handler.go	`41.97% <100.00%> (-0.29%)`	⬇️
pkg/tempurl/tempurl.go	`45.00% <0.00%> (-15.00%)`	⬇️
server/schedulers/shuffle_hot_region.go	`54.73% <0.00%> (-9.48%)`	⬇️
pkg/dashboard/adapter/manager.go	`84.78% <0.00%> (-7.61%)`	⬇️
server/tso/global_allocator.go	`68.99% <0.00%> (-6.98%)`	⬇️
server/tso/local_allocator.go	`82.08% <0.00%> (-2.99%)`	⬇️
server/tso/tso.go	`81.48% <0.00%> (-1.86%)`	⬇️
server/member/member.go	`68.47% <0.00%> (-1.64%)`	⬇️
... and 11 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 680a3a4...b9c4583. Read the comment docs.

JmPotato · 2021-01-22T06:46:49Z

server/cluster/cluster.go

 	c.opt.SetStoreLimit(storeID, typ, ratePerMin)
+	if err := c.opt.Persist(c.storage); err != nil {
+		// roll back the store limit
+		c.opt.SetScheduleConfig(old)


What will happen if the rollback operation fails also?

The rollback operation only resets the memory value

Yisaer

LGTM

Signed-off-by: Ryan Leung <[email protected]>

server/cluster/cluster.go

Signed-off-by: Ryan Leung <[email protected]>

rleungx · 2021-01-27T10:18:13Z

I made some new changes, PTAL @Yisaer @JmPotato

ti-chi-bot · 2021-01-27T14:06:59Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

HunDunDM
Yisaer

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 writing /lgtm in a comment.
Reviewer can cancel approval by writing /lgtm cancel in a comment.

lhy1024 · 2021-01-28T05:04:35Z

/merge

ti-chi-bot · 2021-01-28T05:04:36Z

@lhy1024: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

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 ti-community-infra/tichi repository.

ti-chi-bot · 2021-01-28T05:04:38Z

This pull request has been accepted and is ready to merge.

Commit hash: 4041a3a

Yisaer

LGTM

Yisaer · 2021-01-28T05:05:03Z

/merge

ti-chi-bot · 2021-01-28T05:05:03Z

@Yisaer: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

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 ti-community-infra/tichi repository.

nolouch · 2021-01-28T05:58:22Z

/merge

ti-chi-bot · 2021-01-28T05:58:23Z

@nolouch: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

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 ti-community-infra/tichi repository.

server/cluster/cluster.go

Signed-off-by: Ryan Leung <[email protected]>

rleungx · 2021-01-28T06:11:35Z

/run-integration-lightning-test

HunDunDM · 2021-01-28T07:20:03Z

/merge

ti-chi-bot · 2021-01-28T07:20:04Z

@HunDunDM: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

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 ti-community-infra/tichi repository.

ti-chi-bot · 2021-01-28T07:20:06Z

This pull request has been accepted and is ready to merge.

Commit hash: 0a1a883

ti-chi-bot · 2021-01-28T07:38:02Z

@rleungx: Your PR has out-of-dated, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

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 ti-community-infra/tichi repository.

rleungx · 2021-01-28T07:50:19Z

/run-unit-tests

rleungx · 2021-01-28T07:50:24Z

/rebuild

rleungx · 2021-01-28T07:50:57Z

/run-all-tests

HunDunDM · 2021-01-28T08:09:51Z

/run-all-tests

Signed-off-by: ti-srebot <[email protected]>

ti-srebot · 2021-01-28T08:22:09Z

cherry pick to release-4.0 in PR #3403

Signed-off-by: ti-srebot <[email protected]>

ti-srebot · 2021-01-28T08:22:41Z

cherry pick to release-5.0-rc in PR #3404

Signed-off-by: Ryan Leung <[email protected]>

Signed-off-by: Ryan Leung <[email protected]> Co-authored-by: Ryan Leung <[email protected]> Co-authored-by: 混沌DM <[email protected]>

rleungx added the component/config Configuration logic. label Jan 22, 2021

ti-chi-bot requested review from HunDunDM and JmPotato January 22, 2021 03:43

JmPotato reviewed Jan 22, 2021

View reviewed changes

rleungx added needs-cherry-pick-release-4.0 The PR needs to cherry pick to release-4.0 branch. needs-cherry-pick-5.0-rc labels Jan 26, 2021

Yisaer approved these changes Jan 26, 2021

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jan 26, 2021

rleungx added 2 commits January 27, 2021 17:35

persist store limit

6d0dd26

Signed-off-by: Ryan Leung <[email protected]>

support persist retry time

a35391d

Signed-off-by: Ryan Leung <[email protected]>

HunDunDM reviewed Jan 27, 2021

View reviewed changes

server/cluster/cluster.go Outdated Show resolved Hide resolved

server/cluster/cluster.go Outdated Show resolved Hide resolved

server/cluster/cluster.go Outdated Show resolved Hide resolved

server/cluster/cluster.go Outdated Show resolved Hide resolved

address the comment

4041a3a

Signed-off-by: Ryan Leung <[email protected]>

rleungx force-pushed the persist-limit branch from 03a7876 to 4041a3a Compare January 27, 2021 10:12

rleungx requested a review from Yisaer January 27, 2021 10:17

HunDunDM approved these changes Jan 27, 2021

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jan 27, 2021

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jan 28, 2021

Yisaer approved these changes Jan 28, 2021

View reviewed changes

nolouch reviewed Jan 28, 2021

View reviewed changes

server/cluster/cluster.go Outdated Show resolved Hide resolved

address the comment

5358cc9

Signed-off-by: Ryan Leung <[email protected]>

rleungx force-pushed the persist-limit branch from 7d649f3 to 5358cc9 Compare January 28, 2021 06:02

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Jan 28, 2021

Merge branch 'master' into persist-limit

1368aeb

Merge branch 'master' into persist-limit

0a1a883

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jan 28, 2021

ti-chi-bot added 2 commits January 28, 2021 15:20

Merge branch 'master' into persist-limit

9caf3a8

Merge branch 'master' into persist-limit

b9c4583

ti-chi-bot merged commit 028befd into tikv:master Jan 28, 2021

ti-srebot pushed a commit to ti-srebot/pd that referenced this pull request Jan 28, 2021

cherry pick tikv#3380 to release-4.0

607fe57

Signed-off-by: ti-srebot <[email protected]>

ti-srebot mentioned this pull request Jan 28, 2021

config: fix the store limit cannot be persisted (#3380) #3403

Merged

ti-srebot pushed a commit to ti-srebot/pd that referenced this pull request Jan 28, 2021

cherry pick tikv#3380 to release-5.0-rc

3b2bab4

Signed-off-by: ti-srebot <[email protected]>

ti-srebot mentioned this pull request Jan 28, 2021

config: fix the store limit cannot be persisted (#3380) #3404

Closed

rleungx added a commit to ti-srebot/pd that referenced this pull request Jan 28, 2021

cherry pick tikv#3380 to release-4.0

5fd7621

Signed-off-by: Ryan Leung <[email protected]>

ti-chi-bot pushed a commit that referenced this pull request Jan 28, 2021

cherry pick #3380 to release-4.0 (#3403)

96efd6f

Signed-off-by: Ryan Leung <[email protected]> Co-authored-by: Ryan Leung <[email protected]> Co-authored-by: 混沌DM <[email protected]>

rleungx mentioned this pull request Jul 21, 2021

PD cannot store the configuration persistently pingcap/tidb#22498

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

config: fix the store limit cannot be persisted #3380

config: fix the store limit cannot be persisted #3380

rleungx commented Jan 22, 2021 •

edited

Loading

codecov bot commented Jan 22, 2021 •

edited

Loading

JmPotato Jan 22, 2021

rleungx Jan 22, 2021

Yisaer left a comment

rleungx commented Jan 27, 2021

ti-chi-bot commented Jan 27, 2021

lhy1024 commented Jan 28, 2021

ti-chi-bot commented Jan 28, 2021

ti-chi-bot commented Jan 28, 2021

Yisaer left a comment

Yisaer commented Jan 28, 2021

ti-chi-bot commented Jan 28, 2021

nolouch commented Jan 28, 2021

ti-chi-bot commented Jan 28, 2021

rleungx commented Jan 28, 2021

HunDunDM commented Jan 28, 2021

ti-chi-bot commented Jan 28, 2021

ti-chi-bot commented Jan 28, 2021

ti-chi-bot commented Jan 28, 2021

rleungx commented Jan 28, 2021

rleungx commented Jan 28, 2021

rleungx commented Jan 28, 2021

HunDunDM commented Jan 28, 2021

ti-srebot commented Jan 28, 2021

ti-srebot commented Jan 28, 2021

config: fix the store limit cannot be persisted #3380

config: fix the store limit cannot be persisted #3380

Conversation

rleungx commented Jan 22, 2021 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

codecov bot commented Jan 22, 2021 • edited Loading

Codecov Report

JmPotato Jan 22, 2021

Choose a reason for hiding this comment

rleungx Jan 22, 2021

Choose a reason for hiding this comment

Yisaer left a comment

Choose a reason for hiding this comment

rleungx commented Jan 27, 2021

ti-chi-bot commented Jan 27, 2021

lhy1024 commented Jan 28, 2021

ti-chi-bot commented Jan 28, 2021

ti-chi-bot commented Jan 28, 2021

Yisaer left a comment

Choose a reason for hiding this comment

Yisaer commented Jan 28, 2021

ti-chi-bot commented Jan 28, 2021

nolouch commented Jan 28, 2021

ti-chi-bot commented Jan 28, 2021

rleungx commented Jan 28, 2021

HunDunDM commented Jan 28, 2021

ti-chi-bot commented Jan 28, 2021

ti-chi-bot commented Jan 28, 2021

ti-chi-bot commented Jan 28, 2021

rleungx commented Jan 28, 2021

rleungx commented Jan 28, 2021

rleungx commented Jan 28, 2021

HunDunDM commented Jan 28, 2021

ti-srebot commented Jan 28, 2021

ti-srebot commented Jan 28, 2021

rleungx commented Jan 22, 2021 •

edited

Loading

codecov bot commented Jan 22, 2021 •

edited

Loading