diff --git a/storage/replica_command.go b/storage/replica_command.go index f15eb06610e8..dda9f99e1299 100644 --- a/storage/replica_command.go +++ b/storage/replica_command.go @@ -434,25 +434,25 @@ func (r *Replica) BeginTransaction( reply.Txn = &clonedTxn // Verify transaction does not already exist. - txn := roachpb.Transaction{} - ok, err := engine.MVCCGetProto(ctx, batch, key, hlc.ZeroTimestamp, true, nil, &txn) + tmpTxn := roachpb.Transaction{} + ok, err := engine.MVCCGetProto(ctx, batch, key, hlc.ZeroTimestamp, true, nil, &tmpTxn) if err != nil { return reply, err } if ok { - switch txn.Status { + switch tmpTxn.Status { case roachpb.ABORTED: // Check whether someone has come in ahead and already aborted the // txn. return reply, roachpb.NewTransactionAbortedError() case roachpb.PENDING: - if h.Txn.Epoch > txn.Epoch { + if h.Txn.Epoch > tmpTxn.Epoch { // On a transaction retry there will be an extant txn record // but this run should have an upgraded epoch. The extant txn // record may have been pushed or otherwise updated, so update // this command's txn and rewrite the record. - reply.Txn.Update(&txn) + reply.Txn.Update(&tmpTxn) } else { // Our txn record already exists. This is either a client error, sending // a duplicate BeginTransaction, or it's an artefact of DistSender @@ -462,12 +462,12 @@ func (r *Replica) BeginTransaction( case roachpb.COMMITTED: return reply, roachpb.NewTransactionStatusError( - fmt.Sprintf("BeginTransaction can't overwrite %s", txn), + fmt.Sprintf("BeginTransaction can't overwrite %s", tmpTxn), ) default: return reply, roachpb.NewTransactionStatusError( - fmt.Sprintf("bad txn state: %s", txn), + fmt.Sprintf("bad txn state: %s", tmpTxn), ) } } @@ -482,7 +482,7 @@ func (r *Replica) BeginTransaction( // (which may have been written before this entry). // // See #9265. - if txn.LastActive().Less(threshold) { + if reply.Txn.LastActive().Less(threshold) { return reply, roachpb.NewTransactionAbortedError() } diff --git a/storage/replica_test.go b/storage/replica_test.go index 8ac5de594bac..edb900417085 100644 --- a/storage/replica_test.go +++ b/storage/replica_test.go @@ -2511,6 +2511,17 @@ func TestEndTransactionTxnSpanGCThreshold(t *testing.T) { t.Fatalf("expected txn aborted error, got %v and response %+v", pErr, resp) } } + + // A transaction which starts later (i.e. at a higher timestamp) should not + // be prevented from writing its record. + // See #9522. + { + txn := newTransaction("foo", key, 1, enginepb.SERIALIZABLE, tc.clock) + beginArgs, header := beginTxnArgs(key, txn) + if _, pErr := tc.SendWrappedWith(header, &beginArgs); pErr != nil { + t.Fatal(pErr) + } + } } // TestEndTransactionDeadline_1PC verifies that a transaction that