Skip to content

Commit

Permalink
client: avoid panic when leader gRPC conn is nil (#7911)
Browse files Browse the repository at this point in the history
close #7910

Signed-off-by: Cabinfever_B <[email protected]>
  • Loading branch information
CabinfeverB authored Mar 13, 2024
1 parent 96590de commit 7d22c4f
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 3 deletions.
4 changes: 2 additions & 2 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -745,7 +745,7 @@ func (c *client) GetAllMembers(ctx context.Context) ([]*pdpb.Member, error) {
// follower pd client and the context which holds forward information.
func (c *client) getClientAndContext(ctx context.Context) (pdpb.PDClient, context.Context) {
serviceClient := c.pdSvcDiscovery.GetServiceClient()
if serviceClient == nil {
if serviceClient == nil || serviceClient.GetClientConn() == nil {
return nil, ctx
}
return pdpb.NewPDClient(serviceClient.GetClientConn()), serviceClient.BuildGRPCTargetContext(ctx, true)
Expand All @@ -762,7 +762,7 @@ func (c *client) getRegionAPIClientAndContext(ctx context.Context, allowFollower
}
}
serviceClient = c.pdSvcDiscovery.GetServiceClient()
if serviceClient == nil {
if serviceClient == nil || serviceClient.GetClientConn() == nil {
return nil, ctx
}
return serviceClient, serviceClient.BuildGRPCTargetContext(ctx, !allowFollower)
Expand Down
3 changes: 2 additions & 1 deletion client/pd_service_discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ type ServiceDiscovery interface {
type ServiceClient interface {
// GetURL returns the client url of the PD/etcd server.
GetURL() string
// GetClientConn returns the gRPC connection of the service client
// GetClientConn returns the gRPC connection of the service client.
// It returns nil if the connection is not available.
GetClientConn() *grpc.ClientConn
// BuildGRPCTargetContext builds a context object with a gRPC context.
// ctx: the original context object.
Expand Down
39 changes: 39 additions & 0 deletions tests/integrations/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -787,6 +787,45 @@ func (suite *followerForwardAndHandleTestSuite) TestGetTsoAndRegionByFollowerFor
})
}

func (suite *followerForwardAndHandleTestSuite) TestGetRegionFromLeaderWhenNetworkErr() {
re := suite.Require()
ctx, cancel := context.WithCancel(suite.ctx)
defer cancel()

cluster := suite.cluster
re.NotEmpty(cluster.WaitLeader())
leader := cluster.GetLeaderServer()

follower := cluster.GetServer(cluster.GetFollower())
re.NoError(failpoint.Enable("github.com/tikv/pd/client/grpcutil/unreachableNetwork2", fmt.Sprintf("return(\"%s\")", follower.GetAddr())))

cli := setupCli(re, ctx, suite.endpoints)
defer cli.Close()

cluster.GetLeaderServer().GetServer().GetMember().ResignEtcdLeader(ctx, leader.GetServer().Name(), follower.GetServer().Name())
re.NotEmpty(cluster.WaitLeader())

// here is just for trigger the leader change.
cli.GetRegion(context.Background(), []byte("a"))

testutil.Eventually(re, func() bool {
return cli.GetLeaderURL() == follower.GetAddr()
})
r, err := cli.GetRegion(context.Background(), []byte("a"))
re.Error(err)
re.Nil(r)

re.NoError(failpoint.Disable("github.com/tikv/pd/client/grpcutil/unreachableNetwork2"))
cli.GetServiceDiscovery().CheckMemberChanged()
testutil.Eventually(re, func() bool {
r, err = cli.GetRegion(context.Background(), []byte("a"))
if err == nil && r != nil {
return true
}
return false
})
}

func (suite *followerForwardAndHandleTestSuite) TestGetRegionFromFollower() {
re := suite.Require()
ctx, cancel := context.WithCancel(suite.ctx)
Expand Down

0 comments on commit 7d22c4f

Please sign in to comment.