operator: change the return type of ConfVerChanged from bool to uint64 #2919
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Zheng Xiangsheng <[email protected]>
rleungx
reviewed
Sep 8, 2020
| // ConfVerChanged returns the delta value for version increased by this step. | ||
| func (rp RemovePeer) ConfVerChanged(region *core.RegionInfo) uint64 { | ||
| id := region.GetStorePeer(rp.FromStore).GetId() | ||
| return typeutil.BoolToUint64(id == 0 || (rp.PeerID != 0 && id != rp.PeerID)) |
There was a problem hiding this comment.
When will PeerID be set and when not set?
There was a problem hiding this comment.
It will not be set now, and the builder code will be updated later to use this new logic.
Yisaer
reviewed
Sep 8, 2020
| } | ||
| return false | ||
| } | ||
|
|
||
| // CheckSafety checks if the step meets the safety properties. | ||
| func (pl PromoteLearner) CheckSafety(region *core.RegionInfo) error { | ||
| peer := region.GetStorePeer(pl.ToStore) | ||
| if peer == nil { | ||
| if peer.GetId() != pl.PeerID { |
There was a problem hiding this comment.
Do we still need to check if peer is nil here?
There was a problem hiding this comment.
The GetXX function of ProtoBuf has its own nil judgment.
Comment on lines
+38
to
+44
| // BoolToUint64 converts bool to uint64. | ||
| func BoolToUint64(b bool) uint64 { | ||
| if b { | ||
| return 1 | ||
| } | ||
| return 0 | ||
| } |
There was a problem hiding this comment.
What about adding a unit test here?
There was a problem hiding this comment.
I think this is just a conditional operator.
Signed-off-by: Zheng Xiangsheng <[email protected]>
HunDunDM
changed the title
Signed-off-by: Zheng Xiangsheng <[email protected]>
disksing
approved these changes
Sep 8, 2020
ti-srebot
approved these changes
Sep 8, 2020
ti-srebot
added
status/LGT3
|
/merge |
ti-srebot
added
the
status/can-merge
|
/run-all-tests |
HunDunDM
added a commit
to HunDunDM/pd
that referenced
this pull request
Sep 16, 2020
tikv#2919) Signed-off-by: Zheng Xiangsheng <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Zheng Xiangsheng [email protected]
What problem does this PR solve?
This is a child PR of #2886
For details, please refer to #2860
What is changed and how it works?
ConfVerChangedfrombooltouint64ConfVerChangedbehavior ofRemovePeer:PeerIDis specified, ifPeer.Idis changed, it is also regarded asConfVerChanged = 1Check List
Tests
Release note