Skip to content

Commit da1c4c6

Browse files
craig[bot]stevendanna
andcommitted
Merge #148791
148791: kvpb: fix NPE in test-only assertion r=tbg a=stevendanna This assertion was recently added but didn't account for the case when a batch was being evaluated as 1PC, in which case the transaction has been stripped off. Fixes #148788 Release note: None Co-authored-by: Steven Danna <[email protected]>
2 parents 39d519b + 91ab232 commit da1c4c6

File tree

2 files changed

+36
-0
lines changed

2 files changed

+36
-0
lines changed

pkg/kv/kvpb/api.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2562,6 +2562,12 @@ func validateExclusionTimestampForBatch(ts hlc.Timestamp, h Header) error {
25622562
return nil
25632563
}
25642564

2565+
// If we don't have a transaction, we can't know whether
2566+
// CanForwardReadTimestamp is valid or not.
2567+
if h.Txn == nil {
2568+
return nil
2569+
}
2570+
25652571
// Unless the IsoLevel allows per-statement read snapshots,
25662572
// CanForwardReadTimestamp implies we haven't served a read, so it makes no
25672573
// sense that ExpectExclusionSince would be set since it is the result of a

pkg/kv/kvpb/api_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"testing"
1212

1313
"github.com/cockroachdb/cockroach/pkg/kv/kvnemesis/kvnemesisutil"
14+
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency/isolation"
1415
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency/lock"
1516
"github.com/cockroachdb/cockroach/pkg/roachpb"
1617
"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
@@ -384,6 +385,35 @@ func TestGetValidate(t *testing.T) {
384385
})
385386
}
386387

388+
func TestDeleteValidate(t *testing.T) {
389+
t.Run("ExpectExclusionSinceWithCanForwardReadTimestamp", func(t *testing.T) {
390+
delReq := &DeleteRequest{
391+
ExpectExclusionSince: hlc.Timestamp{WallTime: 1},
392+
}
393+
require.Error(t, delReq.Validate(Header{
394+
CanForwardReadTimestamp: true,
395+
Txn: &roachpb.Transaction{},
396+
}))
397+
})
398+
t.Run("ExpectExclusionSinceWithCanForwardReadTimestampAtWeakerIsolation", func(t *testing.T) {
399+
delReq := &DeleteRequest{
400+
ExpectExclusionSince: hlc.Timestamp{WallTime: 1},
401+
}
402+
txn := &roachpb.Transaction{}
403+
txn.IsoLevel = isolation.ReadCommitted
404+
require.NoError(t, delReq.Validate(Header{
405+
CanForwardReadTimestamp: true,
406+
Txn: txn,
407+
}))
408+
})
409+
t.Run("ExpectExclusionSinceWithCanForwardReadTimestampButNoTxn", func(t *testing.T) {
410+
delReq := &DeleteRequest{
411+
ExpectExclusionSince: hlc.Timestamp{WallTime: 1},
412+
}
413+
require.NoError(t, delReq.Validate(Header{CanForwardReadTimestamp: true}))
414+
})
415+
}
416+
387417
func TestRequestHeaderRoundTrip(t *testing.T) {
388418
var seq kvnemesisutil.Container
389419
seq.Set(123)

0 commit comments

Comments
 (0)