From ebbe06416500db57670feb9e578b6ef6c9b4fae0 Mon Sep 17 00:00:00 2001 From: Nathan VanBenschoten Date: Tue, 23 Jul 2019 14:20:20 -0400 Subject: [PATCH] storage: remain compatible with transactions missing their minimum timestamp Fixes #39008. This was missed in #38782. I must have been thinking about the behavior we'll want to switch to in v20.1. Luckily, the issue was easily caught by `tpcc/mixed-headroom/n5cpu16`. Release note: None --- pkg/storage/batcheval/transaction.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/pkg/storage/batcheval/transaction.go b/pkg/storage/batcheval/transaction.go index 97b44f2100d3..f107a485559a 100644 --- a/pkg/storage/batcheval/transaction.go +++ b/pkg/storage/batcheval/transaction.go @@ -114,9 +114,19 @@ func CanPushWithPriority(pusher, pushee *roachpb.Transaction) bool { func CanCreateTxnRecord(rec EvalContext, txn *roachpb.Transaction) error { // Provide the transaction's minimum timestamp. The transaction could not // have written a transaction record previously with a timestamp below this. - txnMinTS := txn.MinTimestamp - if txnMinTS.IsEmpty() { - return errors.Errorf("no minimum transaction timestamp provided: %v", txn) + // + // We use InclusiveTimeBounds to remain backward compatible. However, if we + // don't need to worry about compatibility, we require the transaction to + // have a minimum timestamp field. + // TODO(nvanbenschoten): Replace this with txn.MinTimestamp in v20.1. + txnMinTS, _ := txn.InclusiveTimeBounds() + if util.RaceEnabled { + newTxnMinTS := txn.MinTimestamp + if newTxnMinTS.IsEmpty() { + return errors.Errorf("no minimum transaction timestamp provided: %v", txn) + } else if newTxnMinTS != txnMinTS { + return errors.Errorf("minimum transaction timestamp differs from lower time bound: %v", txn) + } } ok, minCommitTS, reason := rec.CanCreateTxnRecord(txn.ID, txn.Key, txnMinTS) if !ok {