-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Throttler: SelfMetric
interface, simplify adding new throttler metrics
#16469
Throttler: SelfMetric
interface, simplify adding new throttler metrics
#16469
Conversation
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
…ConfigurationSettings. Discarding Throttler.metricsQuery Signed-off-by: Shlomi Noach <[email protected]>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
File was just renamed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file was mostly just renamed, but also some lines of code moved to other files. There isn't anything important to review here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
File just renamed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New file, with some code extracted from elsewhere, but otherwise introducing nothing new. Nothing to review here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New file, with code moved from other files, but introducing nothing new. Nothing to review here.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16469 +/- ##
==========================================
+ Coverage 68.62% 68.63% +0.01%
==========================================
Files 1551 1558 +7
Lines 199515 199550 +35
==========================================
+ Hits 136915 136963 +48
+ Misses 62600 62587 -13 ☔ View full report in Codecov by Sentry. |
…ics can query it directly Signed-off-by: Shlomi Noach <[email protected]>
SelfMetric
interface, simplify adding new throttler metricsSelfMetric
interface, simplify adding new throttler metrics
Ready for review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parent for all metric types. Implement this to add a new metric.
…/metrics Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! I only had a few minor suggestions/nits that you can resolve as you feel best.
go/vt/vttablet/tabletserver/throttle/base/self_metric_loadavg.go
Outdated
Show resolved
Hide resolved
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Description
An internal cleanup/refactor. Intorducing no change in behavior.
This PR refactors how the throttler reads and treats the multi-metrics (
lag
,custom
,threads_running
,loadavg
) by introducing theSelfMetric
interface with implementing structsThis makes it easy to add new metrics: create a new
self_metric_<name>.go
, implementSelfMetric
, register the metric, and you're good. The throttler will invoke theRead
function for all registered metrics.The PR cleans up even more code from the throttler, eliminating
MySQLMetricConfigurationSettings.
, removing throttler'smetricsQuery
, and more.Related Issue(s)
Checklist
Deployment Notes