From f4393ebb0236e4a645aa3699abe50b0a13e74dce Mon Sep 17 00:00:00 2001 From: yihuang Date: Mon, 19 Jul 2021 09:20:55 +0800 Subject: [PATCH 1/2] fix: start GRPCWebServer in goroutine (#9704) so it don't block other code from executing. Closes #9703 ## Description --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) (cherry picked from commit d5674a5d3e82dcf47c4744af339a819e110e89f0) # Conflicts: # CHANGELOG.md --- CHANGELOG.md | 10 ++++++++++ server/grpc/grpc_web.go | 17 +++++++++++++++-- server/grpc/server.go | 2 +- server/start.go | 4 ++-- server/types/app.go | 5 +++++ testutil/network/util.go | 18 ++++-------------- 6 files changed, 37 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35b40b99f091..0e5cc5f7f2be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,11 +44,21 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes +<<<<<<< HEAD * (bank) [\#9687](https://github.com/cosmos/cosmos-sdk/issues/9687) fixes [\#9159](https://github.com/cosmos/cosmos-sdk/issues/9159). Added migration to prune balances with zero coins. ### CLI Breaking Changes * [\#9621](https://github.com/cosmos/cosmos-sdk/pull/9621) Rollback [\#9371](https://github.com/cosmos/cosmos-sdk/pull/9371) and log warning if there's an empty value for min-gas-price in app.toml +======= +* [\#9651](https://github.com/cosmos/cosmos-sdk/pull/9651) Change inconsistent limit of `0` to `MaxUint64` on InfiniteGasMeter and add GasRemaining func to GasMeter. +* [\#9639](https://github.com/cosmos/cosmos-sdk/pull/9639) Check store keys length before accessing them by making sure that `key` is of length `m+1` (for `key[n:m]`) +* (types) [\#9627](https://github.com/cosmos/cosmos-sdk/pull/9627) Fix nil pointer panic on `NewBigIntFromInt` +* (x/genutil) [\#9574](https://github.com/cosmos/cosmos-sdk/pull/9575) Actually use the `gentx` client tx flags (like `--keyring-dir`) +* (x/distribution) [\#9599](https://github.com/cosmos/cosmos-sdk/pull/9599) Withdraw rewards event now includes a value attribute even if there are 0 rewards (due to situations like 100% commission). +* (x/genutil) [\#9638](https://github.com/cosmos/cosmos-sdk/pull/9638) Added missing validator key save when recovering from mnemonic +* (server) [#9704](https://github.com/cosmos/cosmos-sdk/pull/9704) Start GRPCWebServer in goroutine, avoid blocking other services from starting. +>>>>>>> d5674a5d3 (fix: start GRPCWebServer in goroutine (#9704)) ## [v0.43.0-rc0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.43.0-rc0) - 2021-06-25 diff --git a/server/grpc/grpc_web.go b/server/grpc/grpc_web.go index 593779835a78..c2fc023a24f0 100644 --- a/server/grpc/grpc_web.go +++ b/server/grpc/grpc_web.go @@ -1,12 +1,15 @@ package grpc import ( + "fmt" "net/http" + "time" "github.com/improbable-eng/grpc-web/go/grpcweb" "google.golang.org/grpc" "github.com/cosmos/cosmos-sdk/server/config" + "github.com/cosmos/cosmos-sdk/server/types" ) // StartGRPCWeb starts a gRPC-Web server on the given address. @@ -28,8 +31,18 @@ func StartGRPCWeb(grpcSrv *grpc.Server, config config.Config) (*http.Server, err Addr: config.GRPCWeb.Address, Handler: http.HandlerFunc(handler), } - if err := grpcWebSrv.ListenAndServe(); err != nil { + + errCh := make(chan error) + go func() { + if err := grpcWebSrv.ListenAndServe(); err != nil { + errCh <- fmt.Errorf("[grpc] failed to serve: %w", err) + } + }() + + select { + case err := <-errCh: return nil, err + case <-time.After(types.ServerStartTime): // assume server started successfully + return grpcWebSrv, nil } - return grpcWebSrv, nil } diff --git a/server/grpc/server.go b/server/grpc/server.go index 5bc146d8d12d..40a3c7716d97 100644 --- a/server/grpc/server.go +++ b/server/grpc/server.go @@ -54,7 +54,7 @@ func StartGRPCServer(clientCtx client.Context, app types.Application, address st select { case err := <-errCh: return nil, err - case <-time.After(5 * time.Second): // assume server started successfully + case <-time.After(types.ServerStartTime): // assume server started successfully return grpcSrv, nil } } diff --git a/server/start.go b/server/start.go index 79e7bee28304..d26fc7ad7a7a 100644 --- a/server/start.go +++ b/server/start.go @@ -314,7 +314,7 @@ func startInProcess(ctx *Context, clientCtx client.Context, appCreator types.App select { case err := <-errCh: return err - case <-time.After(5 * time.Second): // assume server started successfully + case <-time.After(types.ServerStartTime): // assume server started successfully } } @@ -368,7 +368,7 @@ func startInProcess(ctx *Context, clientCtx client.Context, appCreator types.App select { case err := <-errCh: return err - case <-time.After(5 * time.Second): // assume server started successfully + case <-time.After(types.ServerStartTime): // assume server started successfully } } diff --git a/server/types/app.go b/server/types/app.go index ba702074b2a6..cf6b6ad9a1ff 100644 --- a/server/types/app.go +++ b/server/types/app.go @@ -3,6 +3,7 @@ package types import ( "encoding/json" "io" + "time" "github.com/gogo/protobuf/grpc" "github.com/spf13/cobra" @@ -16,6 +17,10 @@ import ( "github.com/cosmos/cosmos-sdk/server/config" ) +// ServerStartTime defines the time duration that the server need to stay running after startup +// for the startup be considered successful +const ServerStartTime = 5 * time.Second + type ( // AppOptions defines an interface that is passed into an application // constructor, typically used to set BaseApp options that are either supplied diff --git a/testutil/network/util.go b/testutil/network/util.go index c6a2bd815904..e7542113a87b 100644 --- a/testutil/network/util.go +++ b/testutil/network/util.go @@ -16,6 +16,7 @@ import ( "github.com/cosmos/cosmos-sdk/server/api" servergrpc "github.com/cosmos/cosmos-sdk/server/grpc" + srvtypes "github.com/cosmos/cosmos-sdk/server/types" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" "github.com/cosmos/cosmos-sdk/x/genutil" @@ -90,7 +91,7 @@ func startInProcess(cfg Config, val *Validator) error { select { case err := <-errCh: return err - case <-time.After(5 * time.Second): // assume server started successfully + case <-time.After(srvtypes.ServerStartTime): // assume server started successfully } val.api = apiSrv @@ -105,21 +106,10 @@ func startInProcess(cfg Config, val *Validator) error { val.grpc = grpcSrv if val.AppConfig.GRPCWeb.Enable { - errCh1 := make(chan error) - go func() { - grpcWeb, err := servergrpc.StartGRPCWeb(grpcSrv, *val.AppConfig) - if err != nil { - errCh1 <- err - } - - val.grpcWeb = grpcWeb - }() - select { - case err := <-errCh1: + val.grpcWeb, err = servergrpc.StartGRPCWeb(grpcSrv, *val.AppConfig) + if err != nil { return err - case <-time.After(5 * time.Second): // assume server started successfully } - } } From 7b91a8b947e021c8991b56f8ec837b2b1b646be8 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Mon, 19 Jul 2021 10:40:28 +0200 Subject: [PATCH 2/2] fix changelog --- CHANGELOG.md | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e5cc5f7f2be..f93d61b9de63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,11 @@ Ref: https://keepachangelog.com/en/1.0.0/ # Changelog +## [v0.43.0-rc2](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.43.0-rc2) - 2021-07-19 + +### Bug Fixes + +* (server) [#9704](https://github.com/cosmos/cosmos-sdk/pull/9704) Start GRPCWebServer in goroutine, avoid blocking other services from starting. ## [v0.43.0-rc1](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.43.0-rc1) - 2021-07-14 @@ -44,21 +49,11 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes -<<<<<<< HEAD * (bank) [\#9687](https://github.com/cosmos/cosmos-sdk/issues/9687) fixes [\#9159](https://github.com/cosmos/cosmos-sdk/issues/9159). Added migration to prune balances with zero coins. ### CLI Breaking Changes * [\#9621](https://github.com/cosmos/cosmos-sdk/pull/9621) Rollback [\#9371](https://github.com/cosmos/cosmos-sdk/pull/9371) and log warning if there's an empty value for min-gas-price in app.toml -======= -* [\#9651](https://github.com/cosmos/cosmos-sdk/pull/9651) Change inconsistent limit of `0` to `MaxUint64` on InfiniteGasMeter and add GasRemaining func to GasMeter. -* [\#9639](https://github.com/cosmos/cosmos-sdk/pull/9639) Check store keys length before accessing them by making sure that `key` is of length `m+1` (for `key[n:m]`) -* (types) [\#9627](https://github.com/cosmos/cosmos-sdk/pull/9627) Fix nil pointer panic on `NewBigIntFromInt` -* (x/genutil) [\#9574](https://github.com/cosmos/cosmos-sdk/pull/9575) Actually use the `gentx` client tx flags (like `--keyring-dir`) -* (x/distribution) [\#9599](https://github.com/cosmos/cosmos-sdk/pull/9599) Withdraw rewards event now includes a value attribute even if there are 0 rewards (due to situations like 100% commission). -* (x/genutil) [\#9638](https://github.com/cosmos/cosmos-sdk/pull/9638) Added missing validator key save when recovering from mnemonic -* (server) [#9704](https://github.com/cosmos/cosmos-sdk/pull/9704) Start GRPCWebServer in goroutine, avoid blocking other services from starting. ->>>>>>> d5674a5d3 (fix: start GRPCWebServer in goroutine (#9704)) ## [v0.43.0-rc0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.43.0-rc0) - 2021-06-25