sql: add metrics for prepared statement memory usage #97590
Conversation
michae2
requested review from
yuzefovich,
rharding6373,
DrewKimball and
a team
|
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 |
|
Needs tests, but here's the general idea. |
There was a problem hiding this comment.
I'll leave it up to you in terms of tests, I'd be ok with no explicit tests for this. I only recall having a test about all metrics mentioned in some chart catalog somewhere.
Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @DrewKimball, @michae2, and @rharding6373)
pkg/sql/mem_metrics.go line 117 at r1 (raw file):
Buckets: metric.MemoryUsage64MBBuckets}), SessionCurBytesCount: metric.NewGauge(MetaMemSessionCurBytes), SessionPreparedMaxBytesHist: metric.NewHistogram(metric.HistogramOptions{
nit: it might be nice to extract a small helper to construct this object to avoid some of the code duplication.
There was a problem hiding this comment.
Reviewed 2 of 3 files at r1, all commit messages.
Reviewable status:
Add node-level metrics for memory used by prepared statements across all sessions. Assists: cockroachdb#72581 Epic: None Release note (ui change): Add the following new metrics to track memory usage of prepared statements in sessions: - sql.mem.internal.session.prepared.current - sql.mem.internal.session.prepared.max-avg - sql.mem.internal.session.prepared.max-count - sql.mem.internal.session.prepared.max-max - sql.mem.internal.session.prepared.max-p50 - sql.mem.internal.session.prepared.max-p75 - sql.mem.internal.session.prepared.max-p90 - sql.mem.internal.session.prepared.max-p99 - sql.mem.internal.session.prepared.max-p99.9 - sql.mem.internal.session.prepared.max-p99.99 - sql.mem.internal.session.prepared.max-p99.999 - sql.mem.sql.session.prepared.current - sql.mem.sql.session.prepared.max-avg - sql.mem.sql.session.prepared.max-count - sql.mem.sql.session.prepared.max-max - sql.mem.sql.session.prepared.max-p50 - sql.mem.sql.session.prepared.max-p75 - sql.mem.sql.session.prepared.max-p90 - sql.mem.sql.session.prepared.max-p99 - sql.mem.sql.session.prepared.max-p99.9 - sql.mem.sql.session.prepared.max-p99.99 - sql.mem.sql.session.prepared.max-p99.999
There was a problem hiding this comment.
I'll leave it up to you in terms of tests, I'd be ok with no explicit tests for this. I only recall having a test about all metrics mentioned in some chart catalog somewhere.
Ok, good point. Thank you!
bors r=yuzefovich,DrewKimball
Reviewable status:
pkg/sql/mem_metrics.go line 117 at r1 (raw file):
Previously, yuzefovich (Yahor Yuzefovich) wrote…
nit: it might be nice to extract a small helper to construct this object to avoid some of the code duplication.
Done.
|
Build succeeded: |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from aa31d92 to blathers/backport-release-22.1-97590: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 22.1.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
Add node-level metrics for memory used by prepared statements across all sessions.
Assists: #72581
Epic: None
Release note (ui change): Add the following new metrics to track memory usage of prepared statements in sessions: