From b943d81edf820e12edfb4087ca50753acbe321d3 Mon Sep 17 00:00:00 2001 From: Nathan VanBenschoten Date: Mon, 9 Sep 2019 14:49:03 -0400 Subject: [PATCH 1/2] storage: fix flake in TestTxnRecordLifecycleTransitions Fixes #40537. The test was specifically not restarting transactions at the push timestamp (for the outdated epoch) cases, but the broken subtest TestTxnRecordLifecycleTransitions/push_transaction_(abort)_after_end_transaction_(stage)_with_outdated_epoch was expecting its timestamp to be equal to the push timestamp. Release note: None --- pkg/storage/replica_test.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/pkg/storage/replica_test.go b/pkg/storage/replica_test.go index cab0f3de3f93..3b011a4f33fa 100644 --- a/pkg/storage/replica_test.go +++ b/pkg/storage/replica_test.go @@ -10551,14 +10551,14 @@ func TestTxnRecordLifecycleTransitions(t *testing.T) { }, run: func(txn *roachpb.Transaction, now hlc.Timestamp) error { clone := txn.Clone() - clone.Timestamp.Forward(now) + clone.Timestamp = clone.Timestamp.Add(0, 1) pt := pushTxnArgs(pusher, clone, roachpb.PUSH_ABORT) return sendWrappedWithErr(roachpb.Header{}, &pt) }, expTxn: func(txn *roachpb.Transaction, pushTs hlc.Timestamp) roachpb.TransactionRecord { record := txnWithStatus(roachpb.ABORTED)(txn, pushTs) - record.Timestamp.Forward(pushTs) - record.LastHeartbeat.Forward(pushTs) + record.Timestamp = record.Timestamp.Add(0, 1) + record.LastHeartbeat = record.LastHeartbeat.Add(0, 1) record.Priority = pusher.Priority - 1 return record }, @@ -10608,7 +10608,7 @@ func TestTxnRecordLifecycleTransitions(t *testing.T) { expTxn: func(txn *roachpb.Transaction, pushTs hlc.Timestamp) roachpb.TransactionRecord { record := txnWithStatus(roachpb.ABORTED)(txn, pushTs) record.Epoch = txn.Epoch + 1 - record.Timestamp.Forward(pushTs) + record.Timestamp = record.Timestamp.Add(0, 1) record.LastHeartbeat = record.LastHeartbeat.Add(0, 1) record.Priority = pusher.Priority - 1 return record @@ -10642,7 +10642,7 @@ func TestTxnRecordLifecycleTransitions(t *testing.T) { }, { name: "heartbeat transaction with epoch bump after end transaction (abort)", - setup: func(txn *roachpb.Transaction, now hlc.Timestamp) error { + setup: func(txn *roachpb.Transaction, _ hlc.Timestamp) error { et, etH := endTxnArgs(txn, false /* commit */) return sendWrappedWithErr(etH, &et) }, @@ -10652,7 +10652,7 @@ func TestTxnRecordLifecycleTransitions(t *testing.T) { // threshold against this timestamp instead of its minimum // timestamp. clone := txn.Clone() - clone.Restart(-1, 0, now.Add(0, 1)) + clone.Restart(-1, 0, now) hb, hbH := heartbeatArgs(clone, now) return sendWrappedWithErr(hbH, &hb) }, @@ -11115,7 +11115,7 @@ func TestTxnRecordLifecycleTransitions(t *testing.T) { }, { name: "begin transaction after push transaction (abort)", - setup: func(txn *roachpb.Transaction, now hlc.Timestamp) error { + setup: func(txn *roachpb.Transaction, _ hlc.Timestamp) error { pt := pushTxnArgs(pusher, txn, roachpb.PUSH_ABORT) return sendWrappedWithErr(roachpb.Header{}, &pt) }, @@ -11128,7 +11128,7 @@ func TestTxnRecordLifecycleTransitions(t *testing.T) { }, { name: "heartbeat transaction after push transaction (abort)", - setup: func(txn *roachpb.Transaction, now hlc.Timestamp) error { + setup: func(txn *roachpb.Transaction, _ hlc.Timestamp) error { pt := pushTxnArgs(pusher, txn, roachpb.PUSH_ABORT) return sendWrappedWithErr(roachpb.Header{}, &pt) }, @@ -11141,7 +11141,7 @@ func TestTxnRecordLifecycleTransitions(t *testing.T) { }, { name: "heartbeat transaction with epoch bump after push transaction (abort)", - setup: func(txn *roachpb.Transaction, now hlc.Timestamp) error { + setup: func(txn *roachpb.Transaction, _ hlc.Timestamp) error { pt := pushTxnArgs(pusher, txn, roachpb.PUSH_ABORT) return sendWrappedWithErr(roachpb.Header{}, &pt) }, @@ -11151,7 +11151,7 @@ func TestTxnRecordLifecycleTransitions(t *testing.T) { // threshold against this timestamp instead of its minimum // timestamp. clone := txn.Clone() - clone.Restart(-1, 0, now.Add(0, 1)) + clone.Restart(-1, 0, now) hb, hbH := heartbeatArgs(clone, now) return sendWrappedWithErr(hbH, &hb) }, @@ -11160,7 +11160,7 @@ func TestTxnRecordLifecycleTransitions(t *testing.T) { }, { name: "end transaction (stage) after push transaction (abort)", - setup: func(txn *roachpb.Transaction, now hlc.Timestamp) error { + setup: func(txn *roachpb.Transaction, _ hlc.Timestamp) error { pt := pushTxnArgs(pusher, txn, roachpb.PUSH_ABORT) return sendWrappedWithErr(roachpb.Header{}, &pt) }, @@ -11174,7 +11174,7 @@ func TestTxnRecordLifecycleTransitions(t *testing.T) { }, { name: "end transaction (abort) after push transaction (abort)", - setup: func(txn *roachpb.Transaction, now hlc.Timestamp) error { + setup: func(txn *roachpb.Transaction, _ hlc.Timestamp) error { pt := pushTxnArgs(pusher, txn, roachpb.PUSH_ABORT) return sendWrappedWithErr(roachpb.Header{}, &pt) }, @@ -11188,7 +11188,7 @@ func TestTxnRecordLifecycleTransitions(t *testing.T) { }, { name: "end transaction (commit) after push transaction (abort)", - setup: func(txn *roachpb.Transaction, now hlc.Timestamp) error { + setup: func(txn *roachpb.Transaction, _ hlc.Timestamp) error { pt := pushTxnArgs(pusher, txn, roachpb.PUSH_ABORT) return sendWrappedWithErr(roachpb.Header{}, &pt) }, From fac112b82589ad18b4d8e9f10dca65e9196d192e Mon Sep 17 00:00:00 2001 From: Nathan VanBenschoten Date: Mon, 9 Sep 2019 14:54:25 -0400 Subject: [PATCH 2/2] storage: manually increment clock in TestTxnRecordLifecycleTransitions This ensures that we're not accidentally relying on the fact that `runTs` was usually one logical tick above `txn.Timestamp`. This has caused flakiness in the past (see previous commit). With this change, the flake fixed in the previous commit fails reliably. Release note: None --- pkg/storage/replica_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/storage/replica_test.go b/pkg/storage/replica_test.go index 3b011a4f33fa..05490d143468 100644 --- a/pkg/storage/replica_test.go +++ b/pkg/storage/replica_test.go @@ -11512,7 +11512,9 @@ func TestTxnRecordLifecycleTransitions(t *testing.T) { defer setTxnAutoGC(!c.disableTxnAutoGC)() txn := newTransaction(c.name, roachpb.Key(c.name), 1, tc.Clock()) + manual.Increment(99) runTs := tc.Clock().Now() + if c.setup != nil { if err := c.setup(txn, runTs); err != nil { t.Fatalf("failed during test setup: %+v", err)