-
Notifications
You must be signed in to change notification settings - Fork 759
server: fix the leader cannot election after pd leader lost while etcd leader intact #6447
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
Changes from 5 commits
0acf937
9eaa004
2998eee
2ffb021
14e9f16
6d810b4
7f9b6ba
4353bf7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,7 @@ import ( | |
| "bytes" | ||
| "context" | ||
| "fmt" | ||
| "math/rand" | ||
| "net/http" | ||
| "os" | ||
| "path" | ||
|
|
@@ -105,6 +106,9 @@ const ( | |
| maxRetryTimesGetServicePrimary = 25 | ||
| // retryIntervalGetServicePrimary is the retry interval for getting primary addr. | ||
| retryIntervalGetServicePrimary = 100 * time.Millisecond | ||
|
|
||
| lostPDLeaderMaxTimeoutSecs = 10 | ||
| lostPDLeaderReElectionFactor = 10 | ||
| ) | ||
|
|
||
| // EtcdStartTimeout the timeout of the startup etcd. | ||
|
|
@@ -1432,6 +1436,14 @@ func (s *Server) leaderLoop() { | |
| } | ||
|
|
||
| leader, checkAgain := s.member.CheckLeader() | ||
| // add failpoint to test leader check go to stuck. | ||
| failpoint.Inject("leaderLoopCheckAgain", func(val failpoint.Value) { | ||
| memberString := val.(string) | ||
| memberID, _ := strconv.ParseUint(memberString, 10, 64) | ||
| if s.member.ID() == memberID { | ||
| checkAgain = true | ||
| } | ||
| }) | ||
| if checkAgain { | ||
| continue | ||
| } | ||
|
|
@@ -1459,6 +1471,25 @@ func (s *Server) leaderLoop() { | |
| // To make sure the etcd leader and PD leader are on the same server. | ||
| etcdLeader := s.member.GetEtcdLeader() | ||
| if etcdLeader != s.member.ID() { | ||
| if s.member.GetLeader() == nil { | ||
| lastUpdated := s.member.GetLastLeaderUpdatedTime() | ||
| // use random timeout to avoid leader campaigning storm. | ||
| randomTimeout := time.Duration(rand.Intn(int(lostPDLeaderMaxTimeoutSecs)))*time.Second + lostPDLeaderMaxTimeoutSecs*time.Second + lostPDLeaderReElectionFactor*s.cfg.ElectionInterval.Duration | ||
| // add failpoint to test the campaign leader logic. | ||
| failpoint.Inject("timeoutWaitPDLeader", func() { | ||
| log.Info("timeoutWaitPDLeader is injected, skip wait other etcd leader be etcd leader") | ||
| randomTimeout = time.Duration(rand.Intn(10))*time.Millisecond + 100*time.Millisecond | ||
| }) | ||
| if lastUpdated.Add(randomTimeout).Before(time.Now()) && !lastUpdated.IsZero() && etcdLeader != 0 { | ||
| log.Info("the pd leader is lost for a long time, try to re-campaign a pd leader with resign etcd leader", | ||
| zap.Duration("timeout", randomTimeout), | ||
| zap.Time("last-updated", lastUpdated), | ||
| zap.String("current-leader-member-id", types.ID(etcdLeader).String()), | ||
| zap.String("transferee-member-id", types.ID(s.member.ID()).String()), | ||
| ) | ||
| s.member.MoveEtcdLeader(s.ctx, etcdLeader, s.member.ID()) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we consider the leader's priority?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In most cases, there are no priority. if there exists priority, it also not affect the priority because the higher priority will do move the leader again. so, I think we can keep simple with this implementation. |
||
| } | ||
| } | ||
| log.Info("skip campaigning of pd leader and check later", | ||
| zap.String("server-name", s.Name()), | ||
| zap.Uint64("etcd-leader-id", etcdLeader), | ||
|
|
@@ -1575,6 +1606,16 @@ func (s *Server) campaignLeader() { | |
| log.Info("no longer a leader because lease has expired, pd leader will step down") | ||
| return | ||
| } | ||
| // add failpoint to test exit leader, failpoint judge the member is the give value, then break | ||
| failpoint.Inject("exitCampaignLeader", func(val failpoint.Value) { | ||
| memberString := val.(string) | ||
| memberID, _ := strconv.ParseUint(memberString, 10, 64) | ||
| if s.member.ID() == memberID { | ||
| log.Info("exit PD leader") | ||
| failpoint.Return() | ||
| } | ||
| }) | ||
|
|
||
| etcdLeader := s.member.GetEtcdLeader() | ||
| if etcdLeader != s.member.ID() { | ||
| log.Info("etcd leader changed, resigns pd leadership", zap.String("old-pd-leader-name", s.Name())) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -248,6 +248,30 @@ func TestLeaderResignWithBlock(t *testing.T) { | |
| re.NoError(failpoint.Disable("github.com/tikv/pd/server/raftclusterIsBusy")) | ||
| } | ||
|
|
||
| func TestPDLeaderLostWhileEtcdLeaderIntact(t *testing.T) { | ||
| re := require.New(t) | ||
| ctx, cancel := context.WithCancel(context.Background()) | ||
| defer cancel() | ||
| cluster, err := tests.NewTestCluster(ctx, 2) | ||
| defer cluster.Destroy() | ||
| re.NoError(err) | ||
|
|
||
| err = cluster.RunInitialServers() | ||
| re.NoError(err) | ||
|
|
||
| leader1 := cluster.WaitLeader() | ||
| memberID := cluster.GetServer(leader1).GetLeader().GetMemberId() | ||
|
|
||
| re.NoError(failpoint.Enable("github.com/tikv/pd/server/leaderLoopCheckAgain", fmt.Sprintf("return(\"%d\")", memberID))) | ||
| re.NoError(failpoint.Enable("github.com/tikv/pd/server/exitCampaignLeader", fmt.Sprintf("return(\"%d\")", memberID))) | ||
| re.NoError(failpoint.Enable("github.com/tikv/pd/server/timeoutWaitPDLeader", `return(true)`)) | ||
| leader2 := waitLeaderChange(re, cluster, leader1) | ||
| re.NotEqual(leader1, leader2) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to check the duration that leader changing costs won't be too long?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. without this fix, this test will be failed after timeout after 30s in |
||
| re.NoError(failpoint.Disable("github.com/tikv/pd/server/leaderLoopCheckAgain")) | ||
| re.NoError(failpoint.Disable("github.com/tikv/pd/server/exitCampaignLeader")) | ||
| re.NoError(failpoint.Disable("github.com/tikv/pd/server/timeoutWaitPDLeader")) | ||
| } | ||
|
|
||
| func waitLeaderChange(re *require.Assertions, cluster *tests.TestCluster, old string) string { | ||
| var leader string | ||
| testutil.Eventually(re, func() bool { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file permission has been changed.