kv: temporarily disable parallel commits

This commit allows us to merge #36012 without waiting for #37469. We can
revert this commit once the changes in #37469 land.

The fear of merging #36012 without #37469 and without disabling parallel
commits is that we would then be adding a new transaction state transition
without actually performing implicit commits in parallel with the rest of
the transaction's writes. This appears to slow down nightly benchmarks by
about 5-10%.

Release note: None

docs/generated/settings/settings.html

@@ -53,7 +53,7 @@
 <tr><td><code>kv.snapshot_recovery.max_rate</code></td><td>byte size</td><td><code>8.0 MiB</code></td><td>the rate limit (bytes/sec) to use for recovery snapshots</td></tr>
 <tr><td><code>kv.transaction.max_intents_bytes</code></td><td>integer</td><td><code>262144</code></td><td>maximum number of bytes used to track write intents in transactions</td></tr>
 <tr><td><code>kv.transaction.max_refresh_spans_bytes</code></td><td>integer</td><td><code>256000</code></td><td>maximum number of bytes used to track refresh spans in serializable transactions</td></tr>
-<tr><td><code>kv.transaction.parallel_commits_enabled</code></td><td>boolean</td><td><code>true</code></td><td>if enabled, transactional commits will be parallelized with transactional writes</td></tr>
+<tr><td><code>kv.transaction.parallel_commits_enabled</code></td><td>boolean</td><td><code>false</code></td><td>if enabled, transactional commits will be parallelized with transactional writes</td></tr>
 <tr><td><code>kv.transaction.write_pipelining_enabled</code></td><td>boolean</td><td><code>true</code></td><td>if enabled, transactional writes are pipelined through Raft consensus</td></tr>
 <tr><td><code>kv.transaction.write_pipelining_max_batch_size</code></td><td>integer</td><td><code>128</code></td><td>if non-zero, defines that maximum size batch that will be pipelined through Raft consensus</td></tr>
 <tr><td><code>kv.transaction.write_pipelining_max_outstanding_size</code></td><td>byte size</td><td><code>256 KiB</code></td><td>maximum number of bytes used to track in-flight pipelined writes before disabling pipelining</td></tr>

pkg/kv/txn_interceptor_committer.go

@@ -28,7 +28,7 @@ import (
 var parallelCommitsEnabled = settings.RegisterBoolSetting(
 	"kv.transaction.parallel_commits_enabled",
 	"if enabled, transactional commits will be parallelized with transactional writes",
-	true,
+	false,
 )
 
 // txnCommitter is a txnInterceptor that concerns itself with committing and
@@ -152,7 +152,13 @@ func (tc *txnCommitter) SendLocked(
 	// write set; no writes will be in-flight concurrently with the EndTransaction
 	// request.
 	if len(et.InFlightWrites) > 0 && !tc.canCommitInParallelWithWrites(ba, et) {
-		et.IntentSpans = mergeIntoSpans(et.IntentSpans, et.InFlightWrites)
+		// NB: when parallel commits is disabled, this is the best place to
+		// detect whether the batch has only distinct spans. We can set this
+		// flag based on whether any of previously declared in-flight writes
+		// in this batch overlap with each other. This will have (rare) false
+		// negatives when the in-flight writes overlap with existing intent
+		// spans, but never false positives.
+		et.IntentSpans, ba.Header.DistinctSpans = mergeIntoSpans(et.IntentSpans, et.InFlightWrites)
 		// Disable parallel commits.
 		et.InFlightWrites = nil
 	}
@@ -223,7 +229,7 @@ func (tc *txnCommitter) SendLocked(
 	// determination about the status of our STAGING transaction. To avoid this,
 	// we transition to an explicitly committed transaction as soon as possible.
 	// This also has the side-effect of kicking off intent resolution.
-	mergedIntentSpans := mergeIntoSpans(et.IntentSpans, et.InFlightWrites)
+	mergedIntentSpans, _ := mergeIntoSpans(et.IntentSpans, et.InFlightWrites)
 	tc.makeTxnCommitExplicitAsync(ctx, br.Txn, mergedIntentSpans)
 
 	// Switch the status on the batch response's transaction to COMMITTED. No
@@ -317,15 +323,17 @@ func (tc *txnCommitter) canCommitInParallelWithWrites(
 	return true
 }
 
-func mergeIntoSpans(s []roachpb.Span, ws []roachpb.SequencedWrite) []roachpb.Span {
+// mergeIntoSpans merges all provided sequenced writes into the span slice. It
+// then sorts the spans and merges an that overlap. Returns true iff all of the
+// spans are distinct.
+func mergeIntoSpans(s []roachpb.Span, ws []roachpb.SequencedWrite) ([]roachpb.Span, bool) {
 	if s == nil {
 		s = make([]roachpb.Span, 0, len(ws))
 	}
 	for _, w := range ws {
 		s = append(s, roachpb.Span{Key: w.Key})
 	}
-	s, _ = roachpb.MergeSpans(s)
-	return s
+	return roachpb.MergeSpans(s)
 }
 
 // needTxnRetryAfterStaging determines whether the transaction needs to refresh

pkg/kv/txn_interceptor_committer_test.go

@@ -28,9 +28,12 @@ import (
 )
 
 func makeMockTxnCommitter() (txnCommitter, *mockLockedSender) {
+	st := cluster.MakeTestingClusterSettings()
+	// TODO(nvanbenschoten): remove when parallel commits are enabled.
+	parallelCommitsEnabled.Override(&st.SV, true)
 	mockSender := &mockLockedSender{}
 	return txnCommitter{
-		st:      cluster.MakeTestingClusterSettings(),
+		st:      st,
 		stopper: stop.NewStopper(),
 		wrapped: mockSender,
 		mu:      new(syncutil.Mutex),