Skip to content

Commit

Permalink
kv: return error from ResponseKeyIterate with nil response
Browse files Browse the repository at this point in the history
This commit updates ResponseKeyIterate to reject a nonsensical case.

Epic: None
Release note: None
  • Loading branch information
nvanbenschoten committed Mar 26, 2024
1 parent 349fabb commit b85aaa8
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 3 deletions.
4 changes: 2 additions & 2 deletions pkg/kv/kvpb/batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ func (ba *BatchRequest) RefreshSpanIterate(br *BatchResponse, fn func(roachpb.Sp
if br != nil {
resp = br.Responses[i].GetInner()
}
if ba.WaitPolicy == lock.WaitPolicy_SkipLocked && CanSkipLocked(req) {
if ba.WaitPolicy == lock.WaitPolicy_SkipLocked && CanSkipLocked(req) && resp != nil {
if ba.IndexFetchSpec != nil {
return errors.AssertionFailedf("unexpectedly IndexFetchSpec is set with SKIP LOCKED wait policy")
}
Expand Down Expand Up @@ -574,7 +574,7 @@ func ActualSpan(req Request, resp Response) (roachpb.Span, bool) {
// COL_BATCH_RESPONSE scan format.
func ResponseKeyIterate(req Request, resp Response, fn func(roachpb.Key)) error {
if resp == nil {
return nil
return errors.Errorf("cannot iterate over response keys of %s request with nil response", req.Method())
}
switch v := resp.(type) {
case *GetResponse:
Expand Down
6 changes: 6 additions & 0 deletions pkg/kv/kvpb/batch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,12 @@ func TestResponseKeyIterate(t *testing.T) {
resp: &GetResponse{Value: &roachpb.Value{}},
expKeys: []roachpb.Key{keyA},
},
{
name: "get, no response",
req: &GetRequest{RequestHeader: pointHeader},
resp: nil,
expErr: "cannot iterate over response keys of Get request with nil response",
},
{
name: "scan, missing",
req: &ScanRequest{RequestHeader: rangeHeader, ScanFormat: KEY_VALUES},
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/replica_tscache.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func (r *Replica) updateTimestampCache(
header := req.Header()
start, end := header.Key, header.EndKey

if ba.WaitPolicy == lock.WaitPolicy_SkipLocked && kvpb.CanSkipLocked(req) {
if ba.WaitPolicy == lock.WaitPolicy_SkipLocked && kvpb.CanSkipLocked(req) && resp != nil {
if ba.IndexFetchSpec != nil {
log.Errorf(ctx, "%v", errors.AssertionFailedf("unexpectedly IndexFetchSpec is set with SKIP LOCKED wait policy"))
}
Expand Down

0 comments on commit b85aaa8

Please sign in to comment.