sql: don't bump RC txn read timestamp before commit

Fixes cockroachdb#109628.

This commit removes the bumping of the read committed transactions' read
timestamp in `connExecutor.commitSQLTransactionInternal`. Bumping the
transaction's external read timestamp is not needed before committing,
and it causes the transaction to commit at a higher timestamp than
necessary. On highly contended workloads like the one from cockroachdb#109628, this
can cause unnecessary contention by inflating the contention footprint
of each transaction (i.e. the duration measured in the MVCC time domain
that the transaction holds locks).

By not bumping the read timestamp immediately before committed, we
improve the performance of contended workloads. For example, on the
workload from cockroachdb#109628, we see the following improvement:

\## Summary
```
Serializable:            232.6 tps
Read Committed (before): 225.3 tps
Read Committed (after):  236.0 tps
```
Read Committed improves by **4.7%** and is now **1.5%** faster than
Serializable on the workload.

\## Raw
```
\### Serializable

transaction type: tpcb-cockroach.sql                  transaction type: tpcb-cockroach.sql                  transaction type: tpcb-cockroach.sql
scaling factor: 10                                    scaling factor: 10                                    scaling factor: 10
query mode: simple                                    query mode: simple                                    query mode: simple
number of clients: 25                                 number of clients: 25                                 number of clients: 25
number of threads: 10                                 number of threads: 10                                 number of threads: 10
duration: 120 s                                       duration: 120 s                                       duration: 120 s
number of transactions actually processed: 27935      number of transactions actually processed: 27584      number of transactions actually processed: 28380
number of failed transactions: 0 (0.000%)             number of failed transactions: 0 (0.000%)             number of failed transactions: 0 (0.000%)
number of serialization failures: 0 (0.000%)          number of serialization failures: 0 (0.000%)          number of serialization failures: 0 (0.000%)
number of deadlock failures: 0 (0.000%)               number of deadlock failures: 0 (0.000%)               number of deadlock failures: 0 (0.000%)
number of transactions retried: 0 (0.000%)            number of transactions retried: 0 (0.000%)            number of transactions retried: 0 (0.000%)
total number of retries: 0                            total number of retries: 0                            total number of retries: 0
latency average = 107.483 ms                          latency average = 108.829 ms                          latency average = 105.776 ms
latency stddev = 264.163 ms                           latency stddev = 263.132 ms                           latency stddev = 244.713 ms
initial connection time = 12.315 ms                   initial connection time = 11.692 ms                   initial connection time = 9.448 ms
tps = 232.157370 (without initial connection time)    tps = 229.458565 (without initial connection time)    tps = 236.039695 (without initial connection time)

\### Read Committed (before)

transaction type: tpcb-cockroach.sql                  transaction type: tpcb-cockroach.sql                  transaction type: tpcb-cockroach.sql
scaling factor: 10                                    scaling factor: 10                                    scaling factor: 10
query mode: simple                                    query mode: simple                                    query mode: simple
number of clients: 25                                 number of clients: 25                                 number of clients: 25
number of threads: 10                                 number of threads: 10                                 number of threads: 10
duration: 120 s                                       duration: 120 s                                       duration: 120 s
number of transactions actually processed: 27220      number of transactions actually processed: 27143      number of transactions actually processed: 26966
number of failed transactions: 0 (0.000%)             number of failed transactions: 0 (0.000%)             number of failed transactions: 0 (0.000%)
number of serialization failures: 0 (0.000%)          number of serialization failures: 0 (0.000%)          number of serialization failures: 0 (0.000%)
number of deadlock failures: 0 (0.000%)               number of deadlock failures: 0 (0.000%)               number of deadlock failures: 0 (0.000%)
number of transactions retried: 0 (0.000%)            number of transactions retried: 0 (0.000%)            number of transactions retried: 0 (0.000%)
total number of retries: 0                            total number of retries: 0                            total number of retries: 0
latency average = 110.293 ms                          latency average = 110.646 ms                          latency average = 111.354 ms
latency stddev = 173.640 ms                           latency stddev = 180.792 ms                           latency stddev = 179.226 ms
initial connection time = 8.911 ms                    initial connection time = 9.894 ms                    initial connection time = 10.605 ms
tps = 226.389120 (without initial connection time)    tps = 225.427664 (without initial connection time)    tps = 224.059547 (without initial connection time)

\### Read Committed (after)

transaction type: tpcb-cockroach.sql                  transaction type: tpcb-cockroach.sql                  transaction type: tpcb-cockroach.sql
scaling factor: 10                                    scaling factor: 10                                    scaling factor: 10
query mode: simple                                    query mode: simple                                    query mode: simple
number of clients: 25                                 number of clients: 25                                 number of clients: 25
number of threads: 10                                 number of threads: 10                                 number of threads: 10
duration: 120 s                                       duration: 120 s                                       duration: 120 s
number of transactions actually processed: 28526      number of transactions actually processed: 28564      number of transactions actually processed: 28039
number of failed transactions: 0 (0.000%)             number of failed transactions: 0 (0.000%)             number of failed transactions: 0 (0.000%)
number of serialization failures: 0 (0.000%)          number of serialization failures: 0 (0.000%)          number of serialization failures: 0 (0.000%)
number of deadlock failures: 0 (0.000%)               number of deadlock failures: 0 (0.000%)               number of deadlock failures: 0 (0.000%)
number of transactions retried: 0 (0.000%)            number of transactions retried: 0 (0.000%)            number of transactions retried: 0 (0.000%)
total number of retries: 0                            total number of retries: 0                            total number of retries: 0
latency average = 105.221 ms                          latency average = 105.114 ms                          latency average = 107.065 ms
latency stddev = 196.386 ms                           latency stddev = 197.031 ms                           latency stddev = 203.784 ms
initial connection time = 12.549 ms                   initial connection time = 11.212 ms                   initial connection time = 7.715 ms
tps = 237.329979 (without initial connection time)    tps = 237.417802 (without initial connection time)    tps = 233.194327 (without initial connection time)
```

Release note: None

pkg/sql/conn_executor_exec.go

@@ -1438,12 +1438,21 @@ func (ex *connExecutor) commitSQLTransactionInternal(ctx context.Context) error
 
 	ex.extraTxnState.prepStmtsNamespace.closeAllPortals(ctx, &ex.extraTxnState.prepStmtsNamespaceMemAcc)
 
-	// We need to step the transaction before committing if it has stepping
-	// enabled. If it doesn't have stepping enabled, then we just set the
-	// stepping mode back to what it was.
+	// We need to step the transaction's internal read sequence before committing
+	// if it has stepping enabled. If it doesn't have stepping enabled, then we
+	// just set the stepping mode back to what it was.
+	//
+	// Even if we do step the transaction's internal read sequence, we do not
+	// advance its external read timestamp (applicable only to read committed
+	// transactions). This is because doing so is not needed before committing,
+	// and it would cause the transaction to commit at a higher timestamp than
+	// necessary. On heavily contended workloads like the one from #109628, this
+	// can cause unnecessary 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, true /* allowReadTimestampStep */); err != nil {
+		if err := ex.state.mu.txn.Step(ctx, false /* allowReadTimestampStep */); err != nil {
 			return err
 		}
 	} else {