kvserver: replica load distribution metric

[kvoli]

This PR adds two histograms, tracking the percentiles of replica CPU time and replica Batch requests received. Previously, there was only point in time insight into either distribution via hotranges. This change enables historical timeseries tracking via the metric rebalancing.replicas.cpunanospersecond for CPU and rebalancing.replicas.queriespersecond for Batch Requests.

Informs: #98255

[blathers-crl]

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

[cockroach-teamcity]

This change is 

[kvoli]

Going to hold off requesting reviews until #98266 is settled.

[kvoli]

@aadityasondhi mentioned that they expect #98266 to be resolved before GA. I'm going to go ahead and req reviews - noting that currently the timseries has periodic gaps.

[kvoli]

Reverted to draft. I'm going to rework the approach here using a ManualHistogram after some changes are made to ManualHistogram.

From @aadityasondhi

For histograms such as the one above, I would suggest using ManualWindowHistogram. I do recognize that the current interface is cumbersome and I will rework it to make it more easily usable. When you create this histogram, you will be required to provide the promised update interval. The poller will report the last known value of these histograms as long as T-t < update interval. To avoid race conditions such as the one in this issue, we will allow for 1 missed update but start reporting 0 if the poller misses twice in a row.

[kvoli]

I took a pretty hacky path to use the ManualHistogram, with the assumption that we will update the iface in 23.2 - lmk what you think @aadityasondhi.

[aadityasondhi]

Only reviewed the first commit (related to the refactoring of the manual histogram), but overall it looks great! Some of the ideas here are what I had in mind for the refactor in #98622.

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

[kvoli]

@andrewbaptist, could you review the last 3 commits? They are adding two metrics using the histogram modified in c1.

[andrewbaptist]

Thanks for getting this in, this will be useful to see as we continue to make improvements to the allocator.

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

---

pkg/kv/kvserver/metrics.go line 363 at r1 (raw file):

		Unit:        metric.Unit_NANOSECONDS,
	}
	metaAverageReplicaCPUNanosPerSecond = metric.Metadata{

nit: The name is confusing. In general average for a histogram is confusing since there are two different ways it could be interpreted: 1) average across replicas, 2) average across time. Maybe something like metaRecentReplicaCPUNanosPerSecond. I think part of the problem is that it is just a complex topic because of the two dimensions. Also, can you add a comment to clarify if this applies to all replicas including quiesced ones. I think it does.

---

pkg/kv/kvserver/metrics.go line 2376 at r1 (raw file):

		AverageWriteBytesPerSecond: metric.NewGaugeFloat64(metaAverageWriteBytesPerSecond),
		AverageReadBytesPerSecond:  metric.NewGaugeFloat64(metaAverageReadBytesPerSecond),
		AverageCPUNanosPerSecond:   metric.NewGaugeFloat64(metaAverageCPUNanosPerSecond),

nit: Is this stat redundant with the AverageReplicaCPUNanosPerSecond now? If so can we remove it? If it is still necessary since ManualHistograms can't be used for computing this average just add a short comment why not.

[kvoli]

TYFTR

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @andrewbaptist and @kvoli)

---

pkg/kv/kvserver/metrics.go line 363 at r1 (raw file):

Previously, andrewbaptist (Andrew Baptist) wrote…

nit: The name is confusing. In general average for a histogram is confusing since there are two different ways it could be interpreted: 1) average across replicas, 2) average across time. Maybe something like metaRecentReplicaCPUNanosPerSecond. I think part of the problem is that it is just a complex topic because of the two dimensions. Also, can you add a comment to clarify if this applies to all replicas including quiesced ones. I think it does.

Added comment to clarify it is for all replicas and renamed to recent....

---

pkg/kv/kvserver/metrics.go line 2376 at r1 (raw file):

Previously, andrewbaptist (Andrew Baptist) wrote…

nit: Is this stat redundant with the AverageReplicaCPUNanosPerSecond now? If so can we remove it? If it is still necessary since ManualHistograms can't be used for computing this average just add a short comment why not.

It is however I'd rather not break dependencies (lots of tests, probably users and definitely UI) in this PR, since I intend to backport to 23.1.

[kvoli]

bors r=andrewbaptist

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)