From 03ee71eac4bf86bac6a8a89254f519a63f79ea78 Mon Sep 17 00:00:00 2001 From: William Banfield Date: Mon, 2 Aug 2021 12:50:13 -0400 Subject: [PATCH] PR feedback fixes --- internal/consensus/replay_test.go | 3 --- node/debug.go | 20 ++++++++++++-------- node/node.go | 4 ++-- node/rpc.go | 28 ++++++++++------------------ rpc/core/blocks_test.go | 1 - store/store.go | 4 ---- 6 files changed, 24 insertions(+), 36 deletions(-) diff --git a/internal/consensus/replay_test.go b/internal/consensus/replay_test.go index 81ff39ff2..4d1c9c6b2 100644 --- a/internal/consensus/replay_test.go +++ b/internal/consensus/replay_test.go @@ -1217,9 +1217,6 @@ func (bs *mockBlockStore) PruneBlocks(height int64) (uint64, error) { bs.base = height return pruned, nil } -func (bs *mockBlockStore) Close() error { - return nil -} //--------------------------------------- // Test handshake/init chain diff --git a/node/debug.go b/node/debug.go index d73dbc61c..10cc01d28 100644 --- a/node/debug.go +++ b/node/debug.go @@ -13,11 +13,12 @@ import ( "github.com/tendermint/tendermint/types" ) -// Debug is a type useful for debugging tendermint problems. -// Tendermint nodes will shutdown if a divergent hash is detected. Once in this -// state, they will not start up again. Debug runs just an RPC server on the -// tendermint data stores without running any other components. This way a user -// can query the RPC server to diagnose the issue that caused a crash to begin with. +// Debug manages an RPC service that exports methods to debug a failed node. +// After a node shuts down due to a consensus failure,, it will no longer start +// up and cannot easily be inspected. A Debug value provides a similar interface +// to the node, using the underlying Tendermint data stores, without bringing up +// any other components. A caller can query the Debug service to inspect the +// persisted state and debug the failure. type Debug struct { service.BaseService @@ -35,6 +36,9 @@ func NewDebugFromConfig(cfg *config.Config) (*Debug, error) { } blockStore := store.NewBlockStore(blockStoreDB) stateDB, err := config.DefaultDBProvider(&config.DBContext{ID: _stateStoreID, Config: cfg}) + if err != nil { + return nil, err + } stateStore := sm.NewStore(stateDB) return NewDebug(cfg.RPC, blockStore, stateStore), nil @@ -63,7 +67,7 @@ func (debug *Debug) OnStart() error { } routes := rpcCoreEnv.InfoRoutes() l := log.MustNewDefaultLogger(log.LogFormatPlain, log.LogLevelInfo, false) - listeners, err := startHTTPRPCServer(debug.rpcConfig, l, routes, types.NopEventBus{}) + listeners, err := startRPCServers(debug.rpcConfig, l, routes, types.NopEventBus{}) if err != nil { return err } @@ -72,7 +76,7 @@ func (debug *Debug) OnStart() error { } func (debug *Debug) OnStop() { - for _, listener := range debug.listeners { - listener.Close() + for i := len(debug.listeners) - 1; i >= 0; i-- { + debug.listeners[i].Close() } } diff --git a/node/node.go b/node/node.go index e50d2166b..915f1bbfb 100644 --- a/node/node.go +++ b/node/node.go @@ -579,7 +579,7 @@ func (n *nodeImpl) OnStart() error { routes = rpccore.CombineRoutes(routes, env.UnsafeRoutes()) } - listeners, err := startHTTPRPCServer(n.config.RPC, env.Logger, routes, n.eventBus) + listeners, err := startRPCServers(n.config.RPC, env.Logger, routes, n.eventBus) if err != nil { return err } @@ -591,7 +591,7 @@ func (n *nodeImpl) OnStart() error { if err != nil { return err } - listener, err := startGRPCServer(n.config.RPC, env, env.Logger) + listener, err := startRPCServer(n.config.RPC, env, env.Logger) if err != nil { return err } diff --git a/node/rpc.go b/node/rpc.go index d4a7cbd12..023895ca2 100644 --- a/node/rpc.go +++ b/node/rpc.go @@ -8,6 +8,7 @@ import ( "time" "github.com/rs/cors" + cfg "github.com/tendermint/tendermint/config" "github.com/tendermint/tendermint/libs/log" tmpubsub "github.com/tendermint/tendermint/libs/pubsub" @@ -18,9 +19,7 @@ import ( "github.com/tendermint/tendermint/types" ) -func startGRPCServer(rpcConfig *cfg.RPCConfig, - env *rpccore.Environment, - logger log.Logger) (net.Listener, error) { +func startRPCServer(rpcConfig *cfg.RPCConfig, env *rpccore.Environment, logger log.Logger) (net.Listener, error) { // we expose a simplified api over grpc for convenience to app devs listener, err := rpcserver.Listen(rpcConfig.GRPCListenAddress, rpcConfig.GRPCMaxOpenConnections) if err != nil { @@ -34,11 +33,7 @@ func startGRPCServer(rpcConfig *cfg.RPCConfig, return listener, nil } -func startHTTPRPCServer(rpcConfig *cfg.RPCConfig, - logger log.Logger, - routes rpccore.RoutesMap, - eventBus types.EventBusSubscriber) ([]net.Listener, error) { - +func startRPCServers(rpcConfig *cfg.RPCConfig, logger log.Logger, routes rpccore.RoutesMap, eventBus types.EventBusSubscriber) ([]net.Listener, error) { config := rpcserver.DefaultConfig() config.MaxBodyBytes = rpcConfig.MaxBodyBytes config.MaxHeaderBytes = rpcConfig.MaxHeaderBytes @@ -58,6 +53,7 @@ func startHTTPRPCServer(rpcConfig *cfg.RPCConfig, mux := http.NewServeMux() registerWebsocketHandler(rpcConfig, mux, routes, logger, eventBus) rpcserver.RegisterRPCFuncs(mux, routes, logger) + listenerAddr := listener.Addr().String() var rootHandler http.Handler = mux if rpcConfig.IsCorsEnabled() { @@ -65,12 +61,10 @@ func startHTTPRPCServer(rpcConfig *cfg.RPCConfig, } if rpcConfig.IsTLSEnabled() { go func() { - listenerAddr := listener.Addr().String() keyFile := rpcConfig.KeyFile() certFile := rpcConfig.CertFile() logger.Info("RPC HTTPS server starting", "address", listenerAddr, "certfile", certFile, "keyfile", keyFile) - err := rpcserver.ServeTLS(listener, rootHandler, keyFile, certFile, logger, config) if !errors.Is(err, net.ErrClosed) { logger.Error("RPC HTTPS server stopped with error", "address", listener, "err", err) @@ -80,7 +74,6 @@ func startHTTPRPCServer(rpcConfig *cfg.RPCConfig, }() } else { go func() { - listenerAddr := listener.Addr().String() logger.Info("RPC HTTPS server starting", "address", listenerAddr) err := rpcserver.Serve(listener, rootHandler, logger, config) @@ -102,7 +95,12 @@ func listenersFromRPCConfig(rpcConfig *cfg.RPCConfig) ([]net.Listener, error) { for i, listenAddr := range listenAddrs { listener, err := rpcserver.Listen(listenAddr, rpcConfig.MaxOpenConnections) if err != nil { - closeOpenListeners(listeners) + // close any listeners opened before returning + for _, l := range listeners { + if l != nil { + l.Close() + } + } return nil, err } listeners[i] = listener @@ -142,9 +140,3 @@ func addCORSHandler(rpcConfig *cfg.RPCConfig, h http.Handler) http.Handler { } return h } - -func closeOpenListeners(listeners []net.Listener) { - for _, listener := range listeners { - listener.Close() - } -} diff --git a/rpc/core/blocks_test.go b/rpc/core/blocks_test.go index cfa970cda..29db2f094 100644 --- a/rpc/core/blocks_test.go +++ b/rpc/core/blocks_test.go @@ -133,4 +133,3 @@ func (mockBlockStore) LoadSeenCommit() *types.Commit { retur func (mockBlockStore) PruneBlocks(height int64) (uint64, error) { return 0, nil } func (mockBlockStore) SaveBlock(block *types.Block, blockParts *types.PartSet, seenCommit *types.Commit) { } -func (mockBlockStore) Close() error { return nil } diff --git a/store/store.go b/store/store.go index 04d21de92..8848b76d9 100644 --- a/store/store.go +++ b/store/store.go @@ -283,10 +283,6 @@ func (bs *BlockStore) LoadSeenCommit() *types.Commit { return commit } -func (bs *BlockStore) Close() error { - return bs.db.Close() -} - // PruneBlocks removes block up to (but not including) a height. It returns the number of blocks pruned. func (bs *BlockStore) PruneBlocks(height int64) (uint64, error) { if height <= 0 {