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

refactor!: Remove grpc-web/rosetta and improve server performance #1418

Merged
merged 10 commits into from
Jun 14, 2024

Conversation

tkxkd0159
Copy link
Member

@tkxkd0159 tkxkd0159 commented Jun 12, 2024

Description

closes: #1416

After update:

  • Node operator need to remove rosetta/grpc-web config in app.toml
  • Node operator need to run envoy proxy if they want to support grpc-web
  • Node operator need to run cosmos/rosetta if they want to support Coinbase Rosetta API. This command also can be added in Finschia binary itself (root.go)

At this time, I also want to refactor our server code structure based on cosmos/cosmos-sdk#15041, cosmos/cosmos-sdk#16152.

Motivation and context

  1. We separate rosetta/grpc-web from main core to apply MSA. It will replace legacy code with latest one and improve the complication level of dependencies.
  2. To improve query performance, we modify grpc-gateway to be concurrent.

How has this been tested?

Screenshots (if appropriate):

Checklist:

  • I followed the contributing guidelines and code of conduct.
  • I have added a relevant changelog to CHANGELOG.md
  • I have added tests to cover my changes.
  • I have updated the documentation accordingly.
  • I have updated API documentation client/docs/swagger-ui/swagger.yaml

@tkxkd0159 tkxkd0159 self-assigned this Jun 12, 2024
@tkxkd0159 tkxkd0159 added the A: Client Breaking Breaking Protobuf, gRPC and REST routes used by end-users. label Jun 12, 2024
Copy link

codecov bot commented Jun 12, 2024

Codecov Report

Attention: Patch coverage is 69.35484% with 19 lines in your changes missing coverage. Please review.

Project coverage is 69.57%. Comparing base (6054848) to head (4d9cc76).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1418      +/-   ##
==========================================
+ Coverage   68.66%   69.57%   +0.90%     
==========================================
  Files         767      751      -16     
  Lines       63512    62259    -1253     
==========================================
- Hits        43611    43316     -295     
+ Misses      17234    16308     -926     
+ Partials     2667     2635      -32     
Files Coverage Δ
server/config/config.go 45.94% <ø> (-0.38%) ⬇️
server/config/toml.go 15.78% <ø> (ø)
simapp/simd/cmd/root.go 87.58% <ø> (-0.09%) ⬇️
testutil/network/util.go 76.36% <94.73%> (+3.39%) ⬆️
client/grpc_query.go 26.86% <0.00%> (-0.83%) ⬇️
testutil/network/network.go 84.38% <71.42%> (+1.79%) ⬆️
client/context.go 59.23% <0.00%> (+0.79%) ⬆️
server/grpc/server.go 72.50% <71.42%> (+6.78%) ⬆️
server/api/server.go 56.00% <58.82%> (-1.36%) ⬇️

... and 1 file with indirect coverage changes

@tkxkd0159 tkxkd0159 marked this pull request as ready for review June 12, 2024 04:38
@tkxkd0159 tkxkd0159 changed the title refactor!: Remove grpc-web/rosetta and modify grpc-gateway refactor!: Remove grpc-web/rosetta and improve grpc-gateway Jun 12, 2024
@tkxkd0159 tkxkd0159 changed the title refactor!: Remove grpc-web/rosetta and improve grpc-gateway refactor!: Remove grpc-web/rosetta and improve grpc-gateway performance Jun 12, 2024
@tkxkd0159 tkxkd0159 requested a review from a team June 12, 2024 06:22
@tkxkd0159 tkxkd0159 added the A: improvement Changes in existing functionality label Jun 12, 2024
zemyblue
zemyblue previously approved these changes Jun 12, 2024
Copy link
Member

@zemyblue zemyblue left a comment

Choose a reason for hiding this comment

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

Please introduce the reason why we remove this in this PR's description.

server/start.go Outdated Show resolved Hide resolved
jaeseung-bae
jaeseung-bae previously approved these changes Jun 12, 2024
zemyblue
zemyblue previously approved these changes Jun 13, 2024
0Tech
0Tech previously approved these changes Jun 13, 2024
CHANGELOG.md Outdated Show resolved Hide resolved
testutil/network/util.go Outdated Show resolved Hide resolved
server/start.go Show resolved Hide resolved
@tkxkd0159 tkxkd0159 dismissed stale reviews from 0Tech, zemyblue, and jaeseung-bae via f688899 June 13, 2024 02:59
@tkxkd0159 tkxkd0159 changed the title refactor!: Remove grpc-web/rosetta and improve grpc-gateway performance refactor!: Remove grpc-web/rosetta and improve grpc-gateway/server performance Jun 13, 2024
@tkxkd0159 tkxkd0159 changed the title refactor!: Remove grpc-web/rosetta and improve grpc-gateway/server performance refactor!: Remove grpc-web/rosetta and improve grpc-gateway performance Jun 13, 2024
@tkxkd0159 tkxkd0159 changed the title refactor!: Remove grpc-web/rosetta and improve grpc-gateway performance refactor!: Remove grpc-web/rosetta and improve server performance Jun 13, 2024
@tkxkd0159 tkxkd0159 merged commit 2ed0e6c into main Jun 14, 2024
38 checks passed
@tkxkd0159 tkxkd0159 deleted the modify-grpc-web branch June 14, 2024 08:30
tkxkd0159 added a commit that referenced this pull request Jun 17, 2024
tkxkd0159 added a commit that referenced this pull request Jun 17, 2024
…-gw (backport #1153, #1418) (#1423)

* backport finschia-sdk#1153

* backport finschia-sdk#1418

* Keep using Ostracon under tm naming
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: Client Breaking Breaking Protobuf, gRPC and REST routes used by end-users. A: improvement Changes in existing functionality C:server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Need to fix old server-side features (grpc-web/rosetta, grpc-gateway)
4 participants