Skip to content

Commit

Permalink
fix(server/v2): improve server stop (#22455)
Browse files Browse the repository at this point in the history
  • Loading branch information
julienrbrt authored Nov 8, 2024
1 parent 3ac1665 commit 6eca7eb
Show file tree
Hide file tree
Showing 14 changed files with 57 additions and 95 deletions.
5 changes: 3 additions & 2 deletions server/v2/api/grpc/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"cosmossdk.io/core/transaction"
"cosmossdk.io/log"
serverv2 "cosmossdk.io/server/v2"
"cosmossdk.io/server/v2/api"
"cosmossdk.io/server/v2/api/grpc/gogoreflection"
)

Expand Down Expand Up @@ -224,7 +223,9 @@ func (s *Server[T]) Stop(ctx context.Context) error {
}

s.logger.Info("stopping gRPC server...", "address", s.config.Address)
return api.DoUntilCtxExpired(ctx, s.grpcSrv.GracefulStop)
s.grpcSrv.GracefulStop()

return nil
}

// GetGRPCServer returns the underlying gRPC server.
Expand Down
10 changes: 4 additions & 6 deletions server/v2/api/rest/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,22 @@ const (
)

type Server[T transaction.Tx] struct {
logger log.Logger
router *http.ServeMux

logger log.Logger
router *http.ServeMux
httpServer *http.Server
config *Config
cfgOptions []CfgOption
}

func New[T transaction.Tx](
appManager appmanager.AppManager[T],
logger log.Logger,
appManager appmanager.AppManager[T],
cfg server.ConfigMap,
cfgOptions ...CfgOption,
) (*Server[T], error) {
srv := &Server[T]{
cfgOptions: cfgOptions,
logger: logger.With(log.ModuleKey, ServerName),
cfgOptions: cfgOptions,
router: http.NewServeMux(),
}

Expand Down Expand Up @@ -90,7 +89,6 @@ func (s *Server[T]) Stop(ctx context.Context) error {
}

s.logger.Info("stopping HTTP server")

return s.httpServer.Shutdown(ctx)
}

Expand Down
2 changes: 1 addition & 1 deletion server/v2/api/telemetry/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ var (
const ServerName = "telemetry"

type Server[T transaction.Tx] struct {
config *Config
logger log.Logger
config *Config
server *http.Server
metrics *Metrics
}
Expand Down
22 changes: 0 additions & 22 deletions server/v2/api/utils.go

This file was deleted.

35 changes: 0 additions & 35 deletions server/v2/api/utils_test.go

This file was deleted.

6 changes: 4 additions & 2 deletions server/v2/cometbft/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,13 @@ func New[T transaction.Tx](
// fallback to genesis chain-id
reader, err := os.Open(filepath.Join(home, "config", "genesis.json"))
if err != nil {
panic(err)
return nil, fmt.Errorf("failed to open genesis file: %w", err)
}
defer reader.Close()

chainID, err = genutiltypes.ParseChainIDFromGenesis(reader)
if err != nil {
panic(fmt.Errorf("failed to parse chain-id from genesis file: %w", err))
return nil, fmt.Errorf("failed to parse chain-id from genesis file: %w", err)
}
}

Expand Down Expand Up @@ -216,11 +216,13 @@ func (s *CometBFTServer[T]) Start(ctx context.Context) error {
return err
}

s.logger.Info("starting consensus server")
return s.Node.Start()
}

