Merge pull request #14900 from ahrtr/fix_readyonly_txn_panic_3.4_20221206

[3.4] etcdserver: fix nil pointer panic for readonly txn
This commit is contained in:
Benjamin Wang 2022-12-06 19:25:12 +08:00 committed by GitHub
commit 593711848e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 109 additions and 1 deletions

View File

@ -138,7 +138,11 @@ func warnOfExpensiveReadOnlyTxnRequest(lg *zap.Logger, warningApplyDuration time
for _, r := range txnResponse.Responses {
switch op := r.Response.(type) {
case *pb.ResponseOp_ResponseRange:
resps = append(resps, fmt.Sprintf("range_response_count:%d", len(op.ResponseRange.Kvs)))
if op.ResponseRange != nil {
resps = append(resps, fmt.Sprintf("range_response_count:%d", len(op.ResponseRange.Kvs)))
} else {
resps = append(resps, "range_response:nil")
}
default:
// only range responses should be in a read only txn request
}

View File

@ -20,10 +20,12 @@ import (
"time"
"go.uber.org/zap"
"go.uber.org/zap/zaptest"
"go.etcd.io/etcd/etcdserver/api/membership"
"go.etcd.io/etcd/etcdserver/api/rafthttp"
"go.etcd.io/etcd/etcdserver/api/snap"
pb "go.etcd.io/etcd/etcdserver/etcdserverpb"
"go.etcd.io/etcd/pkg/types"
"go.etcd.io/etcd/raft/raftpb"
)
@ -110,3 +112,105 @@ type testStringerFunc func() string
func (s testStringerFunc) String() string {
return s()
}
// TestWarnOfExpensiveReadOnlyTxnRequest verifies WarnOfExpensiveReadOnlyTxnRequest
// never panic no matter what data the txnResponse contains.
func TestWarnOfExpensiveReadOnlyTxnRequest(t *testing.T) {
testCases := []struct {
name string
txnResp *pb.TxnResponse
}{
{
name: "all readonly responses",
txnResp: &pb.TxnResponse{
Responses: []*pb.ResponseOp{
{
Response: &pb.ResponseOp_ResponseRange{
ResponseRange: &pb.RangeResponse{},
},
},
{
Response: &pb.ResponseOp_ResponseRange{
ResponseRange: &pb.RangeResponse{},
},
},
},
},
},
{
name: "all readonly responses with partial nil responses",
txnResp: &pb.TxnResponse{
Responses: []*pb.ResponseOp{
{
Response: &pb.ResponseOp_ResponseRange{
ResponseRange: &pb.RangeResponse{},
},
},
{
Response: &pb.ResponseOp_ResponseRange{
ResponseRange: nil,
},
},
},
},
},
{
name: "all readonly responses with all nil responses",
txnResp: &pb.TxnResponse{
Responses: []*pb.ResponseOp{
{
Response: &pb.ResponseOp_ResponseRange{
ResponseRange: nil,
},
},
{
Response: &pb.ResponseOp_ResponseRange{
ResponseRange: nil,
},
},
},
},
},
{
name: "partial non readonly responses",
txnResp: &pb.TxnResponse{
Responses: []*pb.ResponseOp{
{
Response: &pb.ResponseOp_ResponseRange{
ResponseRange: nil,
},
},
{
Response: &pb.ResponseOp_ResponsePut{},
},
{
Response: &pb.ResponseOp_ResponseDeleteRange{},
},
},
},
},
{
name: "all non readonly responses",
txnResp: &pb.TxnResponse{
Responses: []*pb.ResponseOp{
{
Response: &pb.ResponseOp_ResponsePut{},
},
{
Response: &pb.ResponseOp_ResponseDeleteRange{},
},
},
},
},
}
for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
lg := zaptest.NewLogger(t)
start := time.Now().Add(-1 * time.Second)
// WarnOfExpensiveReadOnlyTxnRequest shouldn't panic.
warnOfExpensiveReadOnlyTxnRequest(lg, 0, start, &pb.TxnRequest{}, tc.txnResp, nil)
})
}
}