expose vtbackup stats at --port /metrics#11388
Merged
GuptaManan100 merged 3 commits intovitessio:mainfrom Oct 3, 2022
Merged
Conversation
Contributor
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
If a new flag is being introduced:
If a workflow is added or modified:
Bug fixes
Non-trivial changes
New/Existing features
Backward compatibility
|
maxenglander
changed the title
maxenglander
commented
Sep 28, 2022
| mysqlTimeout = 5 * time.Minute | ||
| initDBSQLFile string | ||
| detachedMode bool | ||
| keepAliveTimeout = 0 * time.Second |
Collaborator
Author
There was a problem hiding this comment.
New, the rest is just formatting changes
maxenglander
commented
Sep 28, 2022
maxenglander
commented
Sep 28, 2022
go/cmd/vtbackup/vtbackup.go
Outdated
| } | ||
|
|
||
| func init() { | ||
| mathrand.Seed(time.Now().UnixNano()) |
Collaborator
Author
There was a problem hiding this comment.
Not sure if this is needed. Copy-pasted from another cmd package.
Contributor
There was a problem hiding this comment.
I don't think it's needed here.
Contributor
There was a problem hiding this comment.
I don't think this is needed
maxenglander
commented
Sep 28, 2022
| exit.Return(1) | ||
| } | ||
|
|
||
| if keepAliveTimeout > 0 { |
Collaborator
Author
There was a problem hiding this comment.
Added for local testing, but think it could be useful in K8s context to keep process alive long enough for a Prometheus scrape interval.
maxenglander
commented
Sep 28, 2022
maxenglander
commented
Sep 28, 2022
| // Catch SIGTERM and SIGINT so we get a chance to clean up. | ||
| ctx, cancel := context.WithCancel(context.Background()) | ||
| sigChan := make(chan os.Signal, 1) | ||
| signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM) |
Collaborator
Author
There was a problem hiding this comment.
Signal handling now the responsibility of servenv
maxenglander
force-pushed
the
maxeng-vtbackup-prom-stats
branch
from
f7bf347 to
5f17ded
Compare
Signed-off-by: Max Englander <max@planetscale.com>
maxenglander
force-pushed
the
maxeng-vtbackup-prom-stats
branch
from
5f17ded to
bb164b4
Compare
maxenglander
commented
Sep 29, 2022
| func init() { | ||
| mathrand.Seed(time.Now().UnixNano()) | ||
| servenv.RegisterDefaultFlags() | ||
| dbconfigs.RegisterFlags(dbconfigs.All...) |
Collaborator
Author
There was a problem hiding this comment.
Moved from main
maxenglander
commented
Sep 29, 2022
|
|
||
| func main() { | ||
| defer exit.Recover() | ||
| dbconfigs.RegisterFlags(dbconfigs.All...) |
Collaborator
Author
There was a problem hiding this comment.
Moved to init
Signed-off-by: Max Englander <max@planetscale.com>
maxenglander
added
Component: Backup and Restore
Type: Enhancement
Contributor
GuptaManan100
left a comment
GuptaManan100
left a comment
There was a problem hiding this comment.
All the other changes LGTM!
Signed-off-by: Max Englander <max@planetscale.com>
This was referenced Dec 15, 2022
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Description
As far as I can tell
vtbackupdoes not currently expose metrics. It would be awesome if it did. In particular it would be great to have the following timings:This PR modifies
vtbackupcommand so that a server is started on--port. The server is similar to that launched by other VT components, and includes a/metricsroute. With this PR this route include two useful metrics which are managed by themysqlctlpackage:vtbackup_restore_duration_secondsvtbackup_backup_duration_secondsKeeping this PR small to lay the groundwork. The other metrics can come in a later PR if we're OK with this overall approach.
Use cases
PlanetScale makes ongoing internal and public efforts to improve backup and restore performance. It would be great to have detailed metrics on current performance so that we can make informed decisions on where to put our energy.