Skip to content

Commit

Permalink
Problem: abci handshake is not shutdown gracefully (#288)
Browse files Browse the repository at this point in the history 
* Problem: abci handshake is not shutdown gracefully

Solution:
- use cancellable node constructor

* Update CHANGELOG.md

Signed-off-by: yihuang <[email protected]>

* fix grpc-only

* better app close

* log error

---------

Signed-off-by: yihuang <[email protected]>
  • Loading branch information
@yihuang @mmsqe
yihuang authored and mmsqe committed Sep 5, 2023
1 parent 52b2310 commit ab25767
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 68 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
- (ante) [#1717](https://github.com/evmos/ethermint/pull/1717) Reuse sender recovery result.
- (cli) [#242](https://github.com/crypto-org-chain/ethermint/pull/242) Integrate tendermint bootstrap cmd.
- (cli) [#246](https://github.com/crypto-org-chain/ethermint/pull/246) Call app.Close to cleanup resource on graceful shutdown.
* (cli) [#288](https://github.com/crypto-org-chain/ethermint/pull/288) make abci handshake shutdown gracefully.


## [v0.21.0] - 2023-01-26
Expand Down
5 changes: 2 additions & 3 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ require (
github.com/tidwall/sjson v1.2.5
github.com/tyler-smith/go-bip39 v1.1.0
golang.org/x/net v0.12.0
golang.org/x/sync v0.2.0
golang.org/x/text v0.12.0
google.golang.org/genproto/googleapis/api v0.0.0-20230629202037-9506855d4529
google.golang.org/grpc v1.56.2
Expand Down Expand Up @@ -197,7 +198,6 @@ require (
golang.org/x/crypto v0.11.0 // indirect
golang.org/x/exp v0.0.0-20230711153332-06a737ee72cb // indirect
golang.org/x/oauth2 v0.8.0 // indirect
golang.org/x/sync v0.2.0 // indirect
golang.org/x/sys v0.11.0 // indirect
golang.org/x/term v0.10.0 // indirect
golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
Expand All @@ -217,8 +217,7 @@ require (
replace (
// use cosmos keyring
github.com/99designs/keyring => github.com/cosmos/keyring v1.1.7-0.20210622111912-ef00f8ac3d76
github.com/ChainSafe/go-schnorrkel => github.com/ChainSafe/go-schnorrkel v0.0.0-20200405005733-88cbf1b4c40d
github.com/cometbft/cometbft => github.com/cometbft/cometbft v0.37.2
github.com/cometbft/cometbft => github.com/cometbft/cometbft v0.37.2-0.20230905115601-790d57e1748f
github.com/cometbft/cometbft-db => github.com/crypto-org-chain/cometbft-db v0.0.0-20230412133340-ac70df4b45f6
github.com/cosmos/cosmos-sdk => github.com/crypto-org-chain/cosmos-sdk v0.46.0-beta2.0.20230905040840-b3af5590283b
// Fix upstream GHSA-h395-qcrw-5vmq vulnerability.
Expand Down
8 changes: 4 additions & 4 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,8 @@ filippo.io/edwards25519 v1.0.0/go.mod h1:N1IkdkCkiLB6tki+MYJoSx2JTY9NUlxZE7eHn5E
github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 h1:UQHMgLO+TxOElx5B5HZ4hJQsoJ/PvUvKRhJHDQXO8P8=
github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo=
github.com/ChainSafe/go-schnorrkel v0.0.0-20200405005733-88cbf1b4c40d h1:nalkkPQcITbvhmL4+C4cKA87NW0tfm3Kl9VXRoPywFg=
github.com/ChainSafe/go-schnorrkel v0.0.0-20200405005733-88cbf1b4c40d/go.mod h1:URdX5+vg25ts3aCh8H5IFZybJYKWhJHYMTnf+ULtoC4=
github.com/ChainSafe/go-schnorrkel v1.0.0 h1:3aDA67lAykLaG1y3AOjs88dMxC88PgUuHRrLeDnvGIM=
github.com/ChainSafe/go-schnorrkel v1.0.0/go.mod h1:dpzHYVxLZcp8pjlV+O+UR8K0Hp/z7vcchBSbMBEhCw4=
github.com/DataDog/datadog-go v3.2.0+incompatible/go.mod h1:LButxg5PwREeZtORoXG3tL4fMGNddJ+vMq1mwgfaqoQ=
github.com/DataDog/zstd v1.4.5 h1:EndNeuB0l9syBZhut0wns3gV1hL8zX8LIu6ZiVHWLIQ=
github.com/DataDog/zstd v1.4.5/go.mod h1:1jcaCB/ufaK+sKp1NBhlGmpz41jOoPQ35bpF36t7BBo=
Expand Down Expand Up @@ -340,8 +340,8 @@ github.com/cockroachdb/redact v1.1.5/go.mod h1:BVNblN9mBWFyMyqK1k3AAiSxhvhfK2oOZ
github.com/codahale/hdrhistogram v0.0.0-20161010025455-3a0bb77429bd/go.mod h1:sE/e/2PUdi/liOCUjSTXgM1o87ZssimdTWN964YiIeI=
github.com/coinbase/rosetta-sdk-go/types v1.0.0 h1:jpVIwLcPoOeCR6o1tU+Xv7r5bMONNbHU7MuEHboiFuA=
github.com/coinbase/rosetta-sdk-go/types v1.0.0/go.mod h1:eq7W2TMRH22GTW0N0beDnN931DW0/WOI1R2sdHNHG4c=
github.com/cometbft/cometbft v0.37.2 h1:XB0yyHGT0lwmJlFmM4+rsRnczPlHoAKFX6K8Zgc2/Jc=
github.com/cometbft/cometbft v0.37.2/go.mod h1:Y2MMMN//O5K4YKd8ze4r9jmk4Y7h0ajqILXbH5JQFVs=
github.com/cometbft/cometbft v0.37.2-0.20230905115601-790d57e1748f h1:1v8Q8r5RJhYzxgLY9CpkIQ1dsOpOn5gNLka01kA6C7s=
github.com/cometbft/cometbft v0.37.2-0.20230905115601-790d57e1748f/go.mod h1:gC4iJBO09opzl1pLiPl4fYWn8rRrfO713rRM5tZn5E4=
github.com/confio/ics23/go v0.9.0 h1:cWs+wdbS2KRPZezoaaj+qBleXgUk5WOQFMP3CQFGTr4=
github.com/confio/ics23/go v0.9.0/go.mod h1:4LPZ2NYqnYIVRklaozjNR1FScgDJ2s5Xrp+e/mYVRak=
github.com/containerd/continuity v0.3.0 h1:nisirsYROK15TAMVukJOUyGJjz4BNQJBVsNvAXZJ/eg=
Expand Down
9 changes: 4 additions & 5 deletions gomod2nix.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,8 @@ schema = 3
hash = "sha256-oCalyOZWegRgKHZ1GvYHNkrMKh51j8cOE/K4yBPM5Oc="
replaced = "github.com/cosmos/keyring"
[mod."github.com/ChainSafe/go-schnorrkel"]
version = "v0.0.0-20200405005733-88cbf1b4c40d"
hash = "sha256-i8RXZemJGlSjBT35oPm0SawFiBoIU5Pkq5xp4n/rzCY="
replaced = "github.com/ChainSafe/go-schnorrkel"
version = "v1.0.0"
hash = "sha256-atJ2waz124m0DVHjol8v3NfCLKidU3fYu5AgzT9xCBA="
[mod."github.com/DataDog/zstd"]
version = "v1.4.5"
hash = "sha256-WFHcU2EFRIIc1wSc0jN2VPeJzBMcZTIN5LFNBFk+NAY="
Expand Down Expand Up @@ -121,8 +120,8 @@ schema = 3
version = "v1.0.0"
hash = "sha256-z/0E0NiEGo7zxM7d94ImgUf8P0/KG6hbP9T4Vuym4p0="
[mod."github.com/cometbft/cometbft"]
version = "v0.37.2"
hash = "sha256-lldrYWvcwV6x9uSJWwycf5OCaNkLU6K5g25IbNnMyVE="
version = "v0.37.2-0.20230905115601-790d57e1748f"
hash = "sha256-xWrKY2za+6szbr4FAGUcZKTkP2xgdbCccnqt/RCON8M="
replaced = "github.com/cometbft/cometbft"
[mod."github.com/cometbft/cometbft-db"]
version = "v0.0.0-20230412133340-ac70df4b45f6"
Expand Down
133 changes: 77 additions & 56 deletions server/start.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/crypto/keyring"
"github.com/cosmos/cosmos-sdk/telemetry"
"golang.org/x/sync/errgroup"

"github.com/spf13/cobra"

Expand Down Expand Up @@ -139,7 +140,9 @@ which accepts a path for the resulting pprof file.
withTM, _ := cmd.Flags().GetBool(srvflags.WithTendermint)
if !withTM {
serverCtx.Logger.Info("starting ABCI without Tendermint")
return startStandAlone(serverCtx, opts)
return wrapCPUProfile(serverCtx, func() error {
return startStandAlone(serverCtx, opts)
})
}

serverCtx.Logger.Info("Unlocking keyring")
Expand All @@ -156,7 +159,9 @@ which accepts a path for the resulting pprof file.
serverCtx.Logger.Info("starting ABCI with Tendermint")

// amino is needed here for backwards compatibility of REST routes
err = startInProcess(serverCtx, clientCtx, opts)
err = wrapCPUProfile(serverCtx, func() error {
return startInProcess(serverCtx, clientCtx, opts)
})
errCode, ok := err.(server.ErrorCode)
if !ok {
return err
Expand Down Expand Up @@ -249,6 +254,11 @@ func startStandAlone(ctx *server.Context, opts StartOptions) error {
}

app := opts.AppCreator(ctx.Logger, db, traceWriter, ctx.Viper)
defer func() {
if err := app.Close(); err != nil {
ctx.Logger.Error("close application failed", "error", err.Error())
}
}()

config, err := config.GetConfig(ctx.Viper)
if err != nil {
Expand Down Expand Up @@ -282,68 +292,38 @@ func startStandAlone(ctx *server.Context, opts StartOptions) error {
if err = svr.Stop(); err != nil {
tmos.Exit(err.Error())
}

if err := app.Close(); err != nil {
tmos.Exit(err.Error())
}
}()

// Wait for SIGINT or SIGTERM signal
return server.WaitForQuitSignals()
}

// legacyAminoCdc is used for the legacy REST API
func startInProcess(ctx *server.Context, clientCtx client.Context, opts StartOptions) (err error) {
cfg := ctx.Config
func startInProcess(svrCtx *server.Context, clientCtx client.Context, opts StartOptions) (err error) {
cfg := svrCtx.Config
home := cfg.RootDir
logger := ctx.Logger

if cpuProfile := ctx.Viper.GetString(srvflags.CPUProfile); cpuProfile != "" {
fp, err := ethdebug.ExpandHome(cpuProfile)
if err != nil {
ctx.Logger.Debug("failed to get filepath for the CPU profile file", "error", err.Error())
return err
}

f, err := os.Create(fp)
if err != nil {
return err
}

ctx.Logger.Info("starting CPU profiler", "profile", cpuProfile)
if err := pprof.StartCPUProfile(f); err != nil {
return err
}

defer func() {
ctx.Logger.Info("stopping CPU profiler", "profile", cpuProfile)
pprof.StopCPUProfile()
if err := f.Close(); err != nil {
logger.Error("failed to close CPU profiler file", "error", err.Error())
}
}()
}
logger := svrCtx.Logger

db, err := opts.DBOpener(ctx.Viper, home, server.GetAppDBBackend(ctx.Viper))
traceWriterFile := svrCtx.Viper.GetString(srvflags.TraceStore)
db, err := opts.DBOpener(svrCtx.Viper, home, server.GetAppDBBackend(svrCtx.Viper))
if err != nil {
logger.Error("failed to open DB", "error", err.Error())
return err
}

defer func() {
if err := db.Close(); err != nil {
ctx.Logger.With("error", err).Error("error closing db")
logger.With("error", err).Error("error closing db")
}
}()

traceWriterFile := ctx.Viper.GetString(srvflags.TraceStore)
traceWriter, err := openTraceWriter(traceWriterFile)
if err != nil {
logger.Error("failed to open trace writer", "error", err.Error())
return err
}

config, err := config.GetConfig(ctx.Viper)
config, err := config.GetConfig(svrCtx.Viper)
if err != nil {
logger.Error("failed to get server config", "error", err.Error())
return err
Expand All @@ -354,19 +334,30 @@ func startInProcess(ctx *server.Context, clientCtx client.Context, opts StartOpt
return err
}

app := opts.AppCreator(ctx.Logger, db, traceWriter, ctx.Viper)
app := opts.AppCreator(svrCtx.Logger, db, traceWriter, svrCtx.Viper)
defer func() {
if err := app.Close(); err != nil {
logger.Error("close application failed", "error", err.Error())
}
}()

nodeKey, err := p2p.LoadOrGenNodeKey(cfg.NodeKeyFile())
if err != nil {
logger.Error("failed load or gen node key", "error", err.Error())
return err
}

ctx, cancelFn := context.WithCancel(context.Background())
g, ctx := errgroup.WithContext(ctx)

// listen for quit signals so the calling parent process can gracefully exit
ListenForQuitSignals(g, true, cancelFn, svrCtx.Logger)

genDocProvider := node.DefaultGenesisDocProviderFunc(cfg)

var (
tmNode *node.Node
gRPCOnly = ctx.Viper.GetBool(srvflags.GRPCOnly)
gRPCOnly = svrCtx.Viper.GetBool(srvflags.GRPCOnly)
)

if gRPCOnly {
Expand All @@ -376,15 +367,16 @@ func startInProcess(ctx *server.Context, clientCtx client.Context, opts StartOpt
} else {
logger.Info("starting node with ABCI Tendermint in-process")

tmNode, err = node.NewNode(
tmNode, err = node.NewNodeWithContext(
ctx,
cfg,
pvm.LoadOrGenFilePV(cfg.PrivValidatorKeyFile(), cfg.PrivValidatorStateFile()),
nodeKey,
proxy.NewLocalClientCreator(app),
genDocProvider,
node.DefaultDBProvider,
node.DefaultMetricsProvider(cfg.Instrumentation),
ctx.Logger.With("server", "node"),
logger.With("server", "node"),
)
if err != nil {
logger.Error("failed init node", "error", err.Error())
Expand All @@ -399,7 +391,6 @@ func startInProcess(ctx *server.Context, clientCtx client.Context, opts StartOpt
defer func() {
if tmNode.IsRunning() {
_ = tmNode.Stop()
_ = app.Close()
}
}()
}
Expand All @@ -422,19 +413,19 @@ func startInProcess(ctx *server.Context, clientCtx client.Context, opts StartOpt

// Enable metrics if JSONRPC is enabled and --metrics is passed
// Flag not added in config to avoid user enabling in config without passing in CLI
if config.JSONRPC.Enable && ctx.Viper.GetBool(srvflags.JSONRPCEnableMetrics) {
if config.JSONRPC.Enable && svrCtx.Viper.GetBool(srvflags.JSONRPCEnableMetrics) {
ethmetricsexp.Setup(config.JSONRPC.MetricsAddress)
}

var idxer ethermint.EVMTxIndexer
if config.JSONRPC.EnableIndexer {
idxDB, err := OpenIndexerDB(home, server.GetAppDBBackend(ctx.Viper))
idxDB, err := OpenIndexerDB(home, server.GetAppDBBackend(svrCtx.Viper))
if err != nil {
logger.Error("failed to open evm indexer DB", "error", err.Error())
return err
}

idxLogger := ctx.Logger.With("indexer", "evm")
idxLogger := logger.With("indexer", "evm")
idxer = indexer.NewKVIndexer(idxDB, idxLogger, clientCtx)
indexerService := NewEVMIndexerService(idxer, clientCtx.Client.(rpcclient.Client))
indexerService.SetLogger(idxLogger)
Expand Down Expand Up @@ -498,13 +489,13 @@ func startInProcess(ctx *server.Context, clientCtx client.Context, opts StartOpt
}

clientCtx = clientCtx.WithGRPCClient(grpcClient)
ctx.Logger.Debug("gRPC client assigned to client context", "address", grpcAddress)
svrCtx.Logger.Debug("gRPC client assigned to client context", "address", grpcAddress)
}
}

var apiSrv *api.Server
if config.API.Enable {
apiSrv = api.New(clientCtx, ctx.Logger.With("server", "api"))
apiSrv = api.New(clientCtx, svrCtx.Logger.With("server", "api"))
app.RegisterAPIRoutes(apiSrv, config.API)

if config.Telemetry.Enabled {
Expand Down Expand Up @@ -541,7 +532,7 @@ func startInProcess(ctx *server.Context, clientCtx client.Context, opts StartOpt
if config.GRPCWeb.Enable {
grpcWebSrv, err = servergrpc.StartGRPCWeb(grpcSrv, config.Config)
if err != nil {
ctx.Logger.Error("failed to start grpc-web http server", "error", err.Error())
logger.Error("failed to start grpc-web http server", "error", err.Error())
return err
}

Expand All @@ -568,7 +559,7 @@ func startInProcess(ctx *server.Context, clientCtx client.Context, opts StartOpt

tmEndpoint := "/websocket"
tmRPCAddr := cfg.RPC.ListenAddress
httpSrv, httpSrvDone, err = StartJSONRPC(ctx, clientCtx, tmRPCAddr, tmEndpoint, &config, idxer)
httpSrv, httpSrvDone, err = StartJSONRPC(svrCtx, clientCtx, tmRPCAddr, tmEndpoint, &config, idxer)
if err != nil {
return err
}
Expand All @@ -591,7 +582,7 @@ func startInProcess(ctx *server.Context, clientCtx client.Context, opts StartOpt
// we do not need to start Rosetta or handle any Tendermint related processes.
if gRPCOnly {
// wait for signal capture and gracefully return
return server.WaitForQuitSignals()
return g.Wait()
}

var rosettaSrv crgserver.Server
Expand All @@ -606,14 +597,14 @@ func startInProcess(ctx *server.Context, clientCtx client.Context, opts StartOpt

minGasPrices, err := sdk.ParseDecCoins(config.MinGasPrices)
if err != nil {
ctx.Logger.Error("failed to parse minimum-gas-prices", "error", err.Error())
logger.Error("failed to parse minimum-gas-prices", "error", err.Error())
return err
}

conf := &rosetta.Config{
Blockchain: config.Rosetta.Blockchain,
Network: config.Rosetta.Network,
TendermintRPC: ctx.Config.RPC.ListenAddress,
TendermintRPC: svrCtx.Config.RPC.ListenAddress,
GRPCEndpoint: config.GRPC.Address,
Addr: config.Rosetta.Address,
Retries: config.Rosetta.Retries,
Expand Down Expand Up @@ -643,8 +634,8 @@ func startInProcess(ctx *server.Context, clientCtx client.Context, opts StartOpt
case <-time.After(types.ServerStartTime): // assume server started successfully
}
}
// Wait for SIGINT or SIGTERM signal
return server.WaitForQuitSignals()

return g.Wait()
}

func openDB(_ types.AppOptions, rootDir string, backendType dbm.BackendType) (dbm.DB, error) {
Expand Down Expand Up @@ -677,3 +668,33 @@ func startTelemetry(cfg config.Config) (*telemetry.Metrics, error) {
}
return telemetry.New(cfg.Telemetry)
}

// wrapCPUProfile runs callback in a goroutine, then wait for quit signals.
func wrapCPUProfile(ctx *server.Context, callback func() error) error {
if cpuProfile := ctx.Viper.GetString(srvflags.CPUProfile); cpuProfile != "" {
fp, err := ethdebug.ExpandHome(cpuProfile)
if err != nil {
ctx.Logger.Debug("failed to get filepath for the CPU profile file", "error", err.Error())
return err
}
f, err := os.Create(fp)
if err != nil {
return err
}

ctx.Logger.Info("starting CPU profiler", "profile", cpuProfile)
if err := pprof.StartCPUProfile(f); err != nil {
return err
}

defer func() {
ctx.Logger.Info("stopping CPU profiler", "profile", cpuProfile)
pprof.StopCPUProfile()
if err := f.Close(); err != nil {
ctx.Logger.Info("failed to close cpu-profile file", "profile", cpuProfile, "err", err.Error())
}
}()
}

return callback()
}
Loading

0 comments on commit ab25767

Please sign in to comment.