From 43c3e4265b5280b59e0d0af5f3f02e3141989b06 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Tue, 1 Sep 2020 11:54:21 +0400 Subject: [PATCH] config: rename prof_laddr to pprof_laddr and move it to rpc (#5315) * config: rename prof_laddr to pprof_laddr and move it to rpc also, remove `/unsafe_start_cpu_profiler`, `/unsafe_stop_cpu_profiler` and `/unsafe_write_heap_profile` in favor of pprof server functionality. Closes #5303 * update changelog * log start --- CHANGELOG_PENDING.md | 4 +++ UPGRADING.md | 9 +++++- cmd/tendermint/commands/run_node.go | 1 + config/config.go | 7 ++--- config/toml.go | 10 +++--- docs/tendermint-core/configuration.md | 10 +++--- node/node.go | 5 +-- rpc/core/dev.go | 44 --------------------------- rpc/core/doc.go | 3 -- rpc/core/routes.go | 7 ++--- 10 files changed, 31 insertions(+), 69 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index f31be60ec..160dce365 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -8,6 +8,10 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - [crypto/secp256k1] \#5280 `secp256k1` has been removed from the Tendermint repo. (@marbar3778) +- CLI/RPC/Config + - [config] \#5315 Rename `prof_laddr` to `pprof_laddr` and move it to `rpc` section (@melekes) + - [rpc] \#5315 Remove `/unsafe_start_cpu_profiler`, `/unsafe_stop_cpu_profiler` and `/unsafe_write_heap_profile`. Please use pprof functionality instead (@melekes) + ## FEATURES - [privval] \#5239 Add `chainID` to requests from client. (@marbar3778) diff --git a/UPGRADING.md b/UPGRADING.md index d8237500d..1cb6cd6b7 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -46,7 +46,7 @@ Merkle tree built from: - `BeginBlock#Events`. Merkle hashes of empty trees previously returned nothing, but now return the hash of an empty input, -to conform with RFC-6962. This mainly affects `Header#DataHash`, `Header#LastResultsHash`, and +to conform with RFC-6962. This mainly affects `Header#DataHash`, `Header#LastResultsHash`, and `Header#EvidenceHash`, which are often empty. Non-empty hashes can also be affected, e.g. if their inputs depend on other (empty) Merkle hashes, giving different results. @@ -144,6 +144,13 @@ All requests are now accompanied by the chainID from the network. This is a optional field and can be ignored by key management systems. It is recommended to check the chainID if using the same key management system for multiple chains. +### RPC + +`/unsafe_start_cpu_profiler`, `/unsafe_stop_cpu_profiler` and +`/unsafe_write_heap_profile` were removed. Please use pprof server, which can +be enabled through `--rpc.pprof_laddr=X` flag or `pprof_laddr=X` config setting +in the rpc section. + ## v0.33.4 ### Go API diff --git a/cmd/tendermint/commands/run_node.go b/cmd/tendermint/commands/run_node.go index 41f480fc3..92ec70c3a 100644 --- a/cmd/tendermint/commands/run_node.go +++ b/cmd/tendermint/commands/run_node.go @@ -58,6 +58,7 @@ func AddNodeFlags(cmd *cobra.Command) { config.RPC.GRPCListenAddress, "GRPC listen address (BroadcastTx only). Port required") cmd.Flags().Bool("rpc.unsafe", config.RPC.Unsafe, "Enabled unsafe rpc methods") + cmd.Flags().String("rpc.pprof_laddr", config.RPC.PprofListenAddress, "pprof listen address (https://golang.org/pkg/net/http/pprof)") // p2p flags cmd.Flags().String( diff --git a/config/config.go b/config/config.go index cd658e8ca..20669a5fa 100644 --- a/config/config.go +++ b/config/config.go @@ -210,9 +210,6 @@ type BaseConfig struct { //nolint: maligned // Mechanism to connect to the ABCI application: socket | grpc ABCI string `mapstructure:"abci"` - // TCP or UNIX socket address for the profiling server to listen on - ProfListenAddress string `mapstructure:"prof_laddr"` - // If true, query the ABCI app on connecting to a new peer // so the app can decide if we should keep the connection or not FilterPeers bool `mapstructure:"filter_peers"` // false @@ -230,7 +227,6 @@ func DefaultBaseConfig() BaseConfig { ABCI: "socket", LogLevel: DefaultPackageLogLevels(), LogFormat: LogFormatPlain, - ProfListenAddress: "", FastSyncMode: true, FilterPeers: false, DBBackend: "goleveldb", @@ -383,6 +379,9 @@ type RPCConfig struct { // NOTE: both tls_cert_file and tls_key_file must be present for Tendermint to create HTTPS server. // Otherwise, HTTP server is run. TLSKeyFile string `mapstructure:"tls_key_file"` + + // pprof listen address (https://golang.org/pkg/net/http/pprof) + PprofListenAddress string `mapstructure:"pprof_laddr"` } // DefaultRPCConfig returns a default configuration for the RPC server diff --git a/config/toml.go b/config/toml.go index 936c6bb03..fa112622d 100644 --- a/config/toml.go +++ b/config/toml.go @@ -139,9 +139,6 @@ node_key_file = "{{ js .BaseConfig.NodeKey }}" # Mechanism to connect to the ABCI application: socket | grpc abci = "{{ .BaseConfig.ABCI }}" -# TCP or UNIX socket address for the profiling server to listen on -prof_laddr = "{{ .BaseConfig.ProfListenAddress }}" - # If true, query the ABCI app on connecting to a new peer # so the app can decide if we should keep the connection or not filter_peers = {{ .BaseConfig.FilterPeers }} @@ -232,6 +229,9 @@ tls_cert_file = "{{ .RPC.TLSCertFile }}" # Otherwise, HTTP server is run. tls_key_file = "{{ .RPC.TLSKeyFile }}" +# pprof listen address (https://golang.org/pkg/net/http/pprof) +pprof_laddr = "{{ .RPC.PprofListenAddress }}" + ####################################################### ### P2P Configuration Options ### ####################################################### @@ -363,7 +363,7 @@ temp_dir = "{{ .StateSync.TempDir }}" # Fast Sync version to use: # 1) "v0" (default) - the legacy fast sync implementation # 2) "v1" - refactor of v0 version for better testability -# 2) "v2" - complete redesign of v0, optimized for testability & readability +# 2) "v2" - complete redesign of v0, optimized for testability & readability version = "{{ .FastSync.Version }}" ####################################################### @@ -404,7 +404,7 @@ peer_query_maj23_sleep_duration = "{{ .Consensus.PeerQueryMaj23SleepDuration }}" [tx_index] # What indexer to use for transactions -# +# # The application will set which txs to index. In some cases a node operator will be able # to decide which txs to index based on configuration set in the application. # diff --git a/docs/tendermint-core/configuration.md b/docs/tendermint-core/configuration.md index 9ca8c8574..734aa9c9b 100644 --- a/docs/tendermint-core/configuration.md +++ b/docs/tendermint-core/configuration.md @@ -92,9 +92,6 @@ node_key_file = "config/node_key.json" # Mechanism to connect to the ABCI application: socket | grpc abci = "socket" -# TCP or UNIX socket address for the profiling server to listen on -prof_laddr = "" - # If true, query the ABCI app on connecting to a new peer # so the app can decide if we should keep the connection or not filter_peers = false @@ -185,6 +182,9 @@ tls_cert_file = "" # Otherwise, HTTP server is run. tls_key_file = "" +# pprof listen address (https://golang.org/pkg/net/http/pprof) +pprof_laddr = "" + ####################################################### ### P2P Configuration Options ### ####################################################### @@ -316,7 +316,7 @@ temp_dir = "" # Fast Sync version to use: # 1) "v0" (default) - the legacy fast sync implementation # 2) "v1" - refactor of v0 version for better testability -# 2) "v2" - complete redesign of v0, optimized for testability & readability +# 2) "v2" - complete redesign of v0, optimized for testability & readability version = "v0" ####################################################### @@ -357,7 +357,7 @@ peer_query_maj23_sleep_duration = "2s" [tx_index] # What indexer to use for transactions -# +# # The application will set which txs to index. In some cases a node operator will be able # to decide which txs to index based on configuration set in the application. # diff --git a/node/node.go b/node/node.go index 3e063fb8e..4fb285b07 100644 --- a/node/node.go +++ b/node/node.go @@ -787,9 +787,10 @@ func NewNode(config *cfg.Config, pexReactor = createPEXReactorAndAddToSwitch(addrBook, config, sw, logger) } - if config.ProfListenAddress != "" { + if config.RPC.PprofListenAddress != "" { go func() { - logger.Error("Profile server", "err", http.ListenAndServe(config.ProfListenAddress, nil)) + logger.Info("Starting pprof server", "laddr", config.RPC.PprofListenAddress) + logger.Error("pprof server error", "err", http.ListenAndServe(config.RPC.PprofListenAddress, nil)) }() } diff --git a/rpc/core/dev.go b/rpc/core/dev.go index 94ad3c86a..b70f5f1e1 100644 --- a/rpc/core/dev.go +++ b/rpc/core/dev.go @@ -1,9 +1,6 @@ package core import ( - "os" - "runtime/pprof" - ctypes "github.com/tendermint/tendermint/rpc/core/types" rpctypes "github.com/tendermint/tendermint/rpc/jsonrpc/types" ) @@ -13,44 +10,3 @@ func UnsafeFlushMempool(ctx *rpctypes.Context) (*ctypes.ResultUnsafeFlushMempool env.Mempool.Flush() return &ctypes.ResultUnsafeFlushMempool{}, nil } - -var profFile *os.File - -// UnsafeStartCPUProfiler starts a pprof profiler using the given filename. -func UnsafeStartCPUProfiler(ctx *rpctypes.Context, filename string) (*ctypes.ResultUnsafeProfile, error) { - var err error - profFile, err = os.Create(filename) - if err != nil { - return nil, err - } - err = pprof.StartCPUProfile(profFile) - if err != nil { - return nil, err - } - return &ctypes.ResultUnsafeProfile{}, nil -} - -// UnsafeStopCPUProfiler stops the running pprof profiler. -func UnsafeStopCPUProfiler(ctx *rpctypes.Context) (*ctypes.ResultUnsafeProfile, error) { - pprof.StopCPUProfile() - if err := profFile.Close(); err != nil { - return nil, err - } - return &ctypes.ResultUnsafeProfile{}, nil -} - -// UnsafeWriteHeapProfile dumps a heap profile to the given filename. -func UnsafeWriteHeapProfile(ctx *rpctypes.Context, filename string) (*ctypes.ResultUnsafeProfile, error) { - memProfFile, err := os.Create(filename) - if err != nil { - return nil, err - } - if err := pprof.WriteHeapProfile(memProfFile); err != nil { - return nil, err - } - if err := memProfFile.Close(); err != nil { - return nil, err - } - - return &ctypes.ResultUnsafeProfile{}, nil -} diff --git a/rpc/core/doc.go b/rpc/core/doc.go index b68231725..77ace4e2c 100644 --- a/rpc/core/doc.go +++ b/rpc/core/doc.go @@ -25,7 +25,6 @@ Available endpoints: /health /unconfirmed_txs /unsafe_flush_mempool -/unsafe_stop_cpu_profiler /validators Endpoints that require arguments: @@ -40,8 +39,6 @@ Endpoints that require arguments: /dial_persistent_peers?persistent_peers=_ /subscribe?event=_ /tx?hash=_&prove=_ -/unsafe_start_cpu_profiler?filename=_ -/unsafe_write_heap_profile?filename=_ /unsubscribe?event=_ ``` */ diff --git a/rpc/core/routes.go b/rpc/core/routes.go index ddfb40d51..becc6a58d 100644 --- a/rpc/core/routes.go +++ b/rpc/core/routes.go @@ -6,6 +6,7 @@ import ( // TODO: better system than "unsafe" prefix +// Routes is a map of available routes. var Routes = map[string]*rpc.RPCFunc{ // subscribe/unsubscribe are reserved for websocket events. "subscribe": rpc.NewWSRPCFunc(Subscribe, "query"), @@ -45,14 +46,10 @@ var Routes = map[string]*rpc.RPCFunc{ "broadcast_evidence": rpc.NewRPCFunc(BroadcastEvidence, "evidence"), } +// AddUnsafeRoutes adds unsafe routes. func AddUnsafeRoutes() { // control API Routes["dial_seeds"] = rpc.NewRPCFunc(UnsafeDialSeeds, "seeds") Routes["dial_peers"] = rpc.NewRPCFunc(UnsafeDialPeers, "peers,persistent") Routes["unsafe_flush_mempool"] = rpc.NewRPCFunc(UnsafeFlushMempool, "") - - // profiler API - Routes["unsafe_start_cpu_profiler"] = rpc.NewRPCFunc(UnsafeStartCPUProfiler, "filename") - Routes["unsafe_stop_cpu_profiler"] = rpc.NewRPCFunc(UnsafeStopCPUProfiler, "") - Routes["unsafe_write_heap_profile"] = rpc.NewRPCFunc(UnsafeWriteHeapProfile, "filename") }