func (s *CometBFTServer[T]) Stop(context.Context) error {
if s.Node != nil && s.Node.IsRunning() {
s.logger.Info("stopping consensus server")
return s.Node.Stop()
}

Expand Down
19 changes: 14 additions & 5 deletions server/v2/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package serverv2
import (
"context"
"errors"
"io"
"os"
"os/signal"
"runtime/pprof"
Expand All @@ -22,6 +23,7 @@ import (
func AddCommands[T transaction.Tx](
rootCmd *cobra.Command,
logger log.Logger,
appCloser io.Closer,
globalAppConfig server.ConfigMap,
globalServerConfig ServerConfig,
components ...ServerComponent[T],
Expand All @@ -31,7 +33,7 @@ func AddCommands[T transaction.Tx](
}
srv := NewServer(globalServerConfig, components...)
cmds := srv.CLICommands()
startCmd := createStartCommand(srv, globalAppConfig, logger)
startCmd := createStartCommand(srv, appCloser, globalAppConfig, logger)
startCmd.SetContext(rootCmd.Context())
cmds.Commands = append(cmds.Commands, startCmd)
rootCmd.AddCommand(cmds.Commands...)
Expand Down Expand Up @@ -63,6 +65,7 @@ func AddCommands[T transaction.Tx](
// createStartCommand creates the start command for the application.
func createStartCommand[T transaction.Tx](
server *Server[T],
appCloser io.Closer,
config server.ConfigMap,
logger log.Logger,
) *cobra.Command {
Expand All @@ -85,13 +88,19 @@ func createStartCommand[T transaction.Tx](
// don't block waiting for the OS signal before stopping the server.
cancelFn()
}

if err := server.Stop(ctx); err != nil {
cmd.PrintErrln("failed to stop servers:", err)
}
}()

return wrapCPUProfile(logger, config, func() error {
defer func() {
if err := server.Stop(cmd.Context()); err != nil {
cmd.PrintErrln("failed to stop servers:", err)
}

if err := appCloser.Close(); err != nil {
cmd.PrintErrln("failed to close application:", err)
}
}()

return server.Start(ctx)
})
},
Expand Down
2 changes: 1 addition & 1 deletion server/v2/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ require (
github.com/spf13/pflag v1.0.5
github.com/spf13/viper v1.19.0
github.com/stretchr/testify v1.9.0
golang.org/x/sync v0.9.0
google.golang.org/grpc v1.68.0
google.golang.org/protobuf v1.35.1
)
Expand Down Expand Up @@ -102,6 +101,7 @@ require (
golang.org/x/exp v0.0.0-20240531132922-fd00a4e0eefc // indirect
golang.org/x/mod v0.17.0 // indirect
golang.org/x/net v0.30.0 // indirect
golang.org/x/sync v0.9.0 // indirect
golang.org/x/sys v0.27.0 // indirect
golang.org/x/text v0.20.0 // indirect
golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect
Expand Down
30 changes: 14 additions & 16 deletions server/v2/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package serverv2

import (
"context"
"errors"
"fmt"
"os"
"path/filepath"
Expand All @@ -10,7 +11,6 @@ import (
"github.com/pelletier/go-toml/v2"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
"golang.org/x/sync/errgroup"

"cosmossdk.io/core/transaction"
"cosmossdk.io/log"
Expand Down Expand Up @@ -87,17 +87,17 @@ func (s *Server[T]) Name() string {

// Start starts all components concurrently.
func (s *Server[T]) Start(ctx context.Context) error {
logger := GetLoggerFromContext(ctx)
logger.With(log.ModuleKey, s.Name()).Info("starting servers...")
logger := GetLoggerFromContext(ctx).With(log.ModuleKey, s.Name())
logger.Info("starting servers...")

g, ctx := errgroup.WithContext(ctx)
resCh := make(chan error, len(s.components))
for _, mod := range s.components {
g.Go(func() error {
return mod.Start(ctx)
})
go func() {
resCh <- mod.Start(ctx)
}()
}

if err := g.Wait(); err != nil {
if err := <-resCh; err != nil {
return fmt.Errorf("failed to start servers: %w", err)
}

Expand All @@ -106,19 +106,17 @@ func (s *Server[T]) Start(ctx context.Context) error {
return nil
}

// Stop stops all components concurrently.
// Stop stops all server components synchronously.
func (s *Server[T]) Stop(ctx context.Context) error {
logger := GetLoggerFromContext(ctx)
logger.With(log.ModuleKey, s.Name()).Info("stopping servers...")
logger := GetLoggerFromContext(ctx).With(log.ModuleKey, s.Name())
logger.Info("stopping servers...")

g, ctx := errgroup.WithContext(ctx)
var err error
for _, mod := range s.components {
g.Go(func() error {
return mod.Stop(ctx)
})
err = errors.Join(err, mod.Stop(ctx))
}

return g.Wait()
return err
}

// CLICommands returns all CLI commands of all components.
Expand Down
1 change: 0 additions & 1 deletion server/v2/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ func TestServer(t *testing.T) {
cfg := v.AllSettings()

logger := log.NewLogger(os.Stdout)

ctx := serverv2.SetServerContext(context.Background(), v, logger)

grpcServer, err := grpc.New[transaction.Tx](logger, &mockInterfaceRegistry{}, map[string]appmodulev2.Handler{}, nil, cfg)
Expand Down
4 changes: 2 additions & 2 deletions server/v2/store/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ const ServerName = "store"
// Server manages store config and contains prune & snapshot commands
type Server[T transaction.Tx] struct {
config *root.Config
store storev2.RootStore
store storev2.Backend
}

func New[T transaction.Tx](store storev2.RootStore, cfg server.ConfigMap) (*Server[T], error) {
func New[T transaction.Tx](store storev2.Backend, cfg server.ConfigMap) (*Server[T], error) {
config, err := UnmarshalConfig(cfg)
if err != nil {
return nil, err
Expand Down
9 changes: 9 additions & 0 deletions simapp/v2/app_di.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,15 @@ func (app *SimApp[T]) Store() store.RootStore {
return app.store
}

// Close overwrites the base Close method to close the stores.
func (app *SimApp[T]) Close() error {
if err := app.store.Close(); err != nil {
return err
}

return app.App.Close()
}

func ProvideRootStoreConfig(config runtime.GlobalConfig) (*root.Config, error) {
return serverstore.UnmarshalConfig(config)
}
5 changes: 4 additions & 1 deletion simapp/v2/simdv2/cmd/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cmd

import (
"errors"
"io"

"github.com/spf13/cobra"

Expand Down Expand Up @@ -71,6 +72,7 @@ func InitRootCmd[T transaction.Tx](
return serverv2.AddCommands[T](
rootCmd,
logger,
io.NopCloser(nil),
deps.GlobalConfig,
initServerConfig(),
deps.Consensus,
Expand All @@ -92,7 +94,7 @@ func InitRootCmd[T transaction.Tx](
if err != nil {
return nil, err
}
restServer, err := rest.New[T](simApp.App.AppManager, logger, deps.GlobalConfig)
restServer, err := rest.New[T](logger, simApp.App.AppManager, deps.GlobalConfig)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -123,6 +125,7 @@ func InitRootCmd[T transaction.Tx](
return serverv2.AddCommands[T](
rootCmd,
logger,
simApp,
deps.GlobalConfig,
initServerConfig(),
deps.Consensus,
Expand Down
2 changes: 1 addition & 1 deletion simapp/v2/simdv2/cmd/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func initCometConfig() cometbft.CfgOption {
cfg := cmtcfg.DefaultConfig()

// display only warn logs by default except for p2p and state
cfg.LogLevel = "*:warn,server:info,p2p:info,state:info"
cfg.LogLevel = "*:warn,p2p:info,state:info,server:info,telemetry:info,grpc:info,rest:info,grpc-gateway:info,comet:info,store:info"
// increase block timeout
cfg.Consensus.TimeoutCommit = 5 * time.Second
// overwrite default pprof listen address
Expand Down

0 comments on commit 6eca7eb

Please sign in to comment.