admission: fix wait queue histograms #110060
Conversation
|
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. |
|
This change is |
There was a problem hiding this comment.
Reviewed 1 of 2 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @bananabrick and @irfansharif)
pkg/util/admission/work_queue.go line 639 at r1 (raw file):
if q.granter.tryGet(info.RequestedCount) { q.admitMu.Unlock() q.metrics.incAdmitted(info.Priority)
what about this case?
We previously did not record anything into {IO,CPU} wait queue
histograms when work either bypassed admission control (because of the
nature of the work, or when certain admission queues were disabled
through cluster settings) or used the fast-path (i.e. didn't add
themselves to the tenant heaps). This meant that our histogram
percentiles were not accurate.
NB: This problem didn't exist at the flow control level where work may
not be subject to flow control depending on the mode selected
('apply_to_elastic', 'apply_to_all'). We'd still record a measured wait
duration (~0ms), so we had accurate waiting-for-flow-tokens histograms.
Part of cockroachdb#82743.
Release note: None
irfansharif
force-pushed
the
230905.ac-histogram
branch
from
6b82df9 to
4da2627
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/util/admission/work_queue.go line 639 at r1 (raw file):
Previously, sumeerbhola wrote…
what about this case?
Done.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status:
|
bors r+ |
|
Build succeeded: |
We previously did not record anything into {IO,CPU} wait queue histograms when work either bypassed admission control (because of the nature of the work, or when certain admission queues were disabled through cluster settings). This meant that our histogram percentiles were not accurate.
This problem didn't exist at the flow control level where work may not be subject to flow control depending on the mode selected ('apply_to_elastic', 'apply_to_all'). We'd still record a measured wait duration (~0ms), so we had accurate waiting-for-flow-tokens histograms.
Part of #82743.
Release note: None