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 #11234

Merged
merged 5 commits into from
Mar 9, 2022

Conversation

Thunnini
Copy link
Contributor

Current grpc happens to be concurrent, while the grpc gateway itself is not, since it always uses abci query. Therefore, as the current queries are not concurrent, throughput has the room for improvement. This PR changes the grpc gateway so that when server is ran by a node daemon, it directly calls grpc to make queries concurrent. Any services that uses grpc gateway could improve throughput by fundamental amount, which has been tested and ensured in the process of running an Osmosis node using the current chagnes.

The code base has the following changes:

  • GRPCClient field has been added to Client Context.
  • The Invoke method in Client Context would use ABCI query when GRPCClient field is set to nil, otherwise use the GRPC Client to return results that have used grpc.
  • If GRPC is set to enable in startInProcess, it sets the GRPC Client field in Client Context.

@Thunnini Thunnini changed the title Modify grpc gateway to be concurrent feat: Modify grpc gateway to be concurrent Feb 20, 2022
client/grpc_query.go Outdated Show resolved Hide resolved
server/start.go Outdated Show resolved Hide resolved
client/grpc_query.go Outdated Show resolved Hide resolved
Copy link
Member

@tac0turtle tac0turtle left a comment

Choose a reason for hiding this comment

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

utACK.

Copy link
Contributor

@fdymylja fdymylja left a comment

Choose a reason for hiding this comment

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

Let's implement this as a:

switch ctx.GRPCClient {
case nil:
   abci query
default:
    grpc query   
}

This makes code more readable.

server/start.go Outdated Show resolved Hide resolved
@amaury1093 amaury1093 self-assigned this Feb 28, 2022
@tac0turtle
Copy link
Member

@Thunnini any chance you can update the code and then we can get this merged?

Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

Love it.

Comment on lines +12 to +13
"google.golang.org/grpc"

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"google.golang.org/grpc"
"google.golang.org/grpc"

server/start.go Outdated Show resolved Hide resolved
@Thunnini
Copy link
Contributor Author

Thunnini commented Mar 7, 2022

@marbar3778 Srry things got delayed, I was out bc of covid 😥 Updated reflecting the reviews!

  • Modified so that grpcClient only connects through 127.0.0.1
  • use grpc.WithTransportCredential(insecure.NewCredentials()) instead of grpc.WithInsecure() (deprecated)
  • add clientCtx.InterfaceRegistry to grpcClient call options using grpc.ForceCodec

@tac0turtle tac0turtle added the A:automerge Automatically merge PR once all prerequisites pass. label Mar 9, 2022
@mergify mergify bot merged commit 5356a86 into cosmos:master Mar 9, 2022
p0mvn added a commit to osmosis-labs/cosmos-sdk that referenced this pull request Mar 10, 2022
ValarDragon pushed a commit to osmosis-labs/cosmos-sdk that referenced this pull request Mar 11, 2022
faddat pushed a commit to notional-labs/cosmos-sdk that referenced this pull request Mar 21, 2022
@tac0turtle
Copy link
Member

@Mergifyio backport release/v0.45.x

@mergify
Copy link
Contributor

mergify bot commented Apr 4, 2022

backport release/v0.45.x

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Apr 4, 2022
Current grpc happens to be concurrent, while the grpc gateway itself is not, since it always uses abci query. Therefore, as the current queries are not concurrent, throughput has the room for improvement. This PR changes the grpc gateway so that when server is ran by a node daemon, it directly calls grpc to make queries concurrent. Any services that uses grpc gateway could improve throughput by fundamental amount, which has been tested and ensured in the process of running an Osmosis node using the current chagnes.

The code base has the following changes:
- GRPCClient field has been added to Client Context.
- The `Invoke` method in Client Context would use ABCI query when GRPCClient field is set to nil, otherwise use the GRPC Client to return results that have used grpc.
- If GRPC is set to enable in `startInProcess`, it sets the GRPC Client field in Client Context.

(cherry picked from commit 5356a86)

