kv: don't preemptively refresh under weak isolation levels

Informs cockroachdb#100131.

This commit disables preemptive transaction refreshes under weak
isolation levels. These transactions can tolerate write skew, so they
can commit even if their write timestamp has been bumped. Transactions
run at weak isolation levels may refresh in response to WriteTooOld
errors or ReadWithinUncertaintyInterval errors returned by requests, but
they do not need to refresh preemptively ahead of an EndTxn request.

Release note: None

pkg/kv/kvclient/kvcoord/txn_interceptor_span_refresher.go

@@ -432,6 +432,15 @@ func (sr *txnSpanRefresher) maybeRefreshPreemptivelyLocked(
 		return ba, nil
 	}
 
+	// If the transaction can tolerate write skew, no preemptive refresh is
+	// necessary, even if its write timestamp has been bumped. Transactions run at
+	// weak isolation levels may refresh in response to WriteTooOld errors or
+	// ReadWithinUncertaintyInterval errors returned by requests, but they do not
+	// need to refresh preemptively ahead of an EndTxn request.
+	if ba.Txn.IsoLevel.ToleratesWriteSkew() {
+		return ba, nil
+	}
+
 	// If true, tryRefreshTxnSpans will trivially succeed.
 	refreshFree := ba.CanForwardReadTimestamp
 

pkg/kv/kvclient/kvcoord/txn_interceptor_span_refresher_test.go

@@ -17,6 +17,7 @@ import (
 	"testing"
 
 	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
+	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency/isolation"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/testutils"
@@ -539,6 +540,78 @@ func TestTxnSpanRefresherPreemptiveRefresh(t *testing.T) {
 	require.False(t, tsr.refreshInvalid)
 }
 
+// TestTxnSpanRefresherPreemptiveRefreshIsoLevel tests that the txnSpanRefresher
+// only performed preemptive client-side refreshes of Serializable transactions.
+func TestTxnSpanRefresherPreemptiveRefreshIsoLevel(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
+	tests := []struct {
+		isoLevel   isolation.Level
+		expRefresh bool
+	}{
+		{isolation.Serializable, true},
+		{isolation.Snapshot, false},
+		{isolation.ReadCommitted, false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.isoLevel.String(), func(t *testing.T) {
+			ctx := context.Background()
+			tsr, mockSender := makeMockTxnSpanRefresher()
+
+			txn := makeTxnProto()
+			txn.IsoLevel = tt.isoLevel
+
+			// Push the txn.
+			txn.WriteTimestamp = txn.WriteTimestamp.Add(1, 0)
+			origReadTs := txn.ReadTimestamp
+			pushedWriteTs := txn.WriteTimestamp
+
+			// Send an EndTxn request.
+			ba := &kvpb.BatchRequest{}
+			ba.Header = kvpb.Header{Txn: &txn}
+			etArgs := kvpb.EndTxnRequest{Commit: true}
+			ba.Add(&etArgs)
+
+			mockSender.MockSend(func(ba *kvpb.BatchRequest) (*kvpb.BatchResponse, *kvpb.Error) {
+				require.Len(t, ba.Requests, 1)
+				require.True(t, ba.CanForwardReadTimestamp)
+				require.IsType(t, &kvpb.EndTxnRequest{}, ba.Requests[0].GetInner())
+
+				if tt.expRefresh {
+					// The transaction should be refreshed.
+					require.NotEqual(t, origReadTs, ba.Txn.ReadTimestamp)
+					require.Equal(t, pushedWriteTs, ba.Txn.ReadTimestamp)
+					require.Equal(t, pushedWriteTs, ba.Txn.WriteTimestamp)
+				} else {
+					// The transaction should not be refreshed.
+					require.Equal(t, origReadTs, ba.Txn.ReadTimestamp)
+					require.NotEqual(t, pushedWriteTs, ba.Txn.ReadTimestamp)
+					require.Equal(t, pushedWriteTs, ba.Txn.WriteTimestamp)
+				}
+
+				br := ba.CreateReply()
+				br.Txn = ba.Txn
+				return br, nil
+			})
+
+			br, pErr := tsr.SendLocked(ctx, ba)
+			require.Nil(t, pErr)
+			require.NotNil(t, br)
+
+			expRefreshSuccess := int64(0)
+			if tt.expRefresh {
+				expRefreshSuccess = 1
+			}
+			require.Equal(t, expRefreshSuccess, tsr.refreshSuccess.Count())
+			require.Equal(t, int64(0), tsr.refreshFail.Count())
+			require.Equal(t, int64(0), tsr.refreshAutoRetries.Count())
+			require.True(t, tsr.refreshFootprint.empty())
+			require.False(t, tsr.refreshInvalid)
+		})
+	}
+}
+
 // TestTxnSpanRefresherSplitEndTxnOnAutoRetry tests that EndTxn requests are
 // split into their own sub-batch on auto-retries after a successful refresh.
 // This is done to avoid starvation.