-
Notifications
You must be signed in to change notification settings - Fork 12
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
[Bug] Fix pprof endpoint #505
Conversation
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.
A small change request w.r.t server shutdown.
Would also be great if we have this server creation pattern done with metrics.
pkg/appgateserver/server.go
Outdated
go func() { | ||
<-ctx.Done() | ||
app.logger.Info().Str("endpoint", addr).Msg("stopping a pprof endpoint") | ||
server.Close() |
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.
What about using server.Shutdown(ctx)
which does graceful shut down?
I would also have the same pattern for metrics server, instead of relying on tcp.Listener
server := &http.Server{
Addr: addr,
Handler: pprofMux,
}
...
server.ListenAndServe()
...
server.Shutdown(ctx)
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.
…cept * pokt/main: [Dashboard] Supplier logs dashboard (#502) [Utility] Update SessionSettlement when app goes into a negative amount (#509) [Testing] Supplier staking E2E test (#498) [RelayMiner] Support https backend urls (#507) [Bug] Fix pprof endpoint (#505) [LocalNet] Remove tilt-restart-wrapper (#504) [LocalNet][Hotfix] Turn off pproff by default (#500) [Tooling] Add pprof endpoints and documentation (#484) [Documentation] Basic blockchain operations (#454) [BlockClient] Refactor BlockClient to fetch latest block at init (#444)
Summary
Pprof prevents services from starting. It was not run inside a routine and caused a lock where it shouldn't have been.
Issue
Type of change
Select one or more:
Testing
Documentation changes (only if making doc changes)
make docusaurus_start
; only needed if you make doc changesLocal Testing (only if making code changes)
make go_develop_and_test
make test_e2e
PR Testing (only if making code changes)
devnet-test-e2e
label to the PR.make trigger_ci
if you want to re-trigger tests without any code changesSanity Checklist