diff --git a/docs/generated/settings/settings.html b/docs/generated/settings/settings.html index 7dfae0dbece3..d6f3cc9e42b8 100644 --- a/docs/generated/settings/settings.html +++ b/docs/generated/settings/settings.html @@ -112,6 +112,6 @@ trace.debug.enablebooleanfalseif set, traces for recent requests can be seen in the /debug page trace.lightstep.tokenstringif set, traces go to Lightstep using this token trace.zipkin.collectorstringif set, traces go to the given Zipkin instance (example: '127.0.0.1:9411'); ignored if trace.lightstep.token is set -versioncustom validation2.1-10set the active cluster version in the format '.' +versioncustom validation2.1-11set the active cluster version in the format '.' diff --git a/pkg/kv/txn_coord_sender_test.go b/pkg/kv/txn_coord_sender_test.go index a6103b18abaa..5eedee9a0745 100644 --- a/pkg/kv/txn_coord_sender_test.go +++ b/pkg/kv/txn_coord_sender_test.go @@ -404,6 +404,7 @@ func getTxn(ctx context.Context, txn *client.Txn) (*roachpb.Transaction, *roachp } ba := roachpb.BatchRequest{} + ba.Timestamp = txnMeta.Timestamp ba.Add(qt) db := txn.DB() diff --git a/pkg/settings/cluster/cockroach_versions.go b/pkg/settings/cluster/cockroach_versions.go index c5bcc0baa0d5..07754ad21ca2 100644 --- a/pkg/settings/cluster/cockroach_versions.go +++ b/pkg/settings/cluster/cockroach_versions.go @@ -66,6 +66,7 @@ const ( VersionDirectImport VersionSideloadedStorageNoReplicaID // see versionsSingleton for details VersionPushTxnToInclusive + VersionQueryTxnTimestamp // Add new versions here (step one of two). @@ -443,6 +444,11 @@ var versionsSingleton = keyedVersions([]keyedVersion{ Key: VersionPushTxnToInclusive, Version: roachpb.Version{Major: 2, Minor: 1, Unstable: 10}, }, + { + // VersionQueryTxnTimestamp is https://github.com/cockroachdb/cockroach/pull/36307. + Key: VersionQueryTxnTimestamp, + Version: roachpb.Version{Major: 2, Minor: 1, Unstable: 11}, + }, // Add new versions here (step two of two). diff --git a/pkg/storage/batcheval/cmd_push_txn.go b/pkg/storage/batcheval/cmd_push_txn.go index a791f594da8c..58a287f851ba 100644 --- a/pkg/storage/batcheval/cmd_push_txn.go +++ b/pkg/storage/batcheval/cmd_push_txn.go @@ -115,18 +115,20 @@ func PushTxn( if h.Txn != nil { return result.Result{}, ErrTransactionUnsupported } - - // Verify that the PushTxn's timestamp is not less than the timestamp that - // the request intends to push the transaction to. Transactions should not - // be pushed into the future or their effect may not be fully reflected in - // a future leaseholder's timestamp cache. This is analogous to how reads - // should not be performed at a timestamp in the future. if h.Timestamp.Less(args.PushTo) { - return result.Result{}, errors.Errorf("PushTo %s larger than PushRequest header timestamp %s", args.PushTo, h.Timestamp) + // Verify that the PushTxn's timestamp is not less than the timestamp that + // the request intends to push the transaction to. Transactions should not + // be pushed into the future or their effect may not be fully reflected in + // a future leaseholder's timestamp cache. This is analogous to how reads + // should not be performed at a timestamp in the future. + return result.Result{}, errors.Errorf("request timestamp %s less than PushTo timestamp %s", h.Timestamp, args.PushTo) + } + if h.Timestamp.Less(args.PusheeTxn.Timestamp) { + // This condition must hold for the timestamp cache access/update to be safe. + return result.Result{}, errors.Errorf("request timestamp %s less than pushee txn timestamp %s", h.Timestamp, args.PusheeTxn.Timestamp) } - if !bytes.Equal(args.Key, args.PusheeTxn.Key) { - return result.Result{}, errors.Errorf("request key %s should match pushee's txn key %s", args.Key, args.PusheeTxn.Key) + return result.Result{}, errors.Errorf("request key %s should match pushee txn key %s", args.Key, args.PusheeTxn.Key) } key := keys.TransactionKey(args.PusheeTxn.Key, args.PusheeTxn.ID) diff --git a/pkg/storage/batcheval/cmd_query_txn.go b/pkg/storage/batcheval/cmd_query_txn.go index 56e4cc2c1d15..99def7979eb3 100644 --- a/pkg/storage/batcheval/cmd_query_txn.go +++ b/pkg/storage/batcheval/cmd_query_txn.go @@ -20,6 +20,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/storage/batcheval/result" "github.com/cockroachdb/cockroach/pkg/storage/engine" "github.com/cockroachdb/cockroach/pkg/storage/spanset" @@ -49,11 +50,20 @@ func QueryTxn( ctx context.Context, batch engine.ReadWriter, cArgs CommandArgs, resp roachpb.Response, ) (result.Result, error) { args := cArgs.Args.(*roachpb.QueryTxnRequest) + h := cArgs.Header reply := resp.(*roachpb.QueryTxnResponse) - if cArgs.Header.Txn != nil { + if h.Txn != nil { return result.Result{}, ErrTransactionUnsupported } + // TODO(nvanbenschoten): old clusters didn't attach header timestamps to + // QueryTxn requests, so only perform this check for clusters that will + // always attach a valid timestamps. + checkHeaderTS := cArgs.EvalCtx.ClusterSettings().Version.IsActive(cluster.VersionQueryTxnTimestamp) + if h.Timestamp.Less(args.Txn.Timestamp) && checkHeaderTS { + // This condition must hold for the timestamp cache access to be safe. + return result.Result{}, errors.Errorf("request timestamp %s less than txn timestamp %s", h.Timestamp, args.Txn.Timestamp) + } if !bytes.Equal(args.Key, args.Txn.Key) { return result.Result{}, errors.Errorf("request key %s does not match txn key %s", args.Key, args.Txn.Key) } diff --git a/pkg/storage/batcheval/cmd_recover_txn.go b/pkg/storage/batcheval/cmd_recover_txn.go index 5e5c1f815538..8a73463de8ea 100644 --- a/pkg/storage/batcheval/cmd_recover_txn.go +++ b/pkg/storage/batcheval/cmd_recover_txn.go @@ -64,8 +64,8 @@ func RecoverTxn( return result.Result{}, errors.Errorf("request key %s does not match txn key %s", args.Key, args.Txn.Key) } if h.Timestamp.Less(args.Txn.Timestamp) { - // This condition must hold for the timestamp cache update to be safe. - return result.Result{}, errors.Errorf("request timestamp %v less than txn timestamp %v", h.Timestamp, args.Txn.Timestamp) + // This condition must hold for the timestamp cache access/update to be safe. + return result.Result{}, errors.Errorf("request timestamp %s less than txn timestamp %s", h.Timestamp, args.Txn.Timestamp) } key := keys.TransactionKey(args.Txn.Key, args.Txn.ID) diff --git a/pkg/storage/replica_command.go b/pkg/storage/replica_command.go index 497e216c08ce..b994ffecf524 100644 --- a/pkg/storage/replica_command.go +++ b/pkg/storage/replica_command.go @@ -374,6 +374,9 @@ func (r *Replica) AdminMerge( // descriptor end key. We look up the descriptor here only to get // the new end key and then repeat the lookup inside the // transaction. + // TODO(nvanbenschoten): Why can't we do this in the transaction? + // Performing an initial read won't affect the transaction record + // placement. { var rightDesc roachpb.RangeDescriptor if err := r.store.DB().GetProto(ctx, rightDescKey, &rightDesc); err != nil { diff --git a/pkg/storage/txnrecovery/manager.go b/pkg/storage/txnrecovery/manager.go index 8eda45812084..cba8dca00c55 100644 --- a/pkg/storage/txnrecovery/manager.go +++ b/pkg/storage/txnrecovery/manager.go @@ -252,6 +252,7 @@ func (m *manager) resolveIndeterminateCommitForTxnProbe( // write is prevented, or we run out of in-flight writes to query. for len(queryIntentReqs) > 0 { var b client.Batch + b.Header.Timestamp = m.clock.Now() b.AddRawRequest(&queryTxnReq) for i := 0; i < defaultBatchSize && len(queryIntentReqs) > 0; i++ { b.AddRawRequest(&queryIntentReqs[0]) diff --git a/pkg/storage/txnwait/txnqueue.go b/pkg/storage/txnwait/txnqueue.go index a43934f6d2ec..5550f7c7f939 100644 --- a/pkg/storage/txnwait/txnqueue.go +++ b/pkg/storage/txnwait/txnqueue.go @@ -821,6 +821,7 @@ func (q *Queue) queryTxnStatus( now hlc.Timestamp, ) (*roachpb.Transaction, []uuid.UUID, *roachpb.Error) { b := &client.Batch{} + b.Header.Timestamp = q.store.Clock().Now() b.AddRawRequest(&roachpb.QueryTxnRequest{ RequestHeader: roachpb.RequestHeader{ Key: txnMeta.Key,