diff --git a/pkg/sql/conn_executor_exec.go b/pkg/sql/conn_executor_exec.go index c0b8144d1419..73f37daf45d0 100644 --- a/pkg/sql/conn_executor_exec.go +++ b/pkg/sql/conn_executor_exec.go @@ -2113,6 +2113,18 @@ func (ex *connExecutor) execStmtInOpenStateWithPausablePortal( return nil, nil, nil } +func (ex *connExecutor) stepReadSequence(ctx context.Context) error { + prevSteppingMode := ex.state.mu.txn.ConfigureStepping(ctx, kv.SteppingEnabled) + if prevSteppingMode == kv.SteppingEnabled { + if err := ex.state.mu.txn.Step(ctx, false /* allowReadTimestampStep */); err != nil { + return err + } + } else { + ex.state.mu.txn.ConfigureStepping(ctx, prevSteppingMode) + } + return nil +} + // handleAOST gets the AsOfSystemTime clause from the statement, and sets // the timestamps of the transaction accordingly. func (ex *connExecutor) handleAOST(ctx context.Context, stmt tree.Statement) error { @@ -2385,13 +2397,8 @@ func (ex *connExecutor) commitSQLTransactionInternal(ctx context.Context) (retEr // write-write contention between transactions by inflating the contention // footprint of each transaction (i.e. the duration measured in MVCC time that // the transaction holds locks). - prevSteppingMode := ex.state.mu.txn.ConfigureStepping(ctx, kv.SteppingEnabled) - if prevSteppingMode == kv.SteppingEnabled { - if err := ex.state.mu.txn.Step(ctx, false /* allowReadTimestampStep */); err != nil { - return err - } - } else { - ex.state.mu.txn.ConfigureStepping(ctx, prevSteppingMode) + if err := ex.stepReadSequence(ctx); err != nil { + return err } if err := ex.createJobs(ctx); err != nil { @@ -2516,7 +2523,14 @@ func (ex *connExecutor) rollbackSQLTransaction( isKVTxnOpen = ex.state.mu.txn.IsOpen() } if isKVTxnOpen { - if err := ex.state.mu.txn.Rollback(ctx); err != nil { + // Step the read sequence before rolling back because the read sequence + // may be in the span reverted by a savepoint. + err := ex.stepReadSequence(ctx) + err = errors.CombineErrors(err, ex.state.mu.txn.Rollback(ctx)) + if err != nil { + if buildutil.CrdbTestBuild && errors.IsAssertionFailure(err) { + log.Dev.Fatalf(ctx, "txn rollback failed: %+v", err) + } log.Dev.Warningf(ctx, "txn rollback failed: %s", err) } } diff --git a/pkg/sql/logictest/testdata/logic_test/savepoints b/pkg/sql/logictest/testdata/logic_test/savepoints index a9786e9ddcbd..f2e22205fc9d 100644 --- a/pkg/sql/logictest/testdata/logic_test/savepoints +++ b/pkg/sql/logictest/testdata/logic_test/savepoints @@ -33,3 +33,22 @@ SELECT * FROM a statement ok COMMIT + +# Regression test for https://github.com/cockroachdb/cockroach/issues/135118. +# Rolling back a savepoint before rolling back the transaction caused the +# transaction rollback to fail. + +statement ok +BEGIN + +statement ok +SAVEPOINT sp1 + +statement ok +INSERT INTO a(id) VALUES (1); + +statement ok +ROLLBACK TO SAVEPOINT sp1 + +statement ok +ROLLBACK