Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Modify grpc gateway to be concurrent (backport #11234) #11536

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* [\#11124](https://github.com/cosmos/cosmos-sdk/pull/11124) Add `GetAllVersions` to application store
* (x/auth) [\#10880](https://github.com/cosmos/cosmos-sdk/pull/10880) Added a new query to the tx query service that returns a block with transactions fully decoded.
* [#11314](https://github.com/cosmos/cosmos-sdk/pull/11314) Add state rollback command.
* [\#11234](https://github.com/cosmos/cosmos-sdk/pull/11234) Add `GRPCClient` field to Client Context. If `GRPCClient` field is set to nil, the `Invoke` method would use ABCI query, otherwise use gprc.

### Bug Fixes

Expand Down
15 changes: 11 additions & 4 deletions client/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,12 @@ import (
"io"
"os"

"github.com/spf13/viper"

"gopkg.in/yaml.v2"

"github.com/gogo/protobuf/proto"
"github.com/pkg/errors"
"github.com/spf13/viper"
rpcclient "github.com/tendermint/tendermint/rpc/client"
"google.golang.org/grpc"
"gopkg.in/yaml.v2"

"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
Expand All @@ -28,6 +27,7 @@ type Context struct {
ChainID string
// Deprecated: Codec codec will be changed to Codec: codec.Codec
JSONCodec codec.JSONCodec
GRPCClient *grpc.ClientConn
Codec codec.Codec
InterfaceRegistry codectypes.InterfaceRegistry
Input io.Reader
Expand Down Expand Up @@ -140,6 +140,13 @@ func (ctx Context) WithClient(client rpcclient.Client) Context {
return ctx
}

// WithGRPCClient returns a copy of the context with an updated GRPC client
// instance.
func (ctx Context) WithGRPCClient(grpcClient *grpc.ClientConn) Context {
ctx.GRPCClient = grpcClient
return ctx
}

// WithUseLedger returns a copy of the context with an updated UseLedger flag.
func (ctx Context) WithUseLedger(useLedger bool) Context {
ctx.UseLedger = useLedger
Expand Down
10 changes: 8 additions & 2 deletions client/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ var protoCodec = encoding.GetCodec(proto.Name)
func (ctx Context) Invoke(grpcCtx gocontext.Context, method string, req, reply interface{}, opts ...grpc.CallOption) (err error) {
// Two things can happen here:
// 1. either we're broadcasting a Tx, in which call we call Tendermint's broadcast endpoint directly,
// 2. or we are querying for state, in which case we call ABCI's Query.
// 2-1. or we are querying for state, in which case we call grpc if grpc client set.
// 2-2. or we are querying for state, in which case we call ABCI's Query if grpc client not set.

// In both cases, we don't allow empty request args (it will panic unexpectedly).
if reflect.ValueOf(req).IsNil() {
Expand All @@ -50,7 +51,12 @@ func (ctx Context) Invoke(grpcCtx gocontext.Context, method string, req, reply i
return err
}

// Case 2. Querying state.
if ctx.GRPCClient != nil {
// Case 2-1. Invoke grpc.
return ctx.GRPCClient.Invoke(grpcCtx, method, req, reply, opts...)
}

// Case 2-2. Querying state via abci query.
reqBz, err := protoCodec.Marshal(req)
if err != nil {
return err
Expand Down
21 changes: 21 additions & 0 deletions server/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package server

import (
"fmt"
"net"
"net/http"
"os"
"runtime/pprof"
Expand Down Expand Up @@ -304,6 +305,26 @@ func startInProcess(ctx *Context, clientCtx client.Context, appCreator types.App

clientCtx := clientCtx.WithHomeDir(home).WithChainID(genDoc.ChainID)

if config.GRPC.Enable {
_, port, err := net.SplitHostPort(config.GRPC.Address)
if err != nil {
return err
}
grpcAddress := fmt.Sprintf("127.0.0.1:%s", port)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

config.GRPC.Address specifies both host and port, but here you force host to 127.0.0.1 in the address which you dial. If the configured address isn't reachable via that IP, will this break?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you cannot reach 127.0.0.1, yes, this would not work. But this isn't the original PR, so I'm not sure why this was needed.

cc @Thunnini

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you cannot reach 127.0.0.1, yes, this would not work.

I guess I'm asking about the situation where the gRPC address was set to a non-localhost IP. For example, if you have a host with a network interface that has a [public] IP of 1.2.3.4 you can bind a listener to that IP and it won't be reachable via 127.0.0.1.


// If grpc is enabled, configure grpc client for grpc gateway.
grpcClient, err := grpc.Dial(
grpcAddress,
grpc.WithInsecure(),
)
if err != nil {
return err
}

clientCtx = clientCtx.WithGRPCClient(grpcClient)
ctx.Logger.Debug("grpc client assigned to client context", "target", grpcAddress)
}

apiSrv = api.New(clientCtx, ctx.Logger.With("module", "api-server"))
app.RegisterAPIRoutes(apiSrv, config.API)
errCh := make(chan error)
Expand Down