From 759fdac56ec59c75aa872e8e00d0a614bcc593a3 Mon Sep 17 00:00:00 2001 From: Andrei Matei Date: Wed, 21 Apr 2021 18:29:35 -0400 Subject: [PATCH 1/2] kv: s/txn.Timestamp/txn.WriteTimestamp in comments txn.Timestamp had been renamed to WriteTimestamp, but sufficiently many comments were left dangling. Release note: None --- pkg/kv/kvserver/batcheval/cmd_push_txn.go | 2 +- pkg/kv/kvserver/replica_evaluate.go | 5 +++-- pkg/kv/txn.go | 2 +- pkg/roachpb/batch.go | 2 +- pkg/sql/catalog/lease/lease.go | 2 +- pkg/storage/mvcc.go | 8 ++++---- pkg/storage/mvcc_test.go | 2 +- 7 files changed, 12 insertions(+), 11 deletions(-) diff --git a/pkg/kv/kvserver/batcheval/cmd_push_txn.go b/pkg/kv/kvserver/batcheval/cmd_push_txn.go index 3103146222c8..5d4ee4b811d7 100644 --- a/pkg/kv/kvserver/batcheval/cmd_push_txn.go +++ b/pkg/kv/kvserver/batcheval/cmd_push_txn.go @@ -93,7 +93,7 @@ func declareKeysPushTransaction( // Push can proceed: the pushee's transaction record is modified and // rewritten, based on the value of args.PushType. If args.PushType // is PUSH_ABORT, txn.Status is set to ABORTED. If args.PushType is -// PUSH_TIMESTAMP, txn.Timestamp is set to just after args.PushTo. +// PUSH_TIMESTAMP, txn.WriteTimestamp is set to just after args.PushTo. // // If the pushee is aborted, its timestamp will be forwarded to match // its last client activity timestamp (i.e. last heartbeat), if available. diff --git a/pkg/kv/kvserver/replica_evaluate.go b/pkg/kv/kvserver/replica_evaluate.go index 70777d742686..434bfeb09b7c 100644 --- a/pkg/kv/kvserver/replica_evaluate.go +++ b/pkg/kv/kvserver/replica_evaluate.go @@ -451,8 +451,9 @@ func evaluateBatch( return nil, mergedResult, roachpb.NewErrorWithTxn(writeTooOldState.err, baHeader.Txn) } - // The batch evaluation will not return an error (i.e. either everything went fine or - // we're deferring a WriteTooOldError by having bumped baHeader.Txn.Timestamp). + // The batch evaluation will not return an error (i.e. either everything went + // fine or we're deferring a WriteTooOldError by having bumped + // baHeader.Txn.WriteTimestamp). // Update the batch response timestamp field to the timestamp at which the // batch's reads were evaluated. diff --git a/pkg/kv/txn.go b/pkg/kv/txn.go index 021ea7cd0ab5..29663d06cb9c 100644 --- a/pkg/kv/txn.go +++ b/pkg/kv/txn.go @@ -1148,7 +1148,7 @@ func (txn *Txn) SetFixedTimestamp(ctx context.Context, ts hlc.Timestamp) { // // The transaction's epoch is bumped, simulating to an extent what the // TxnCoordSender does on retriable errors. The transaction's timestamp is only -// bumped to the extent that txn.ReadTimestamp is racheted up to txn.Timestamp. +// bumped to the extent that txn.ReadTimestamp is racheted up to txn.WriteTimestamp. // TODO(andrei): This method should take in an up-to-date timestamp, but // unfortunately its callers don't currently have that handy. func (txn *Txn) GenerateForcedRetryableError(ctx context.Context, msg string) error { diff --git a/pkg/roachpb/batch.go b/pkg/roachpb/batch.go index 48f98bfc14b5..6cb9eaba9b79 100644 --- a/pkg/roachpb/batch.go +++ b/pkg/roachpb/batch.go @@ -41,7 +41,7 @@ func (h Header) WriteTimestamp() hlc.Timestamp { // SetActiveTimestamp sets the correct timestamp at which the request is to be // carried out. For transactional requests, ba.Timestamp must be zero initially // and it will be set to txn.ReadTimestamp (note though this mostly impacts -// reads; writes use txn.Timestamp). For non-transactional requests, if no +// reads; writes use txn.WriteTimestamp). For non-transactional requests, if no // timestamp is specified, nowFn is used to create and set one. func (ba *BatchRequest) SetActiveTimestamp(nowFn func() hlc.Timestamp) error { if txn := ba.Txn; txn != nil { diff --git a/pkg/sql/catalog/lease/lease.go b/pkg/sql/catalog/lease/lease.go index ff637b879b36..6892138e45c2 100644 --- a/pkg/sql/catalog/lease/lease.go +++ b/pkg/sql/catalog/lease/lease.go @@ -1270,7 +1270,7 @@ func makeNameCacheKey(parentID descpb.ID, parentSchemaID descpb.ID, name string) // API. The descriptor acquired needs to be released. A transaction // can use a descriptor as long as its timestamp is within the // validity window for the descriptor: -// descriptor.ModificationTime <= txn.Timestamp < expirationTime +// descriptor.ModificationTime <= txn.ReadTimestamp < expirationTime // // Exported only for testing. // diff --git a/pkg/storage/mvcc.go b/pkg/storage/mvcc.go index 0ff081440c65..069de3d23cdb 100644 --- a/pkg/storage/mvcc.go +++ b/pkg/storage/mvcc.go @@ -1373,7 +1373,7 @@ func replayTransactionalWrite( // Note that, when writing transactionally, the txn's timestamps // dictate the timestamp of the operation, and the timestamp parameter // is redundant. Specifically, the intent is written at the txn's -// provisional commit timestamp, txn.Timestamp, unless it is +// provisional commit timestamp, txn.WriteTimestamp, unless it is // forwarded by an existing committed value above that timestamp. // However, reads (e.g., for a ConditionalPut) are performed at the // txn's read timestamp (txn.ReadTimestamp) to ensure that the @@ -1472,9 +1472,9 @@ func mvccPutInternal( // Determine the read and write timestamps for the write. For a // non-transactional write, these will be identical. For a transactional - // write, we read at the transaction's original timestamp (forwarded by any - // refresh timestamp) but write intents at its provisional commit timestamp. - // See the comment on the txn.Timestamp field definition for rationale. + // write, we read at the transaction's read timestamp but write intents at its + // provisional commit timestamp. See the comment on the txn.WriteTimestamp field + // definition for rationale. readTimestamp := timestamp writeTimestamp := timestamp if txn != nil { diff --git a/pkg/storage/mvcc_test.go b/pkg/storage/mvcc_test.go index 41edc781bcf0..0e3a188a93f5 100644 --- a/pkg/storage/mvcc_test.go +++ b/pkg/storage/mvcc_test.go @@ -2988,7 +2988,7 @@ func TestMVCCResolveIntentTxnTimestampMismatch(t *testing.T) { tsEarly := txn.WriteTimestamp txn.TxnMeta.WriteTimestamp.Forward(tsEarly.Add(10, 0)) - // Write an intent which has txn.Timestamp > meta.timestamp. + // Write an intent which has txn.WriteTimestamp > meta.timestamp. if err := MVCCPut(ctx, engine, nil, testKey1, tsEarly, value1, txn); err != nil { t.Fatal(err) } From b0560375b6f87517002273b9a9550824d13df5d0 Mon Sep 17 00:00:00 2001 From: Yahor Yuzefovich Date: Wed, 21 Apr 2021 21:55:44 -0700 Subject: [PATCH 2/2] sql: allow 'experimental_always' for 'vectorize' cluster setting Originally, we disallowed it because it was hard to get out of that state. Now we support `SET CLUSTER SETTING` in a more native manner in the vectorized engine, so we can have an experimental value as the cluster setting. Also return the error that prompted us to wrap a row-execution processor into the vectorized flow rather than the error that the wrapping is prohibited. Release note: None --- pkg/sql/colexec/colbuilder/execplan.go | 4 +++- pkg/sql/exec_util.go | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/sql/colexec/colbuilder/execplan.go b/pkg/sql/colexec/colbuilder/execplan.go index 9795e561e1f3..515273cf49cc 100644 --- a/pkg/sql/colexec/colbuilder/execplan.go +++ b/pkg/sql/colexec/colbuilder/execplan.go @@ -704,7 +704,9 @@ func NewColOperator( resultPreSpecPlanningStateShallowCopy := *r if err = supportedNatively(spec); err != nil { - if err := canWrap(flowCtx.EvalCtx.SessionData.VectorizeMode, spec); err != nil { + if wrapErr := canWrap(flowCtx.EvalCtx.SessionData.VectorizeMode, spec); wrapErr != nil { + // Return the original error for why we don't support this spec + // natively since it is more interesting. return r, err } if log.V(1) { diff --git a/pkg/sql/exec_util.go b/pkg/sql/exec_util.go index e3777e588d09..c6f55a63207d 100644 --- a/pkg/sql/exec_util.go +++ b/pkg/sql/exec_util.go @@ -423,8 +423,9 @@ var VectorizeClusterMode = settings.RegisterEnumSetting( "default vectorize mode", "on", map[int64]string{ - int64(sessiondatapb.VectorizeOff): "off", - int64(sessiondatapb.VectorizeOn): "on", + int64(sessiondatapb.VectorizeOff): "off", + int64(sessiondatapb.VectorizeOn): "on", + int64(sessiondatapb.VectorizeExperimentalAlways): "experimental_always", }, )