Merge cockroachdb#73786

73786: kvserver: document writeStats semantics r=nvanbenschoten a=tbg

Because they're weird. We are summing up these two:

https://github.com/cockroachdb/cockroach/blob/afb50d6227e78daa72ac8fa08222bc58a5767648/pkg/kv/kvserver/replica_application_state_machine.go#L929-L928

https://github.com/cockroachdb/cockroach/blob/afb50d6227e78daa72ac8fa08222bc58a5767648/pkg/kv/kvserver/replica_application_state_machine.go#L644-L646

Oddness:

- the first and the second often measure the same thing, so we're double
  counting: a `Put(x)` will result in a mutation but may also result in
  a key being added, so we're recording that effect twice.
- a `Put(x)` on an existing key will only be counted in `b.mutations` so
  it counts only once.
- AddSSTable is only reflected in the second one, but the `KeyCount`
  could be an estimate (or so I believe; not 100% sure).
- `b.mutations` also contains update to `RangeAppliedState`, so it's
  counting at least one mutation per batch.

Overall, the writes per second have to be taken with the grain of salt
that they will in practice over-count by a factor of anywhere between
one and three, the extreme case being a `Put` that creates a new key,
which will have a contribution of one `RangeAppliedState` write, one
`KeyCount`, and the actual mutation in the batch.

Touches cockroachdb#73731.
Touches cockroachdb#42277.

Release note: None


Co-authored-by: Tobias Grieger <[email protected]>

pkg/kv/kvserver/replica.go

@@ -220,8 +220,18 @@ type Replica struct {
 	// leaseholderStats tracks all incoming BatchRequests to the replica and which
 	// localities they come from in order to aid in lease rebalancing decisions.
 	leaseholderStats *replicaStats
-	// writeStats tracks the number of keys written by applied raft commands
-	// in order to aid in replica rebalancing decisions.
+	// writeStats tracks the number of mutations (as counted by the pebble batch
+	// to be applied to the state machine), and additionally, the number of keys
+	// added to MVCCStats, which notably may be approximate in the case of an
+	// AddSSTable. In other words, writeStats should loosely track the write
+	// activity on the replica on a per-key basis, though in an inconsistent way
+	// that in particular may overcount by a factor of roughly two.
+	//
+	// Note that while writeStats were originally introduced to aid in rebalancing
+	// decisions in [1], at the time of writing they are not used for that
+	// purpose.
+	//
+	// [1]: https://github.com/cockroachdb/cockroach/pull/16664
 	writeStats *replicaStats
 
 	// creatingReplica is set when a replica is created as uninitialized