Skip to content

Commit

Permalink
Merge #35667
Browse files Browse the repository at this point in the history
35667: intentresolver: fix test flake due to timing of batches r=ajwerner a=ajwerner

Before this change we'd assume that two requests addressed to the same range
would end up in the same batch. This patch adopts the more flexible logic
introduced in #35273 to resolve similar test flakes in TestCleanupIntents.
This new infrastructure behaves correctly when batches are sent due to time
with less batching than the test assumed.

Fixes #35340.

Release note: None

Co-authored-by: Andrew Werner <[email protected]>
  • Loading branch information
craig[bot] and ajwerner committed Mar 13, 2019
2 parents 48885a4 + 601b37c commit 41e67d0
Showing 1 changed file with 20 additions and 15 deletions.
35 changes: 20 additions & 15 deletions pkg/storage/intentresolver/intent_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func TestCleanupTxnIntentsOnGCAsync(t *testing.T) {
type testCase struct {
txn *roachpb.Transaction
intents []roachpb.Intent
sendFuncs []sendFunc
sendFuncs *sendFuncs
expectPushed bool
expectSucceed bool
}
Expand All @@ -115,14 +115,17 @@ func TestCleanupTxnIntentsOnGCAsync(t *testing.T) {
txn2.Status = roachpb.COMMITTED
cases := []*testCase{
// This one has an unexpired pending transaction so it's skipped
{txn: txn0},
{
txn: txn0,
sendFuncs: newSendFuncs(t),
},
// Txn1 is pending and expired so the code should attempt to push the txn.
// The provided sender will fail on the first request. The callback should
// indicate that the transaction was pushed but that the resolution was not
// successful.
{
txn: txn1,
sendFuncs: []sendFunc{failSendFunc},
sendFuncs: newSendFuncs(t, failSendFunc),
expectPushed: true,
},
// Txn1 is pending and expired so the code should attempt to push the txn
Expand All @@ -143,11 +146,11 @@ func TestCleanupTxnIntentsOnGCAsync(t *testing.T) {
Txn: txn1.TxnMeta,
},
},
sendFuncs: []sendFunc{
sendFuncs: newSendFuncs(t,
singlePushTxnSendFunc(t),
resolveIntentsSendFunc(t),
failSendFunc,
},
),
expectPushed: true,
},
// Txn1 is pending and expired so the code should attempt to push the txn
Expand All @@ -164,12 +167,15 @@ func TestCleanupTxnIntentsOnGCAsync(t *testing.T) {
{Span: roachpb.Span{Key: roachpb.Key("aa")}, Txn: txn1.TxnMeta},
{Span: roachpb.Span{Key: key, EndKey: roachpb.Key("b")}, Txn: txn1.TxnMeta},
},
sendFuncs: []sendFunc{
singlePushTxnSendFunc(t),
resolveIntentsSendFunc(t),
resolveIntentsSendFunc(t),
gcSendFunc(t),
},
sendFuncs: func() *sendFuncs {
s := newSendFuncs(t)
s.pushFrontLocked(
singlePushTxnSendFunc(t),
resolveIntentsSendFuncs(s, 3, 2),
gcSendFunc(t),
)
return s
}(),
expectPushed: true,
expectSucceed: true,
},
Expand All @@ -179,15 +185,14 @@ func TestCleanupTxnIntentsOnGCAsync(t *testing.T) {
{
txn: txn2,
intents: []roachpb.Intent{},
sendFuncs: []sendFunc{gcSendFunc(t)},
sendFuncs: newSendFuncs(t, gcSendFunc(t)),
expectSucceed: true,
},
}

for _, c := range cases {
t.Run("", func(t *testing.T) {
sf := &sendFuncs{sendFuncs: c.sendFuncs}
ir := newIntentResolverWithSendFuncs(cfg, sf)
ir := newIntentResolverWithSendFuncs(cfg, c.sendFuncs)
var didPush, didSucceed bool
done := make(chan struct{})
onComplete := func(pushed, succeeded bool) {
Expand All @@ -199,7 +204,7 @@ func TestCleanupTxnIntentsOnGCAsync(t *testing.T) {
t.Fatalf("unexpected error sending async transaction")
}
<-done
if sf.len() != 0 {
if c.sendFuncs.len() != 0 {
t.Errorf("Not all send funcs called")
}
if didSucceed != c.expectSucceed {
Expand Down

0 comments on commit 41e67d0

Please sign in to comment.