-
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
[servenv] optional pprof endpoints #14796
[servenv] optional pprof endpoints #14796
Conversation
Signed-off-by: Andrew Mason <[email protected]>
Signed-off-by: Andrew Mason <[email protected]>
Signed-off-by: Andrew Mason <[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
|
Signed-off-by: Andrew Mason <[email protected]>
go/vt/servenv/http.go
Outdated
} | ||
|
||
if !pflag.Lookup("pprof-http").Changed { | ||
log.Warning("Beginning in vitess version 20, pprof-http will default to `false`; to continue enabling pprof endpoints, please manually set this flag before upgrading.") |
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.
Do we have a standard casing for vitess
? Or Vitess
? And should we use Vitess 20
, Vitess v20
as a shorter version instead of writing version
out?
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 .... am not sure!
Looks like we definitely prefer v20
to version 20
:
go/vt/vtctl/reparentutil/emergency_reparenter.go: // TODO(timvaillancourt): remove legacyERS* gauges in v19+.
go/vt/vttablet/onlineddl/executor.go: // v18 fix. Remove in v19
go/vt/vttablet/onlineddl/executor.go: // This fix is created in v18 and can be removed in v19
go/vt/vttablet/tabletmanager/vreplication/flags.go: // Deprecated and ignored in v19.
go/vt/vttablet/tabletmanager/vreplication/flags.go: fs.MarkDeprecated("vreplication_tablet_type", "As of v19 this is ignored and will be removed in a future release.")
go/vt/vttablet/tabletserver/health_streamer.go: // tables and views in v19.
and it seems like we mostly prefer capitalized Vitess but i'm not completely sure. let me make those changes!
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.
actually .. looking at the flag deprecation above, let's just drop "vitess" from the language entirely and avoid the issue :P
Signed-off-by: Andrew Mason <[email protected]>
return | ||
} | ||
|
||
if !pflag.Lookup("pprof-http").Changed { |
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'm assuming this is the case, this is true
if the value is explicitly provided (but still the same as the default)?
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.
Signed-off-by: Eduardo J. Ortega U. <[email protected]>
Description
this PR adds a flag to allow users to deploy vitess components without including the
/debug/pprof
endpoints. we will begin by defaulting to opted-in to avoid changing behavior when upgrading to v19, but in v20 we will switch to disabled by default.Demos!
Related Issue(s)
Checklist
Deployment Notes