Skip to content

Commit 9e96bee

Browse files
committed
kvcoord: fix QueryLocks and LeaseInfo handling in txnWriteBuffer
This commit fixes an oversight in how we handled QueryLocks and LeaseInfo requests in the txnWriteBuffer. We simply pass the request and the response through, but we forgot to properly propagate the response out. The bug was exposed in a recent refactor 65476fc. Release note: None
1 parent 442b45c commit 9e96bee

File tree

2 files changed

+19
-4
lines changed

2 files changed

+19
-4
lines changed

pkg/kv/kvclient/kvcoord/txn_interceptor_write_buffer.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1168,11 +1168,18 @@ func (rr requestRecord) toResp(
11681168
case *kvpb.QueryLocksRequest, *kvpb.LeaseInfoRequest:
11691169
// These requests don't interact with buffered writes, so we simply
11701170
// let the response through unchanged.
1171+
ru = br
11711172

11721173
default:
11731174
return ru, kvpb.NewError(unsupportedMethodError(req.Method()))
11741175
}
11751176

1177+
if buildutil.CrdbTestBuild {
1178+
if ru.GetInner() == nil {
1179+
panic(errors.AssertionFailedf("expected response to be set for type %T", rr.origRequest))
1180+
}
1181+
}
1182+
11761183
return ru, nil
11771184
}
11781185

pkg/kv/kvclient/kvcoord/txn_interceptor_write_buffer_test.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -270,15 +270,21 @@ func TestTxnWriteBufferBlindWritesIncludingOtherRequests(t *testing.T) {
270270
getB := &kvpb.GetRequest{RequestHeader: kvpb.RequestHeader{Key: keyB}}
271271
delC := delArgs(keyC, txn.Sequence)
272272
scanDE := &kvpb.ScanRequest{RequestHeader: kvpb.RequestHeader{Key: keyD, EndKey: keyE}}
273+
queryLocks := &kvpb.QueryLocksRequest{RequestHeader: kvpb.RequestHeader{Key: keyA, EndKey: keyE}, IncludeUncontended: true}
274+
leaseInfo := &kvpb.LeaseInfoRequest{}
273275
ba.Add(putA)
274276
ba.Add(getB)
275277
ba.Add(delC)
276278
ba.Add(scanDE)
279+
ba.Add(queryLocks)
280+
ba.Add(leaseInfo)
277281

278282
mockSender.MockSend(func(ba *kvpb.BatchRequest) (*kvpb.BatchResponse, *kvpb.Error) {
279-
require.Len(t, ba.Requests, 2)
283+
require.Len(t, ba.Requests, 4)
280284
require.IsType(t, &kvpb.GetRequest{}, ba.Requests[0].GetInner())
281285
require.IsType(t, &kvpb.ScanRequest{}, ba.Requests[1].GetInner())
286+
require.IsType(t, &kvpb.QueryLocksRequest{}, ba.Requests[2].GetInner())
287+
require.IsType(t, &kvpb.LeaseInfoRequest{}, ba.Requests[3].GetInner())
282288

283289
br := ba.CreateReply()
284290
br.Txn = ba.Txn
@@ -289,13 +295,15 @@ func TestTxnWriteBufferBlindWritesIncludingOtherRequests(t *testing.T) {
289295
require.Nil(t, pErr)
290296
require.NotNil(t, br)
291297

292-
// Expect 4 responses, even though only 2 KV requests were sent. Moreover,
293-
// ensure that the responses are in the correct order.
294-
require.Len(t, br.Responses, 4)
298+
// Expect 6 responses, even though only 4 KV requests were sent. Moreover,
299+
// ensure that the responses are in the correct order and non-nil.
300+
require.Len(t, br.Responses, 6)
295301
require.IsType(t, &kvpb.PutResponse{}, br.Responses[0].GetInner())
296302
require.IsType(t, &kvpb.GetResponse{}, br.Responses[1].GetInner())
297303
require.IsType(t, &kvpb.DeleteResponse{}, br.Responses[2].GetInner())
298304
require.IsType(t, &kvpb.ScanResponse{}, br.Responses[3].GetInner())
305+
require.IsType(t, &kvpb.QueryLocksResponse{}, br.Responses[4].GetInner())
306+
require.IsType(t, &kvpb.LeaseInfoResponse{}, br.Responses[5].GetInner())
299307

300308
// Verify the writes were buffered correctly.
301309
expBufferedWrites := []bufferedWrite{

0 commit comments

Comments
 (0)