kvserver: document writeStats semantics

[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

cockroach/pkg/kv/kvserver/replica_application_state_machine.go

Lines 644 to 646 in afb50d6

	if added := res.Delta.KeyCount; added > 0 {
	b.r.writeStats.recordCount(float64(added), 0)
	}

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 #73731.
Touches #42277.

Release note: None

[cockroach-teamcity]

This change is 

[dt]

(removing myself from reviewers since I'd just be 🔬 🐶 )

[nvanbenschoten]

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @erikgrinaker and @tbg)

---

pkg/kv/kvserver/replica.go, line 223 at r1 (raw file):

in order to aid in replica rebalancing decisions

Recent discussions have revealed that this is not currently true. These stats are not used by the allocator to make rebalancing decisions today, even if that was the intended behavior when the stats were introduced in #16664. This feels like a good time to mention that in this comment.

[tbg]

TFTR! Made the comment update:

	// 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

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @erikgrinaker)

[tbg]

bors r=nvanbenschoten

[craig]

Build succeeded:

• GitHub CI (Cockroach)