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

Rollback config in store when kv.persist failed #1475

Closed
bradyjoestar opened this issue Mar 23, 2019 · 2 comments
Closed

Rollback config in store when kv.persist failed #1475

bradyjoestar opened this issue Mar 23, 2019 · 2 comments
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. type/enhancement The issue or PR belongs to an enhancement.

Comments

@bradyjoestar
Copy link
Contributor

bradyjoestar commented Mar 23, 2019

I am reading the following code
https://github.com/pingcap/pd/blob/f72cc23e5765b32c0d503e4fecd4b37fea44c98f/server/server.go#L531-L542
If s.scheduleOpt.persist(s.kv) returns err, it will not rollback to the old config in scheduleOpt

Maybe we'd better add the rollback code? just like this:

old := s.scheduleOpt.load() 
s.scheduleOpt.store(&cfg) 
if err := s.scheduleOpt.persist(s.kv); err != nil { 
        s.scheduleOpt.store(old) 
 	return err 
 } 
@nolouch
Copy link
Contributor

nolouch commented Mar 23, 2019

@bradyjoestar good catch!

@bradyjoestar
Copy link
Contributor Author

@nolouch I will create a new pr for these similar case. 😬

@rleungx rleungx added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. type/enhancement The issue or PR belongs to an enhancement. labels Mar 24, 2019
@rleungx rleungx closed this as completed Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

No branches or pull requests

3 participants