storage: remove error from Replica.applyTimestampCache()

Stumbled upon a function with an error in its return signature
that never returns an error. Better to remove it and the stale
comment that goes with it. The removal of the code paths which
could have returned an error occurred in cockroachdb#33396.

Release justification: Low risk, does not change logic. Could also
hold off.

Release note: None

pkg/storage/replica_test.go

@@ -11831,9 +11831,7 @@ func TestReplicaTelemetryCounterForPushesDueToClosedTimestamp(t *testing.T) {
 				ba.Add(putReq(keyA))
 				minReadTS := r.store.Clock().Now()
 				ba.Timestamp = minReadTS.Next()
-				bumped, pErr := r.applyTimestampCache(ctx, &ba, minReadTS)
-				require.Nil(t, pErr)
-				require.False(t, bumped)
+				require.False(t, r.applyTimestampCache(ctx, &ba, minReadTS))
 				require.Equal(t, int32(0), telemetry.Read(batchesPushedDueToClosedTimestamp))
 			},
 		},
@@ -11844,9 +11842,7 @@ func TestReplicaTelemetryCounterForPushesDueToClosedTimestamp(t *testing.T) {
 				ba.Add(putReq(keyA))
 				ba.Timestamp = r.store.Clock().Now()
 				minReadTS := ba.Timestamp.Next()
-				bumped, pErr := r.applyTimestampCache(ctx, &ba, minReadTS)
-				require.Nil(t, pErr)
-				require.True(t, bumped)
+				require.True(t, r.applyTimestampCache(ctx, &ba, minReadTS))
 				require.Equal(t, int32(1), telemetry.Read(batchesPushedDueToClosedTimestamp))
 			},
 		},
@@ -11858,9 +11854,7 @@ func TestReplicaTelemetryCounterForPushesDueToClosedTimestamp(t *testing.T) {
 				ba.Timestamp = r.store.Clock().Now()
 				minReadTS := ba.Timestamp.Next()
 				r.store.tsCache.Add(keyA, keyA, minReadTS.Next(), uuid.MakeV4(), true)
-				bumped, pErr := r.applyTimestampCache(ctx, &ba, minReadTS)
-				require.Nil(t, pErr)
-				require.True(t, bumped)
+				require.True(t, r.applyTimestampCache(ctx, &ba, minReadTS))
 				require.Equal(t, int32(0), telemetry.Read(batchesPushedDueToClosedTimestamp))
 			},
 		},
@@ -11872,9 +11866,7 @@ func TestReplicaTelemetryCounterForPushesDueToClosedTimestamp(t *testing.T) {
 				ba.Timestamp = r.store.Clock().Now()
 				minReadTS := ba.Timestamp.Next()
 				r.store.tsCache.Add(keyA, keyA, minReadTS.Next(), uuid.MakeV4(), false)
-				bumped, pErr := r.applyTimestampCache(ctx, &ba, minReadTS)
-				require.Nil(t, pErr)
-				require.True(t, bumped)
+				require.True(t, r.applyTimestampCache(ctx, &ba, minReadTS))
 				require.Equal(t, int32(0), telemetry.Read(batchesPushedDueToClosedTimestamp))
 			},
 		},
@@ -11889,9 +11881,7 @@ func TestReplicaTelemetryCounterForPushesDueToClosedTimestamp(t *testing.T) {
 				minReadTS := ba.Timestamp.Next()
 				t.Log(ba.Timestamp, minReadTS, minReadTS.Next())
 				r.store.tsCache.Add(keyAA, keyAA, minReadTS.Next(), uuid.MakeV4(), true)
-				bumped, pErr := r.applyTimestampCache(ctx, &ba, minReadTS)
-				require.Nil(t, pErr)
-				require.True(t, bumped)
+				require.True(t, r.applyTimestampCache(ctx, &ba, minReadTS))
 				require.Equal(t, int32(0), telemetry.Read(batchesPushedDueToClosedTimestamp))
 			},
 		},
@@ -11906,9 +11896,7 @@ func TestReplicaTelemetryCounterForPushesDueToClosedTimestamp(t *testing.T) {
 				minReadTS := ba.Timestamp.Next()
 				t.Log(ba.Timestamp, minReadTS, minReadTS.Next())
 				r.store.tsCache.Add(keyAA, keyAA, minReadTS.Next(), uuid.MakeV4(), false)
-				bumped, pErr := r.applyTimestampCache(ctx, &ba, minReadTS)
-				require.Nil(t, pErr)
-				require.True(t, bumped)
+				require.True(t, r.applyTimestampCache(ctx, &ba, minReadTS))
 				require.Equal(t, int32(0), telemetry.Read(batchesPushedDueToClosedTimestamp))
 			},
 		},

pkg/storage/replica_tscache.go

@@ -249,13 +249,9 @@ func init() {
 // the read timestamp cache. That is, if the read timestamp cache returns a
 // value below minReadTS, minReadTS (without an associated txn id) will be used
 // instead to adjust the batch's timestamp.
-//
-// The timestamp cache also has a role in preventing replays of BeginTransaction
-// reordered after an EndTransaction. If that's detected, an error will be
-// returned.
 func (r *Replica) applyTimestampCache(
 	ctx context.Context, ba *roachpb.BatchRequest, minReadTS hlc.Timestamp,
-) (bool, *roachpb.Error) {
+) bool {
 	// bumpedDueToMinReadTS is set to true if the highest timestamp bump encountered
 	// below is due to the minReadTS.
 	var bumpedDueToMinReadTS bool
@@ -317,7 +313,7 @@ func (r *Replica) applyTimestampCache(
 	if bumpedDueToMinReadTS {
 		telemetry.Inc(batchesPushedDueToClosedTimestamp)
 	}
-	return bumped, nil
+	return bumped
 }
 
 // CanCreateTxnRecord determines whether a transaction record can be created for

pkg/storage/replica_write.go

@@ -123,9 +123,7 @@ func (r *Replica) executeWriteBatch(
 	// commands which require this command to move its timestamp
 	// forward. Or, in the case of a transactional write, the txn
 	// timestamp and possible write-too-old bool.
-	if bumped, pErr := r.applyTimestampCache(ctx, ba, minTS); pErr != nil {
-		return nil, pErr
-	} else if bumped {
+	if bumped := r.applyTimestampCache(ctx, ba, minTS); bumped {
 		// If we bump the transaction's timestamp, we must absolutely
 		// tell the client in a response transaction (for otherwise it
 		// doesn't know about the incremented timestamp). Response