-
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
VReplication: Add throttler stats #15221
Conversation
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
|
feaaecb
to
d83a882
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #15221 +/- ##
==========================================
+ Coverage 67.41% 67.46% +0.04%
==========================================
Files 1560 1561 +1
Lines 192752 193219 +467
==========================================
+ Hits 129952 130352 +400
- Misses 62800 62867 +67 ☔ View full report in Codecov by Sentry. |
2dad697
to
fcd5240
Compare
Signed-off-by: Matt Lord <mattalord@gmail.com>
fcd5240
to
2b89132
Compare
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 like a good approach!
func (vr *vreplicator) updateTimeThrottled(appThrottled throttlerapp.Name) error { | ||
at := appThrottled.String() | ||
vr.stats.ThrottledCounts.Add([]string{"tablet", at}, 1) |
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.
I believe we want to always update stats in a goroutine so as not to make the metrics themselves affect the code's flow (the metric introduces an atomic write).
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 don't do that in vreplication today. I would expect this to be faster/lighter than a log message, which we don't typically do in a goroutine (which has its own overhead). Your usage of the function that creates the underlying stat resources if needed (in the tablet throttler) I think makes more sense to do in a goroutine like you are.
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.
Yeah, I'm about to get rid of that usage 😛
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
be6f198
to
9857538
Compare
b2b7429
to
aa6e67a
Compare
aa6e67a
to
8850009
Compare
Signed-off-by: Matt Lord <mattalord@gmail.com>
8850009
to
ac6f772
Compare
From: vitessio/vitess#15221 Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.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.
Unrelated fix for flaky tests seen in the code coverage workflow:
--- FAIL: TestMinimalMode (60.00s)
main_flaky_test.go:79:
Error Trace: /home/runner/work/vitess/vitess/go/vt/vttablet/tabletserver/vstreamer/main_flaky_test.go:79
/home/runner/work/vitess/vitess/go/vt/vttablet/tabletserver/vstreamer/vstreamer_test.go:1855
Error: Received unexpected error:
could not launch mysql: signal: killed
Test: TestMinimalMode
From: vitessio/vitess#15221 Signed-off-by: Matt Lord <mattalord@gmail.com>
Description
This pairs with #15223 to increase users' ability to observe how the tablet throttler and vreplication interact over time in order to understand the system and explain/debug observed behaviors/results.
Related Issue(s)
Checklist