Skip to content

Commit

Permalink
Revert "Use the PD leader member in server if it's not in etcd members (
Browse files Browse the repository at this point in the history
#7753)" (#7755)

ref #7752

This reverts commit d3b9307 from #7753.

Signed-off-by: JmPotato <ghzpotato@gmail.com>
  • Loading branch information
JmPotato authored Jan 25, 2024
1 parent d3b9307 commit 9a82b47
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 28 deletions.
8 changes: 4 additions & 4 deletions pkg/member/member.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ func (m *EmbeddedEtcdMember) KeepLeader(ctx context.Context) {

// PreCheckLeader does some pre-check before checking whether it's the leader.
func (m *EmbeddedEtcdMember) PreCheckLeader() error {
if m.GetEtcdLeaderID() == 0 {
if m.GetEtcdLeader() == 0 {
return errs.ErrEtcdLeaderNotFound
}
return nil
Expand Down Expand Up @@ -281,7 +281,7 @@ func (m *EmbeddedEtcdMember) ResetLeader() {

// CheckPriority checks whether the etcd leader should be moved according to the priority.
func (m *EmbeddedEtcdMember) CheckPriority(ctx context.Context) {
etcdLeader := m.GetEtcdLeaderID()
etcdLeader := m.GetEtcdLeader()
if etcdLeader == m.ID() || etcdLeader == 0 {
return
}
Expand Down Expand Up @@ -318,8 +318,8 @@ func (m *EmbeddedEtcdMember) MoveEtcdLeader(ctx context.Context, old, new uint64
return nil
}

// GetEtcdLeaderID returns the etcd leader ID.
func (m *EmbeddedEtcdMember) GetEtcdLeaderID() uint64 {
// GetEtcdLeader returns the etcd leader ID.
func (m *EmbeddedEtcdMember) GetEtcdLeader() uint64 {
return m.etcd.Server.Lead()
}

Expand Down
2 changes: 1 addition & 1 deletion server/api/member.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func getMembers(svr *server.Server) (*pdpb.GetMembersResponse, error) {
if e != nil {
log.Error("failed to load deploy path", zap.Uint64("member", m.GetMemberId()), errs.ZapError(e))
}
if svr.GetMember().GetEtcdLeaderID() == 0 {
if svr.GetMember().GetEtcdLeader() == 0 {
log.Warn("no etcd leader, skip get leader priority", zap.Uint64("member", m.GetMemberId()))
continue
}
Expand Down
13 changes: 2 additions & 11 deletions server/grpc_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -473,9 +473,9 @@ func (s *GrpcServer) GetMembers(context.Context, *pdpb.GetMembersRequest) (*pdpb
}

var etcdLeader, pdLeader *pdpb.Member
etcdLeaderID := s.member.GetEtcdLeaderID()
leaderID := s.member.GetEtcdLeader()
for _, m := range members {
if m.MemberId == etcdLeaderID {
if m.MemberId == leaderID {
etcdLeader = m
break
}
Expand All @@ -499,15 +499,6 @@ func (s *GrpcServer) GetMembers(context.Context, *pdpb.GetMembersRequest) (*pdpb
break
}
}
failpoint.Inject("noLeaderInMembers", func() {
pdLeader = nil
})
// If the leader is not in the member list, we should fallback to
// the `leader` set in the PD server to gain a better availability.
// See https://github.com/tikv/pd/issues/7752 for more details.
if pdLeader == nil && leader.GetMemberId() == etcdLeaderID {
pdLeader = leader
}

return &pdpb.GetMembersResponse{
Header: s.header(),
Expand Down
4 changes: 2 additions & 2 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1645,7 +1645,7 @@ func (s *Server) leaderLoop() {
}

// To make sure the etcd leader and PD leader are on the same server.
etcdLeader := s.member.GetEtcdLeaderID()
etcdLeader := s.member.GetEtcdLeader()
if etcdLeader != s.member.ID() {
if s.member.GetLeader() == nil {
lastUpdated := s.member.GetLastLeaderUpdatedTime()
Expand Down Expand Up @@ -1800,7 +1800,7 @@ func (s *Server) campaignLeader() {
}
})

etcdLeader := s.member.GetEtcdLeaderID()
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()))
return
Expand Down
12 changes: 2 additions & 10 deletions tests/server/cluster/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -543,19 +543,11 @@ func TestGetPDMembers(t *testing.T) {
grpcPDClient := testutil.MustNewGrpcClient(re, leaderServer.GetAddr())
clusterID := leaderServer.GetClusterID()
req := &pdpb.GetMembersRequest{Header: testutil.NewRequestHeader(clusterID)}
resp, err := grpcPDClient.GetMembers(ctx, req)
resp, err := grpcPDClient.GetMembers(context.Background(), req)
re.NoError(err)
re.Equal(pdpb.ErrorType_OK, resp.GetHeader().GetError().GetType())
// A more strict test can be found at api/member_test.go
re.NotEmpty(resp.GetMembers())
re.Equal(leaderServer.GetLeader(), resp.GetLeader())
// Test the member list does not include the PD leader.
re.NoError(failpoint.Enable("github.com/tikv/pd/server/noLeaderInMembers", `return(true)`))
resp, err = grpcPDClient.GetMembers(ctx, req)
re.NoError(err)
re.Equal(pdpb.ErrorType_OK, resp.GetHeader().GetError().GetType())
re.NotEmpty(resp.GetMembers())
re.Equal(leaderServer.GetLeader(), resp.GetLeader())
re.NoError(failpoint.Disable("github.com/tikv/pd/server/noLeaderInMembers"))
}

func TestNotLeader(t *testing.T) {
Expand Down

0 comments on commit 9a82b47

Please sign in to comment.