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: change merge match peers strategy #1339

Merged
merged 7 commits into from
Dec 6, 2018
Merged

Conversation

Connor1996
Copy link
Member

@Connor1996 Connor1996 commented Nov 22, 2018

What problem does this PR solve?

There is a case that a follower is added and transfer leader to it, and the apply process of it is slow so leader regards it as a voter but actually it is still a learner. Once that, the follower can't be a leader, but the old leader can't know that so there is no leader to serve for a while.

What is changed and how it works?

change merge match peers strategy, transfer leader to the first added follower if there is no intersection store, and remove one peer after adding one peer.

Check List

  • Unit test

Copy link
Member

@zhangjinpeng87 zhangjinpeng87 left a comment

Choose a reason for hiding this comment

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

LGTM

@nolouch
Copy link
Contributor

nolouch commented Nov 26, 2018

@Connor1996 CI failed.

@Connor1996
Copy link
Member Author

@nolouch Because range cannot keep same order every time. It is a little hard to write this test case, and I am trying to figure it out.

@disksing
Copy link
Contributor

@Connor1996 Any update? Maybe you can make sure regions have only 1 intersection.

Signed-off-by: Connor1996 <zbk602423539@gmail.com>
Signed-off-by: Connor1996 <zbk602423539@gmail.com>
@Connor1996
Copy link
Member Author

PTAL @disksing @zhangjinpeng1987

@Connor1996 Connor1996 changed the title schedule: change merge leader strategy schedule: change merge match peers strategy Nov 30, 2018
Signed-off-by: Connor1996 <zbk602423539@gmail.com>
steps = append(steps, RemovePeer{FromStore: peer.GetStoreId()})
kind |= OpRegion
index++
Copy link
Member

Choose a reason for hiding this comment

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

Please check if index == len(toAdds) after this loop.

Copy link
Contributor

@nolouch nolouch left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Connor1996 <zbk602423539@gmail.com>
@nolouch nolouch merged commit 6ebba48 into tikv:master Dec 6, 2018
Connor1996 added a commit to Connor1996/pd that referenced this pull request Dec 11, 2018
Connor1996 added a commit to Connor1996/pd that referenced this pull request Dec 11, 2018
Connor1996 added a commit that referenced this pull request Dec 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants