storage: recompute and adjust stats via consistency checker

A number of bugs in our MVCCStats logic has been fixed recently (see for example
\cockroachdb#20996) and others are still present (cockroachdb#20554). For both, and also potentially for
future bugs or deliberate adjustments of the computations, we need a mechanism
to recompute the stats in order to purge incorrect numbers from the system over
time. Such a mechanism is introduced here.

It consists of two main components:

- A new RPC `RecomputeStats`, which applies to a single Range and
computes the difference between the persisted stats and the recomputed
ones; it can "inject" a suitable delta into the stats (thus fixing the
divergence) or do a "dry run".
- A trigger in the consistency checker that runs on the coordinating
node (the lease holder). The consistency checker already recomputes the
stats, and it can compare them against the persisted stats and judge
whether there is a divergence. If there is one, naively one may hope to
just insert the newly computed diff into the range, but this is not
ideal due to concerns about double application and racing consistency
checks. Instead, use `RecomputeStats` (which, of course, was introduced
for this purpose) which strikes a balance between efficiency and
correctness.

Updates cockroachdb#20554.

Release note (general change): added a mechanism to recompute range
stats on demand.

pkg/cli/debug.go

@@ -661,7 +661,7 @@ func runDebugCheckStoreDescriptors(ctx context.Context, db *engine.RocksDB) erro
 			if err != nil {
 				return false, err
 			}
-			ms, err := storage.ComputeStatsForRange(&desc, db, claimedMS.LastUpdateNanos)
+			ms, err := rditer.ComputeStatsForRange(&desc, db, claimedMS.LastUpdateNanos)
 			if err != nil {
 				return false, err
 			}

pkg/roachpb/api.go

@@ -520,6 +520,9 @@ func (*AdminScatterRequest) Method() Method { return AdminScatter }
 // Method implements the Request interface.
 func (*AddSSTableRequest) Method() Method { return AddSSTable }
 
+// Method implements the Request interface.
+func (*RecomputeStatsRequest) Method() Method { return RecomputeStats }
+
 // ShallowCopy implements the Request interface.
 func (gr *GetRequest) ShallowCopy() Request {
 	shallowCopy := *gr
@@ -742,6 +745,12 @@ func (r *AddSSTableRequest) ShallowCopy() Request {
 	return &shallowCopy
 }
 
+// ShallowCopy implements the Request interface.
+func (r *RecomputeStatsRequest) ShallowCopy() Request {
+	shallowCopy := *r
+	return &shallowCopy
+}
+
 // NewGet returns a Request initialized to get the value at key.
 func NewGet(key Key) Request {
 	return &GetRequest{
@@ -964,6 +973,7 @@ func (*TransferLeaseRequest) flags() int {
 	// lease holder.
 	return isWrite | isAlone | skipLeaseCheck
 }
+func (*RecomputeStatsRequest) flags() int           { return isWrite | isAlone }
 func (*ComputeChecksumRequest) flags() int          { return isWrite | isRange }
 func (*DeprecatedVerifyChecksumRequest) flags() int { return isWrite }
 func (*CheckConsistencyRequest) flags() int         { return isAdmin | isRange }

pkg/roachpb/api.proto

@@ -379,6 +379,35 @@ message CheckConsistencyResponse {
   ResponseHeader header = 1 [(gogoproto.nullable) = false, (gogoproto.embed) = true];
 }
 
+// An RecomputeStatsRequest triggers a stats recomputation on the Range addressed by
+// the request.
+//
+// An error will be returned if the start key does not match the start key of the
+// target Range.
+//
+// The stats recomputation touches essentially the whole range, but the command
+// avoids having to block other commands by taking care to not interleave
+// with splits, and by using the commutativity of stats updates. As a result,
+// it is safe to invoke at any time, including repeatedly, though it should be
+// used conservatively due to performing a full scan of the Range.
+message RecomputeStatsRequest {
+  option (gogoproto.equal) = true;
+
+  Span header = 1 [(gogoproto.nullable) = false, (gogoproto.embed) = true];
+  // When dry_run is true, the stats delta is computed, but no stats adjustment
+  // is performed. This isn't useful outside of testing since RecomputeStats is
+  // safe and idempotent.
+  bool dry_run = 2;
+}
+
+// An RecomputeStatsResponse is the response to an RecomputeStatsRequest.
+message RecomputeStatsResponse {
+  ResponseHeader header = 1 [(gogoproto.nullable) = false, (gogoproto.embed) = true];
+
+  // added_delta is the adjustment made to the range's stats, i.e. `new_stats = old_stats + added_delta`.
+  storage.engine.enginepb.MVCCNetworkStats added_delta = 2 [(gogoproto.nullable) = false];
+}
+
 // A BeginTransactionRequest is the argument to the BeginTransaction() method.
 message BeginTransactionRequest {
   option (gogoproto.equal) = true;
@@ -1150,6 +1179,7 @@ message RequestUnion {
   QueryTxnRequest query_txn = 33;
   AdminScatterRequest admin_scatter = 36;
   AddSSTableRequest add_sstable = 37;
+  RecomputeStatsRequest recompute_stats = 39;
 }
 
 // A ResponseUnion contains exactly one of the responses.
@@ -1200,6 +1230,7 @@ message ResponseUnion {
   QueryTxnResponse query_txn = 33;
   AdminScatterResponse admin_scatter = 36;
   AddSSTableResponse add_sstable = 37;
+  RecomputeStatsResponse recompute_stats = 39;
 }
 
 // A Header is attached to a BatchRequest, encapsulating routing and auxiliary

pkg/roachpb/method.go

@@ -138,4 +138,6 @@ const (
 	AdminScatter
 	// AddSSTable links a file into the RocksDB log-structured merge-tree.
 	AddSSTable
+	// RecomputeStats applies a delta to a Range's MVCCStats to fix computational errors.
+	RecomputeStats
 )

pkg/sql/tablewriter.go

@@ -819,6 +819,7 @@ func (td *tableDeleter) deleteAllRowsFast(
 	// ClearRange cannot be run in a transaction, so create a
 	// non-transactional batch to send the request.
 	b := &client.Batch{}
+	// TODO(tschottdorf): this might need a cluster migration.
 	b.AddRawRequest(&roachpb.ClearRangeRequest{
 		Span: roachpb.Span{
 			Key:    resume.Key,

pkg/storage/api.proto

@@ -18,6 +18,7 @@ option go_package = "storage";
 
 import "roachpb/internal_raft.proto";
 import "roachpb/metadata.proto";
+import "storage/engine/enginepb/mvcc3.proto";
 import "gogoproto/gogo.proto";
 
 // StoreRequestHeader locates a Store on a Node.
@@ -49,6 +50,8 @@ message CollectChecksumResponse {
   // TODO(tschottdorf): with larger ranges, this is no longer tenable.
   // See https://github.com/cockroachdb/cockroach/issues/21128.
   roachpb.RaftSnapshotData snapshot = 2;
+  // delta carries the stats of the range minus the recomputed stats.
+  storage.engine.enginepb.MVCCNetworkStats delta = 3 [(gogoproto.nullable) = false];
 }
 
 service Consistency {