server/api:support offline a store with physically destroyed #3388
Conversation
…ans it can never up again Signed-off-by: shirly <AndreMouche@126.com>
Codecov Report
@@ Coverage Diff @@
## master #3388 +/- ##
==========================================
- Coverage 75.00% 74.87% -0.13%
==========================================
Files 243 243
Lines 23305 23314 +9
==========================================
- Hits 17479 17456 -23
- Misses 4266 4285 +19
- Partials 1560 1573 +13
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Signed-off-by: shirly <AndreMouche@126.com>
| if store.IsPhysicallyDestroyed() { | ||
| return errors.Errorf("The store is already physically destroyed") | ||
| } |
There was a problem hiding this comment.
Maybe we can return nil here as the store is already PhysicallyDestroyed
There was a problem hiding this comment.
Since physically-destroyed is not allowed from true to false.
There was a problem hiding this comment.
I think we should define a errorCode like ErrStoreTombstone and to explain it as The store have been force removed
| if store.IsPhysicallyDestroyed() { | ||
| return errors.Errorf("The store is already physically destroyed") | ||
| } |
There was a problem hiding this comment.
I think we should define a errorCode like ErrStoreTombstone and to explain it as The store have been force removed
Signed-off-by: shirly <AndreMouche@126.com>
| } | ||
|
|
||
| // OfflineStore offline a store | ||
| func OfflineStore(physicallyDestroyed bool) StoreCreateOption { |
There was a problem hiding this comment.
Can we just use func SetStoreState(state metapb.StoreState, destroyed ...bool) StoreCreateOption here?
There was a problem hiding this comment.
I think ... bool is not a good design.
There was a problem hiding this comment.
how about
func setStoreState(state metapb.StoreState, physicallyDestroyed bool)
func OfflineStore(physicallyDestroyed bool)
func UpStore()
func TombstoneStore()
|
[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. DetailsReviewer can indicate their review by writing |
ti-chi-bot
added
status/LGT2
|
/merge |
|
@Yisaer: It seems you want to merge this PR, I will help you trigger all the tests: /run-all-tests DetailsInstructions 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. |
|
This pull request has been accepted and is ready to merge. DetailsCommit hash: c97ec36 |
ti-chi-bot
added
the
status/can-merge
|
@AndreMouche: 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 DetailsInstructions 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. |
5 participants
Signed-off-by: shirly AndreMouche@126.com
What problem does this PR solve?
It is part of #3076
This PR support to offline a store and set it physically destroyed which means this store can never up and we can start a new store with the same address
What is changed and how it works?
force is a boolean value:
Check List
Tests
Code changes
Related changes
pingcap/kvproto#711
Release note