cockroach icon indicating copy to clipboard operation
cockroach copied to clipboard

obs: replace runtime.ReadMemStats with runtime/metrics

Open lyang24 opened this issue 1 year ago • 4 comments

This commit replaces Memstats collection with go runtime metrics. The latter approach do not require stw to collect memory stats.

A subtle change to be noted: statsProfiler still call runtime.ReadMemStats on spot when it determines memory stats should be saved. The previous behavior was to save with the Memstats that was refreshed periodically.

Fixes: https://github.com/cockroachdb/cockroach/issues/119461

Release note: None

Testing: Single node kv workload side by side comparison left=this commit and right=master memory image gc pause time image

lyang24 avatar Feb 23 '24 07:02 lyang24

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?

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

Before a member of our team reviews your PR, I have some potential action items for you:

  • Please ensure your git commit message contains a release note.
  • When CI has completed, please ensure no errors have appeared.

I was unable to automatically find a reviewer. You can try CCing one of the following members:

  • A person you worked with closely on this PR.
  • The person who created the ticket, or a CRDB organization member involved with the ticket (author, commenter, etc.).
  • Join our community slack channel and ask on #contributors.
  • Try find someone else from here.

:owl: Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

blathers-crl[bot] avatar Feb 23 '24 07:02 blathers-crl[bot]

This change is Reviewable

cockroach-teamcity avatar Feb 23 '24 07:02 cockroach-teamcity

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?

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

  • Please ensure your git commit message contains a release note.
  • When CI has completed, please ensure no errors have appeared.

I was unable to automatically find a reviewer. You can try CCing one of the following members:

  • A person you worked with closely on this PR.
  • The person who created the ticket, or a CRDB organization member involved with the ticket (author, commenter, etc.).
  • Join our community slack channel and ask on #contributors.
  • Try find someone else from here.

:owl: Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

blathers-crl[bot] avatar Feb 23 '24 08:02 blathers-crl[bot]

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?

Thank you for updating your pull request.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

I was unable to automatically find a reviewer. You can try CCing one of the following members:

  • A person you worked with closely on this PR.
  • The person who created the ticket, or a CRDB organization member involved with the ticket (author, commenter, etc.).
  • Join our community slack channel and ask on #contributors.
  • Try find someone else from here.

:owl: Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

blathers-crl[bot] avatar Feb 23 '24 09:02 blathers-crl[bot]

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?

Thank you for updating your pull request.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

:owl: Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

blathers-crl[bot] avatar Mar 07 '24 08:03 blathers-crl[bot]

After reading the doc i am fairly confident on all the new metrics from runtime/metrics.

This is a risky change at the end of the day, Hi @nvanbenschoten do you mind to take a second look when you have time

lyang24 avatar Mar 07 '24 09:03 lyang24

bors r+

lyang24 avatar Mar 12 '24 18:03 lyang24

Build succeeded:

craig[bot] avatar Mar 12 '24 19:03 craig[bot]