Skip to content

Commit 2a79cc3

Browse files
committed
internal/client: close stream on first recv error
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
1 parent 3d4b3ea commit 2a79cc3

2 files changed

Lines changed: 98 additions & 0 deletions

File tree

internal/client/client.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,7 @@ func (c *RPCClient) getCopStreamResponse(ctx context.Context, client tikvpb.Tikv
464464
first, err = copStream.Recv()
465465
if err != nil {
466466
if errors.Cause(err) != io.EOF {
467+
copStream.Close()
467468
return nil, errors.WithStack(err)
468469
}
469470
logutil.BgLogger().Debug("copstream returns nothing for the request.")
@@ -501,6 +502,7 @@ func (c *RPCClient) getBatchCopStreamResponse(ctx context.Context, client tikvpb
501502
first, err = copStream.Recv()
502503
if err != nil {
503504
if errors.Cause(err) != io.EOF {
505+
copStream.Close()
504506
return nil, errors.WithStack(err)
505507
}
506508
logutil.BgLogger().Debug("batch copstream returns nothing for the request.")
@@ -535,6 +537,7 @@ func (c *RPCClient) getMPPStreamResponse(ctx context.Context, client tikvpb.Tikv
535537
first, err = copStream.Recv()
536538
if err != nil {
537539
if errors.Cause(err) != io.EOF {
540+
copStream.Close()
538541
return nil, errors.WithStack(err)
539542
}
540543
}

internal/client/client_test.go

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import (
5151
"github.com/pingcap/kvproto/pkg/coprocessor"
5252
"github.com/pingcap/kvproto/pkg/kvrpcpb"
5353
"github.com/pingcap/kvproto/pkg/metapb"
54+
"github.com/pingcap/kvproto/pkg/mpp"
5455
"github.com/pingcap/kvproto/pkg/tikvpb"
5556
"github.com/pkg/errors"
5657
"github.com/stretchr/testify/assert"
@@ -61,10 +62,104 @@ import (
6162
"github.com/tikv/client-go/v2/tikvrpc"
6263
"github.com/tikv/client-go/v2/util/async"
6364
"go.uber.org/zap"
65+
"google.golang.org/grpc"
6466
"google.golang.org/grpc/connectivity"
6567
"google.golang.org/grpc/metadata"
6668
)
6769

70+
type firstRecvErrTikvClient struct {
71+
tikvpb.TikvClient
72+
streamCtx context.Context
73+
err error
74+
}
75+
76+
func (c *firstRecvErrTikvClient) CoprocessorStream(ctx context.Context, _ *coprocessor.Request, _ ...grpc.CallOption) (tikvpb.Tikv_CoprocessorStreamClient, error) {
77+
c.streamCtx = ctx
78+
return &firstRecvErrCopStream{err: c.err}, nil
79+
}
80+
81+
func (c *firstRecvErrTikvClient) BatchCoprocessor(ctx context.Context, _ *coprocessor.BatchRequest, _ ...grpc.CallOption) (tikvpb.Tikv_BatchCoprocessorClient, error) {
82+
c.streamCtx = ctx
83+
return &firstRecvErrBatchCopStream{err: c.err}, nil
84+
}
85+
86+
func (c *firstRecvErrTikvClient) EstablishMPPConnection(ctx context.Context, _ *mpp.EstablishMPPConnectionRequest, _ ...grpc.CallOption) (tikvpb.Tikv_EstablishMPPConnectionClient, error) {
87+
c.streamCtx = ctx
88+
return &firstRecvErrMPPStream{err: c.err}, nil
89+
}
90+
91+
type firstRecvErrCopStream struct {
92+
grpc.ClientStream
93+
err error
94+
}
95+
96+
func (s *firstRecvErrCopStream) Recv() (*coprocessor.Response, error) {
97+
return nil, s.err
98+
}
99+
100+
type firstRecvErrBatchCopStream struct {
101+
grpc.ClientStream
102+
err error
103+
}
104+
105+
func (s *firstRecvErrBatchCopStream) Recv() (*coprocessor.BatchResponse, error) {
106+
return nil, s.err
107+
}
108+
109+
type firstRecvErrMPPStream struct {
110+
grpc.ClientStream
111+
err error
112+
}
113+
114+
func (s *firstRecvErrMPPStream) Recv() (*mpp.MPPDataPacket, error) {
115+
return nil, s.err
116+
}
117+
118+
func TestStreamFirstRecvErrorClosesLease(t *testing.T) {
119+
rpcClient := &RPCClient{}
120+
recvErr := errors.New("first recv failed")
121+
cases := []struct {
122+
name string
123+
req *tikvrpc.Request
124+
call func(*firstRecvErrTikvClient, *tikvrpc.Request, *connPool) (*tikvrpc.Response, error)
125+
}{
126+
{
127+
name: "cop stream",
128+
req: tikvrpc.NewRequest(tikvrpc.CmdCopStream, &coprocessor.Request{}),
129+
call: func(client *firstRecvErrTikvClient, req *tikvrpc.Request, connPool *connPool) (*tikvrpc.Response, error) {
130+
return rpcClient.getCopStreamResponse(context.Background(), client, req, time.Minute, connPool)
131+
},
132+
},
133+
{
134+
name: "batch cop stream",
135+
req: tikvrpc.NewRequest(tikvrpc.CmdBatchCop, &coprocessor.BatchRequest{}),
136+
call: func(client *firstRecvErrTikvClient, req *tikvrpc.Request, connPool *connPool) (*tikvrpc.Response, error) {
137+
return rpcClient.getBatchCopStreamResponse(context.Background(), client, req, time.Minute, connPool)
138+
},
139+
},
140+
{
141+
name: "mpp stream",
142+
req: tikvrpc.NewRequest(tikvrpc.CmdMPPConn, &mpp.EstablishMPPConnectionRequest{}),
143+
call: func(client *firstRecvErrTikvClient, req *tikvrpc.Request, connPool *connPool) (*tikvrpc.Response, error) {
144+
return rpcClient.getMPPStreamResponse(context.Background(), client, req, time.Minute, connPool)
145+
},
146+
},
147+
}
148+
149+
for _, tt := range cases {
150+
t.Run(tt.name, func(t *testing.T) {
151+
client := &firstRecvErrTikvClient{err: recvErr}
152+
connPool := &connPool{streamTimeout: make(chan *tikvrpc.Lease, 1)}
153+
154+
resp, err := tt.call(client, tt.req, connPool)
155+
156+
require.Nil(t, resp)
157+
require.ErrorContains(t, err, recvErr.Error())
158+
require.Equal(t, context.Canceled, client.streamCtx.Err())
159+
})
160+
}
161+
}
162+
68163
func TestConn(t *testing.T) {
69164
defer config.UpdateGlobal(func(conf *config.Config) {
70165
conf.TiKVClient.MaxBatchSize = 0

0 commit comments

Comments
 (0)