-
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
Add innodb_read_rows as vttablet metric #7520
Conversation
f33a58d
to
b44187e
Compare
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
@@ -99,6 +100,7 @@ func NewEngine(env tabletenv.Env) *Engine { | |||
_ = env.Exporter().NewGaugeDurationFunc("SchemaReloadTime", "vttablet keeps table schemas in its own memory and periodically refreshes it from MySQL. This config controls the reload time.", se.ticks.Interval) | |||
se.tableFileSizeGauge = env.Exporter().NewGaugesWithSingleLabel("TableFileSize", "tracks table file size", "Table") | |||
se.tableAllocatedSizeGauge = env.Exporter().NewGaugesWithSingleLabel("TableAllocatedSize", "tracks table allocated size", "Table") | |||
se.innoDbReadRowsGauge = env.Exporter().NewGaugesWithSingleLabel("InnodbRowsRead", "number of rows read by mysql", "Database") |
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.
Why do we need a label? Why can't we simply use NewGauge
?
if len(readRowsData.Rows) == 1 && len(readRowsData.Rows[0]) == 2 { | ||
value, err := evalengine.ToInt64(readRowsData.Rows[0][1]) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
se.innoDbReadRowsGauge.Set("read_rows", value) | ||
} |
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.
we should log a message or return an error if the condition is not met.
Signed-off-by: Andres Taylor <andres@planetscale.com>
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.
LGTM. Mental note that we'll probably add more status variables in the future, at which time we'll need to consolidate the show status
command. We wouldn't want to run a separate show status like'var'
for any single variable we monitor.
We want to be able to track how much work the underlying storage has had to do, not just see how much output there's been.
Related Issue: #7372