Skip to content

release-22.2: schedulerlatency: export Go scheduling latency metric#88403

Merged
irfansharif merged 2 commits intorelease-22.2from
blathers/backport-release-22.2-87883
Sep 22, 2022
Merged

release-22.2: schedulerlatency: export Go scheduling latency metric#88403
irfansharif merged 2 commits intorelease-22.2from
blathers/backport-release-22.2-87883

Conversation

@blathers-crl
Copy link
Copy Markdown

@blathers-crl blathers-crl bot commented Sep 21, 2022 •
edited by irfansharif
Loading

Backport 1/1 commits from #87883, 1/1 commits from #88458 on behalf of @irfansharif.

/cc @cockroachdb/release

And record data into CRDB's internal time-series database. Informs
#82743 and #87823. To export scheduling latencies to prometheus, we
choose an exponential bucketing scheme with base multiple of 1.1, and
the output range bounded to [50us, 100ms). This makes for ~70 buckets.
It's worth noting that the default histogram buckets used in Go are
not fit for our purposes. If we care about improving it, we could
consider patching the runtime.

  bucket[  0] width=0s boundary=[-Inf, 0s)
  bucket[  1] width=1ns boundary=[0s, 1ns)
  bucket[  2] width=1ns boundary=[1ns, 2ns)
  bucket[  3] width=1ns boundary=[2ns, 3ns)
  bucket[  4] width=1ns boundary=[3ns, 4ns)
  ...
  bucket[270] width=16.384µs boundary=[737.28µs, 753.664µs)
  bucket[271] width=16.384µs boundary=[753.664µs, 770.048µs)
  bucket[272] width=278.528µs boundary=[770.048µs, 1.048576ms)
  bucket[273] width=32.768µs boundary=[1.048576ms, 1.081344ms)
  bucket[274] width=32.768µs boundary=[1.081344ms, 1.114112ms)
  ...
  bucket[717] width=1h13m18.046511104s boundary=[53h45m14.046488576s, 54h58m32.09299968s)
  bucket[718] width=1h13m18.046511104s boundary=[54h58m32.09299968s, 56h11m50.139510784s)
  bucket[719] width=1h13m18.046511104s boundary=[56h11m50.139510784s, 57h25m8.186021888s)
  bucket[720] width=57h25m8.186021888s boundary=[57h25m8.186021888s, +Inf)

Release note: None
Release justification: observability-only PR, low-risk high-benefit; would help understand admission control out in the wild

Release justification:

@irfansharif
schedulerlatency: export Go scheduling latency metric
11a8072 
And record data into CRDB's internal time-series database. Informs
\#82743 and #87823. To export scheduling latencies to prometheus, we
choose an exponential bucketing scheme with base multiple of 1.1, and
the output range bounded to [50us, 100ms). This makes for ~70 buckets.
It's worth noting that the default histogram buckets used in Go are
not fit for our purposes. If we care about improving it, we could
consider patching the runtime.

  bucket[  0] width=0s boundary=[-Inf, 0s)
  bucket[  1] width=1ns boundary=[0s, 1ns)
  bucket[  2] width=1ns boundary=[1ns, 2ns)
  bucket[  3] width=1ns boundary=[2ns, 3ns)
  bucket[  4] width=1ns boundary=[3ns, 4ns)
  ...
  bucket[270] width=16.384µs boundary=[737.28µs, 753.664µs)
  bucket[271] width=16.384µs boundary=[753.664µs, 770.048µs)
  bucket[272] width=278.528µs boundary=[770.048µs, 1.048576ms)
  bucket[273] width=32.768µs boundary=[1.048576ms, 1.081344ms)
  bucket[274] width=32.768µs boundary=[1.081344ms, 1.114112ms)
  ...
  bucket[717] width=1h13m18.046511104s boundary=[53h45m14.046488576s, 54h58m32.09299968s)
  bucket[718] width=1h13m18.046511104s boundary=[54h58m32.09299968s, 56h11m50.139510784s)
  bucket[719] width=1h13m18.046511104s boundary=[56h11m50.139510784s, 57h25m8.186021888s)
  bucket[720] width=57h25m8.186021888s boundary=[57h25m8.186021888s, +Inf)

Release note: None
@blathers-crl blathers-crl bot requested a review from a team as a code owner September 21, 2022 21:14
@blathers-crl blathers-crl bot requested a review from a team September 21, 2022 21:14
@blathers-crl blathers-crl bot requested review from a team as code owners September 21, 2022 21:14
@blathers-crl blathers-crl bot force-pushed the blathers/backport-release-22.2-87883 branch from 0a96b57 to 11a8072 Compare September 21, 2022 21:14
@blathers-crl
Copy link
Copy Markdown
Author

blathers-crl bot commented Sep 21, 2022 •
edited by irfansharif
Loading

Thanks for opening a backport.

Please check the backport criteria before merging:

  • Patches should only be created for serious issues or test-only changes.
  • Patches should not break backwards-compatibility.
  • Patches should change as little code as possible.
  • Patches should not change on-disk formats or node communication protocols.
  • Patches should not add new functionality.
  • Patches must not add, edit, or otherwise modify cluster versions; or add version gates.
If some of the basic criteria cannot be satisfied, ensure that the exceptional criteria are satisfied within.
  • There is a high priority need for the functionality that cannot wait until the next release and is difficult to address in another way.
  • The new functionality is additive-only and only runs for clusters which have specifically “opted in” to it (e.g. by a cluster setting).
  • New code is protected by a conditional check that is trivial to verify and ensures that it only runs for opt-in clusters.
  • The PM and TL on the team that owns the changed code have signed off that the change obeys the above rules.

Add a brief release justification to the body of your PR to justify this backport.

Some other things to consider:

  • What did we do to ensure that a user that doesn’t know & care about this backport, has no idea that it happened?
  • Will this work in a cluster of mixed patch versions? Did we test that?
  • If a user upgrades a patch version, uses this feature, and then downgrades, what happens?

@blathers-crl blathers-crl bot added blathers-backport This is a backport that Blathers created automatically. O-robot Originated from a bot. labels Sep 21, 2022
@cockroach-teamcity
Copy link
Copy Markdown
Member

cockroach-teamcity commented Sep 21, 2022

This change is Reviewable

@irfansharif irfansharif mentioned this pull request Sep 22, 2022
Merged
sumeerbhola
sumeerbhola approved these changes Sep 22, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@sumeerbhola sumeerbhola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @aadityasondhi, @andrewbaptist, and @irfansharif)

@irfansharif irfansharif merged commit bc0b006 into release-22.2 Sep 22, 2022
@irfansharif irfansharif deleted the blathers/backport-release-22.2-87883 branch September 22, 2022 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sumeerbhola sumeerbhola sumeerbhola approved these changes

@aadityasondhi aadityasondhi Awaiting requested review from aadityasondhi

@andrewbaptist andrewbaptist Awaiting requested review from andrewbaptist

@irfansharif irfansharif Awaiting requested review from irfansharif

Assignees

@irfansharif irfansharif

Labels

blathers-backport This is a backport that Blathers created automatically. O-robot Originated from a bot.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cockroach-teamcity @sumeerbhola @irfansharif