feat: Modify grpc gateway to be concurrent (backport #11234) #11536
Conversation
![mergify[bot]](https://avatars.githubusercontent.com/in/10562?s=60&v=4){kind=small}
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
| if err != nil { | ||
| return err | ||
| } | ||
| grpcAddress := fmt.Sprintf("127.0.0.1:%s", port) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
with 0.46 around the corner we can close this |
This is an automatic backport of pull request #11234 done by Mergify.
Cherry-pick of 5356a86 has failed:
To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/github/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally
Mergify commands and options
More conditions and actions can be found in the documentation.
You can also trigger Mergify actions by commenting on this pull request:
@Mergifyio refreshwill re-evaluate the rules@Mergifyio rebasewill rebase this PR on its base branch@Mergifyio updatewill merge the base branch into this PR@Mergifyio backport <destination>will backport this PR on<destination>branchAdditionally, on Mergify dashboard you can:
Finally, you can contact us on https://mergify.com