cockroachdb / cockroach

CockroachDB — the cloud native, distributed SQL database designed for high availability, effortless scale, and control over data placement.
https://www.cockroachlabs.com
Other
30.13k stars 3.81k forks source link

pkg/util/schedulerlatency: runtimeHistogram implements WindowedHistogram interface when it's not windowed #116690

Open abarganier opened 11 months ago

abarganier commented 11 months ago

Is your feature request related to a problem? Please describe. The runtimeHistogram explicitly implements the WindowedHistogram interface, but if you look at its implementation, it uses a cumulative histogram when calculating things like quantiles, average, etc:

https://github.com/cockroachdb/cockroach/blob/master/pkg/util/schedulerlatency/histogram.go#L158-L161

This is incorrect and misleading behavior. These values are not windowed, and this will cause the metric to experience problems in DB Console when graphing the metric's quantiles/averages, similar to what we're seeing in the following issues:

Describe the solution you'd like runtimeHistogram should support windowing, like every other histogram does, or it should not exist.

An even better option would be to find a way to get rid of it altogether in favor of consolidating the histogram implementations, as outlined in https://github.com/cockroachdb/cockroach/issues/116584

Jira issue: CRDB-34722

abarganier commented 11 months ago

Another weird thing about this histogram - why is count not monotonically increasing?

Screenshot 2023-12-18 at 1 59 00 PM

This seems like a basic invariant that all histograms should abide by. Maybe I'm misunderstanding the use case here, but it's definitely confusing.