storage: fix stats inconsistency due to WriteTooOld optimization

This fixes a bug introduced in cockroachdb#22315 with the introduction of a "local
retry" to avoid WriteTooOldError inside of 1PC transactions that were
not carrying out any reads. Such a transaction can simply be
re-evaluated at a higher timestamp, which can allow it to commit.

The bug was that when such a re-evaluation was carried out, we were
not discarding the MVCCStats accrued from the first attempt. In
effect, the request would thus be double-counted in stats, which
would set off the consistency checker.

The fix is easy: discard the delta before re-evaluating.

Fixes cockroachdb#31870.

Release note (bug fix): remove a source of (benign) stats
inconsistencies (i.e. the stats of a range not accurately reflecting its
contents).

pkg/storage/replica_test.go

@@ -9380,6 +9380,19 @@ func TestReplicaLocalRetries(t *testing.T) {
 		if pErr != nil {
 			return hlc.Timestamp{}, pErr.GetDetail()
 		}
+
+		// Check that we didn't mess up the stats.
+		// Regression test for #31870.
+		snap := tc.engine.NewSnapshot()
+		defer snap.Close()
+		res, err := tc.repl.sha512(context.Background(), *tc.repl.Desc(), tc.engine, nil /* diff */, roachpb.ChecksumMode_CHECK_FULL)
+		if err != nil {
+			return hlc.Timestamp{}, err
+		}
+		if res.PersistedMS != res.RecomputedMS {
+			return hlc.Timestamp{}, errors.Errorf("stats are inconsistent:\npersisted:\n%+v\nrecomputed:\n%+v", res.PersistedMS, res.RecomputedMS)
+		}
+
 		return br.Timestamp, nil
 	}
 	get := func(key string) (hlc.Timestamp, error) {

pkg/storage/replica_write.go

@@ -362,8 +362,10 @@ func (r *Replica) evaluateWriteBatchWithLocalRetries(
 	spans *spanset.SpanSet,
 	canRetry bool,
 ) (batch engine.Batch, br *roachpb.BatchResponse, res result.Result, pErr *roachpb.Error) {
+	goldenMS := *ms
 	for retries := 0; ; retries++ {
 		if batch != nil {
+			*ms = goldenMS
 			batch.Close()
 		}
 		batch = r.store.Engine().NewBatch()