Merge #144736

144736: kvclient,sql: harden the code for buffered writes r=yuzefovich a=yuzefovich

**kvclient: remove no longer valid assertion about batch splitting**

This commit removes a fatal assertion that batch with MaxSpanRequestKeys
or TargetBytes set doesn't need to be split. This assertion has been in
place for many years, but it no longer holds true in some cases with
buffered writes (namely when we buffered some writes, but then they were
disabled, so we need to flush the buffer together with the next batch:
that next batch can fail the assertion).

**sql: disable buffered writes on the txn when it's used by IE**

This commit is a follow-up to 07d8902
where we disabled the session variable that controls whether buffered
writes are enabled when the variable is consulted by the internal
executor. That change had an oversight - that if the IE is using already
created txn, the session variable has no influence, so this commit
explicitly disables buffered writes on the txn whenever it's used by the
IE for the first time.

Epic: None
Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>

Author: craig[bot]

pkg/kv/kvclient/kvcoord/dist_sender.go

@@ -1222,12 +1222,6 @@ func (ds *DistSender) Send(
 		splitET = true
 	}
 	parts := splitBatchAndCheckForRefreshSpans(ba, splitET)
-	if len(parts) > 1 && (ba.MaxSpanRequestKeys != 0 || ba.TargetBytes != 0) {
-		// We already verified above that the batch contains only scan requests of the same type.
-		// Such a batch should never need splitting.
-		log.Fatalf(ctx, "batch with MaxSpanRequestKeys=%d, TargetBytes=%d needs splitting",
-			redact.Safe(ba.MaxSpanRequestKeys), redact.Safe(ba.TargetBytes))
-	}
 	var singleRplChunk [1]*kvpb.BatchResponse
 	rplChunks := singleRplChunk[:0:1]
 

pkg/sql/internal.go

@@ -1185,6 +1185,12 @@ func (ie *InternalExecutor) execInternal(
 
 	applyInternalExecutorSessionExceptions(sd)
 	applyOverrides(sessionDataOverride, sd)
+	if txn != nil && txn.Type() == kv.RootTxn {
+		// For 25.2, we're being conservative and explicitly disabling buffered
+		// writes for the internal executor.
+		// TODO(yuzefovich): remove this for 25.3.
+		txn.SetBufferedWritesEnabled(false)
+	}
 	attributeToUser := sessionDataOverride.AttributeToUser && attributeToUserEnabled.Get(&ie.s.cfg.Settings.SV)
 	growStackSize := sessionDataOverride.GrowStackSize
 	if !rw.async() && (txn != nil && txn.Type() == kv.RootTxn) {