From b500b0104fc8430d8674afb43e4e0cc8c0c4bde1 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Tue, 23 May 2023 11:25:47 +0200 Subject: [PATCH] refactor: Apply Yihuang's server blocking improvement (#16252) --- server/start.go | 29 +++++++---------------------- server/util.go | 20 +++++++++++++++----- 2 files changed, 22 insertions(+), 27 deletions(-) diff --git a/server/start.go b/server/start.go index 38d4214ba742..fd895b9cb921 100644 --- a/server/start.go +++ b/server/start.go @@ -262,8 +262,7 @@ func startStandAlone(svrCtx *Context, app types.Application, opts StartCmdOption svr.SetLogger(servercmtlog.CometLoggerWrapper{Logger: svrCtx.Logger.With("module", "abci-server")}) - ctx := getCtx(svrCtx) - g, ctx := errgroup.WithContext(ctx) + g, ctx := getCtx(svrCtx, false) g.Go(func() error { if err := svr.Start(); err != nil { @@ -315,8 +314,7 @@ func startInProcess(svrCtx *Context, svrCfg serverconfig.Config, clientCtx clien } } - ctx := getCtx(svrCtx) - g, ctx := errgroup.WithContext(ctx) + g, ctx := getCtx(svrCtx, true) grpcSrv, clientCtx, err := startGrpcServer(ctx, g, svrCfg.GRPC, clientCtx, svrCtx, app) if err != nil { @@ -334,22 +332,8 @@ func startInProcess(svrCtx *Context, svrCfg serverconfig.Config, clientCtx clien } } - // At this point it is safe to block the process if we're in gRPC-only mode as - // we do not need to handle any CometBFT related processes. - if gRPCOnly { - // wait for signal capture and gracefully return - return g.Wait() - } - - // In case the operator has both gRPC and API servers disabled, there is - // nothing blocking this root process, so we need to block manually, so we'll - // create an empty blocking loop. - g.Go(func() error { - <-ctx.Done() - return nil - }) - // wait for signal capture and gracefully return + // we are guaranteed to be waiting for the "ListenForQuitSignals" goroutine. return g.Wait() } @@ -375,11 +359,12 @@ func startApp(svrCtx *Context, appCreator types.AppCreator, opts StartCmdOptions return app, cleanupFn, nil } -func getCtx(svrCtx *Context) context.Context { +func getCtx(svrCtx *Context, block bool) (*errgroup.Group, context.Context) { ctx, cancelFn := context.WithCancel(context.Background()) + g, ctx := errgroup.WithContext(ctx) // listen for quit signals so the calling parent process can gracefully exit - ListenForQuitSignals(cancelFn, svrCtx.Logger) - return ctx + ListenForQuitSignals(g, block, cancelFn, svrCtx.Logger) + return g, ctx } func startCmtNode(cfg *cmtcfg.Config, app types.Application, svrCtx *Context) (tmNode *node.Node, cleanupFn func(), err error) { diff --git a/server/util.go b/server/util.go index 16cd9a9fe4b7..14a94581e343 100644 --- a/server/util.go +++ b/server/util.go @@ -22,6 +22,7 @@ import ( "github.com/spf13/cobra" "github.com/spf13/pflag" "github.com/spf13/viper" + "golang.org/x/sync/errgroup" "cosmossdk.io/log" "cosmossdk.io/store" @@ -371,18 +372,27 @@ func ExternalIP() (string, error) { // the cleanup function is called, indicating the caller can gracefully exit or // return. // -// Note, this performs a non-blocking process so the caller must ensure the -// corresponding context derived from the cancelFn is used correctly. -func ListenForQuitSignals(cancelFn context.CancelFunc, logger log.Logger) { +// Note, the blocking behavior of this depends on the block argument. +// The caller must ensure the corresponding context derived from the cancelFn is used correctly. +func ListenForQuitSignals(g *errgroup.Group, block bool, cancelFn context.CancelFunc, logger log.Logger) { sigCh := make(chan os.Signal, 1) signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM) - go func() { + f := func() { sig := <-sigCh cancelFn() logger.Info("caught signal", "signal", sig.String()) - }() + } + + if block { + g.Go(func() error { + f() + return nil + }) + } else { + go f() + } } // GetAppDBBackend gets the backend type to use for the application DBs.