# Conflicts:
#	CHANGELOG.md
#	client/context.go
#	client/grpc_query.go
yihuang added a commit to yihuang/ethermint that referenced this pull request Sep 20, 2022
Solution:
- Initiate the GRPCClient introduced in [sdk 0.46](cosmos/cosmos-sdk#11234).
fedekunze added a commit to evmos/ethermint that referenced this pull request Sep 20, 2022
* Problem: grpc query is not run concurrently

Solution:
- Initiate the GRPCClient introduced in [sdk 0.46](cosmos/cosmos-sdk#11234).

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update server/start.go

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

* wrap error

* Update server/start.go

* more complete copy

* fix grpc crash

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
SmartDev-0205 added a commit to SmartDev-0205/ETHERMINT that referenced this pull request Nov 2, 2022
* Problem: grpc query is not run concurrently

Solution:
- Initiate the GRPCClient introduced in [sdk 0.46](cosmos/cosmos-sdk#11234).

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update server/start.go

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

* wrap error

* Update server/start.go

* more complete copy

* fix grpc crash

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
T0psecurity added a commit to T0psecurity/ethermint that referenced this pull request May 10, 2023
* Problem: grpc query is not run concurrently

Solution:
- Initiate the GRPCClient introduced in [sdk 0.46](cosmos/cosmos-sdk#11234).

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update server/start.go

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

* wrap error

* Update server/start.go

* more complete copy

* fix grpc crash

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
mjtechworks pushed a commit to mjtechworks/ethermint that referenced this pull request May 10, 2023
* Problem: grpc query is not run concurrently

Solution:
- Initiate the GRPCClient introduced in [sdk 0.46](cosmos/cosmos-sdk#11234).

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update server/start.go

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

* wrap error

* Update server/start.go

* more complete copy

* fix grpc crash

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
splend1d-man pushed a commit to SigmaGmbH/swisstronik-evm-module that referenced this pull request Jul 25, 2023
* Problem: grpc query is not run concurrently

Solution:
- Initiate the GRPCClient introduced in [sdk 0.46](cosmos/cosmos-sdk#11234).

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update server/start.go

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

* wrap error

* Update server/start.go

* more complete copy

* fix grpc crash

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
niceDeve added a commit to niceDeve/ethermint that referenced this pull request Sep 2, 2023
* Problem: grpc query is not run concurrently

Solution:
- Initiate the GRPCClient introduced in [sdk 0.46](cosmos/cosmos-sdk#11234).

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update server/start.go

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

* wrap error

* Update server/start.go

* more complete copy

* fix grpc crash

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
ivanshukhov pushed a commit to planq-network/ethermint that referenced this pull request Nov 7, 2023
* Problem: grpc query is not run concurrently

Solution:
- Initiate the GRPCClient introduced in [sdk 0.46](cosmos/cosmos-sdk#11234).

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update server/start.go

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

* wrap error

* Update server/start.go

* more complete copy

* fix grpc crash

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
(cherry picked from commit 6574019)
NinjaDev0107 added a commit to NinjaDev0107/ethermint that referenced this pull request Nov 20, 2023
* Problem: grpc query is not run concurrently

Solution:
- Initiate the GRPCClient introduced in [sdk 0.46](cosmos/cosmos-sdk#11234).

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update server/start.go

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

* wrap error

* Update server/start.go

* more complete copy

* fix grpc crash

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
IceCaptain0715 added a commit to IceCaptain0715/ethermint that referenced this pull request Nov 21, 2023
* Problem: grpc query is not run concurrently

Solution:
- Initiate the GRPCClient introduced in [sdk 0.46](cosmos/cosmos-sdk#11234).

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update server/start.go

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

* wrap error

* Update server/start.go

* more complete copy

* fix grpc crash

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
JoeDev0107 added a commit to JoeDev0107/ethermint that referenced this pull request Nov 27, 2023
* Problem: grpc query is not run concurrently

Solution:
- Initiate the GRPCClient introduced in [sdk 0.46](cosmos/cosmos-sdk#11234).

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update server/start.go

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

* wrap error

* Update server/start.go

* more complete copy

* fix grpc crash

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
morgan1119 added a commit to morgan1119/ethermint that referenced this pull request Apr 10, 2024
* Problem: grpc query is not run concurrently

Solution:
- Initiate the GRPCClient introduced in [sdk 0.46](cosmos/cosmos-sdk#11234).

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update server/start.go

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

* wrap error

* Update server/start.go

* more complete copy

* fix grpc crash

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:automerge Automatically merge PR once all prerequisites pass. C:CLI
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants