From bbdd88cad62ac6422a34bda0789ce19c8239cf62 Mon Sep 17 00:00:00 2001
From: Pavel Kalinnikov <pavel@cockroachlabs.com>
Date: Mon, 17 Oct 2022 18:16:07 +0100
Subject: [PATCH] kvserver: make replica sha512 a function

This function does not use the replica, so does not have to be a method.

Epic: None
Release note: None
---
 pkg/kv/kvserver/helpers_test.go             | 3 +--
 pkg/kv/kvserver/replica_consistency.go      | 9 +++++----
 pkg/kv/kvserver/replica_consistency_test.go | 7 +++----
 pkg/kv/kvserver/replica_test.go             | 2 +-
 4 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/pkg/kv/kvserver/helpers_test.go b/pkg/kv/kvserver/helpers_test.go
index 9984c51c7871..8c861611b9b9 100644
--- a/pkg/kv/kvserver/helpers_test.go
+++ b/pkg/kv/kvserver/helpers_test.go
@@ -555,9 +555,8 @@ func WatchForDisappearingReplicas(t testing.TB, store *Store) {
 func ChecksumRange(
 	ctx context.Context, desc roachpb.RangeDescriptor, snap storage.Reader,
 ) ([]byte, error) {
-	var r *Replica // TODO(pavelkalinnikov): make this less ugly.
 	lim := quotapool.NewRateLimiter("test", 1<<30, 1<<30)
-	res, err := r.sha512(ctx, desc, snap, roachpb.ChecksumMode_CHECK_FULL, lim)
+	res, err := replicaSHA512(ctx, desc, snap, roachpb.ChecksumMode_CHECK_FULL, lim)
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/kv/kvserver/replica_consistency.go b/pkg/kv/kvserver/replica_consistency.go
index 9a527e8e30ee..f450fe4538f9 100644
--- a/pkg/kv/kvserver/replica_consistency.go
+++ b/pkg/kv/kvserver/replica_consistency.go
@@ -480,9 +480,10 @@ type replicaHash struct {
 	PersistedMS, RecomputedMS enginepb.MVCCStats
 }
 
-// sha512 computes the SHA512 hash of all the replica data at the snapshot.
-// It will dump all the kv data into snapshot if it is provided.
-func (*Replica) sha512(
+// replicaSHA512 computes the SHA512 hash of the replica data at the given
+// snapshot. Either the full replicated state is taken into account, or only
+// RangeAppliedState (which includes MVCC stats), depending on the mode.
+func replicaSHA512(
 	ctx context.Context,
 	desc roachpb.RangeDescriptor,
 	snap storage.Reader,
@@ -690,7 +691,7 @@ func (r *Replica) computeChecksumPostApply(
 		); err != nil {
 			log.Errorf(ctx, "checksum collection did not join: %v", err)
 		} else {
-			result, err := r.sha512(ctx, desc, snap, cc.Mode, r.store.consistencyLimiter)
+			result, err := replicaSHA512(ctx, desc, snap, cc.Mode, r.store.consistencyLimiter)
 			if err != nil {
 				log.Errorf(ctx, "checksum computation failed: %v", err)
 				result = nil
diff --git a/pkg/kv/kvserver/replica_consistency_test.go b/pkg/kv/kvserver/replica_consistency_test.go
index dbee6d69a60e..eea1e318aa94 100644
--- a/pkg/kv/kvserver/replica_consistency_test.go
+++ b/pkg/kv/kvserver/replica_consistency_test.go
@@ -168,7 +168,6 @@ func TestReplicaChecksumSHA512(t *testing.T) {
 	eng := storage.NewDefaultInMemForTesting()
 	defer eng.Close()
 
-	repl := &Replica{} // We don't actually need the replica at all, just the method.
 	desc := roachpb.RangeDescriptor{
 		RangeID:  1,
 		StartKey: roachpb.RKey("a"),
@@ -176,7 +175,7 @@ func TestReplicaChecksumSHA512(t *testing.T) {
 	}
 
 	// Hash the empty state.
-	rh, err := repl.sha512(ctx, desc, eng, roachpb.ChecksumMode_CHECK_FULL, lim)
+	rh, err := replicaSHA512(ctx, desc, eng, roachpb.ChecksumMode_CHECK_FULL, lim)
 	require.NoError(t, err)
 	fmt.Fprintf(sb, "checksum0: %x\n", rh.SHA512[:])
 
@@ -214,13 +213,13 @@ func TestReplicaChecksumSHA512(t *testing.T) {
 			require.NoError(t, storage.MVCCPut(ctx, eng, nil, key, ts, localTS, value, nil))
 		}
 
-		rh, err = repl.sha512(ctx, desc, eng, roachpb.ChecksumMode_CHECK_FULL, lim)
+		rh, err = replicaSHA512(ctx, desc, eng, roachpb.ChecksumMode_CHECK_FULL, lim)
 		require.NoError(t, err)
 		fmt.Fprintf(sb, "checksum%d: %x\n", i+1, rh.SHA512[:])
 	}
 
 	// Run another check to obtain stats for the final state.
-	rh, err = repl.sha512(ctx, desc, eng, roachpb.ChecksumMode_CHECK_FULL, lim)
+	rh, err = replicaSHA512(ctx, desc, eng, roachpb.ChecksumMode_CHECK_FULL, lim)
 	require.NoError(t, err)
 
 	jsonpb := protoutil.JSONPb{Indent: "  "}
diff --git a/pkg/kv/kvserver/replica_test.go b/pkg/kv/kvserver/replica_test.go
index 6be47e1ba076..c020ab0cc65f 100644
--- a/pkg/kv/kvserver/replica_test.go
+++ b/pkg/kv/kvserver/replica_test.go
@@ -10431,7 +10431,7 @@ func TestReplicaServersideRefreshes(t *testing.T) {
 		// Regression test for #31870.
 		snap := tc.engine.NewSnapshot()
 		defer snap.Close()
-		res, err := tc.repl.sha512(ctx, *tc.repl.Desc(), tc.engine,
+		res, err := replicaSHA512(ctx, *tc.repl.Desc(), tc.engine,
 			roachpb.ChecksumMode_CHECK_FULL,
 			quotapool.NewRateLimiter("ConsistencyQueue", quotapool.Limit(math.MaxFloat64), math.MaxInt64))
 		if err != nil {