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

schedule: add some operator steps related to joint consensus #2895

Merged
merged 6 commits into from
Sep 9, 2020

Conversation

HunDunDM
Copy link
Member

@HunDunDM HunDunDM commented Sep 4, 2020

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?

  • add step DemoteFollower
  • add steps ChangePeerV2Enter and ChangePeerVLeave with a sub-step DemoteVoter
  • change the return value of ConfVerChanged from bool to uint64
  • adjust the ConfVerChanged behavior of RemovePeer:
    • when PeerID is specified, if PeerID is changed, it is also regarded as ConfVerChanged = 1

Check List

Tests

  • Unit test

Release note

  • No release note

@HunDunDM HunDunDM self-assigned this Sep 4, 2020
@HunDunDM HunDunDM added the component/schedule Scheduling logic. label Sep 4, 2020
@HunDunDM HunDunDM mentioned this pull request Sep 4, 2020
37 tasks
@Yisaer Yisaer self-requested a review September 7, 2020 08:17
peer := region.GetStoreVoter(pl.ToStore)
if peer == nil || peer.Id != pl.PeerID ||
(peer.Role != metapb.PeerRole_IncomingVoter && peer.Role != metapb.PeerRole_Voter) {
return 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about adding a log here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function will be executed repeatedly, and no error occurs here. What information needs to be logged here?

(notInJointState && count != 0) ||
(inJointState && count != len(cpe.PromoteLearners)+len(cpe.DemoteVoters)) {
// change is not atomic, or there are other peers in the joint state
return errors.New("unexpected peer role")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better to add more detail, or we cannot know which condition fail

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Different error messages have been returned for different situations. PTAL

@HunDunDM HunDunDM changed the title schedule: add some operator steps related to joint consensus [WIP] schedule: add some operator steps related to joint consensus Sep 7, 2020
@HunDunDM HunDunDM changed the title [WIP] schedule: add some operator steps related to joint consensus schedule: add some operator steps related to joint consensus Sep 8, 2020
Signed-off-by: Zheng Xiangsheng <[email protected]>
Signed-off-by: Zheng Xiangsheng <[email protected]>
Signed-off-by: Zheng Xiangsheng <[email protected]>
Copy link
Contributor

@Yisaer Yisaer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 9, 2020
@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Sep 9, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Sep 9, 2020
@disksing
Copy link
Contributor

disksing commented Sep 9, 2020

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Sep 9, 2020
@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@HunDunDM merge failed.

@HunDunDM
Copy link
Member Author

HunDunDM commented Sep 9, 2020

/run-integration-lightning-test

@HunDunDM HunDunDM merged commit 3b1c037 into tikv:master Sep 9, 2020
@HunDunDM HunDunDM deleted the joint/steps branch September 9, 2020 11:26
HunDunDM added a commit to HunDunDM/pd that referenced this pull request Sep 16, 2020
* schedule: add some operator steps related to joint consensus

Signed-off-by: Zheng Xiangsheng <[email protected]>

* refine errors

Signed-off-by: Zheng Xiangsheng <[email protected]>

* rename

Signed-off-by: Zheng Xiangsheng <[email protected]>

* address comment

Signed-off-by: Zheng Xiangsheng <[email protected]>

Co-authored-by: ti-srebot <[email protected]>
Signed-off-by: Zheng Xiangsheng <[email protected]>
# Conflicts:
#	server/schedule/operator_controller.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/schedule Scheduling logic. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants