Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
VReplication: Add throttler stats #15221
Changes from 1 commit
2b89132
7ef27a4
b74bb57
4692c42
0a5d94f
9857538
ac6f772
b8a81f6
981dc0b
451385a
14168b2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check warning on line 375 in go/vt/binlog/binlogplayer/binlog_player.go
Codecov / codecov/patch
go/vt/binlog/binlogplayer/binlog_player.go#L375
Check warning on line 515 in go/vt/vttablet/tabletmanager/vreplication/stats.go
Codecov / codecov/patch
go/vt/vttablet/tabletmanager/vreplication/stats.go#L510-L515
Check warning on line 518 in go/vt/vttablet/tabletmanager/vreplication/stats.go
Codecov / codecov/patch
go/vt/vttablet/tabletmanager/vreplication/stats.go#L518
Check warning on line 591 in go/vt/vttablet/tabletmanager/vreplication/vreplicator.go
Codecov / codecov/patch
go/vt/vttablet/tabletmanager/vreplication/vreplicator.go#L590-L591
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 😛
Check warning on line 594 in go/vt/vttablet/tabletmanager/vreplication/vreplicator.go
Codecov / codecov/patch
go/vt/vttablet/tabletmanager/vreplication/vreplicator.go#L594